Martin Sivák has posted comments on this change. Change subject: engine: scheduling host- find free VF ......................................................................
Patch Set 14: (3 comments) The Scheduler is supposed to select a host based on constraint and weight checks and without any side effects. So I would prefer if this worked a bit differently: - new singleton(?) device tracking service (to keep track of vnic allocations) would lock on something cluster related - the main scheduler would select a host that has enough free vnic "slots" using proper filter and weight policy units without any API change - necessary devices are allocated on the returned host and recorded in the new vnic tracking service / VM / Host entities as needed - device tracker's lock is released The policy units can of course use the new tracking service, VM and host info to get the necessary info (read only), but give us the option to mock these please (for test purposes). Ideally we could move the pending memory info we currently have to the device tracker / resource manager as well to make the scheduler clean of side effects. We plan to do that in the future anyway.. https://gerrit.ovirt.org/#/c/37931/14//COMMIT_MSG Commit Message: Line 5: CommitDate: 2015-04-16 09:03:50 +0300 Line 6: Line 7: engine: scheduling host- find free VF Line 8: Line 9: A VF is marked as suitable for a passthrough vnic in case: What does VF and PF stand for? Please link to a bug or design documentation as I have no idea why this needs to change the scheduling API. Line 10: 1. It is free (reported by vdsCaps, not attached to vm, doesn't have vlan Line 11: device or network attached to it). Line 12: 2. Its PF has the vnic's network (or label) in it's vf's configuration. Line 13: https://gerrit.ovirt.org/#/c/37931/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/PolicyUnitImpl.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/PolicyUnitImpl.java: Line 157: this.memoryChecker = memoryChecker; Line 158: Line 159: } Line 160: Line 161: public void setHostToVnicToVfMap(Map<Guid, Map<Guid, String>> hostToVnicToVfMap) { Why do all policy units need such a map? PolicyUnitImpl is the base class for all policy units. Line 162: this.hostToVnicToVfMap = hostToVnicToVfMap; Line 163: } https://gerrit.ovirt.org/#/c/37931/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SchedulingManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SchedulingManager.java: Line 256: List<Guid> hostWhiteList, Line 257: Guid destHostId, Line 258: List<String> messages, Line 259: VdsFreeMemoryChecker memoryChecker, Line 260: Map<Guid, String> passthroughVnicToVfMap, > couldn't see how this is related to the schedule api. I could accept a change that puts additional information to the SchedulingResult class (+ necessary return value changes). But I would like to know the reason why this is reported from the scheduler instead of being provided to it with the VM. The way we anticipated the scheduler to operate is that it will get all the information about a VM and selects a host where such VM can run. Line 261: String correlationId) { Line 262: clusterLockMap.putIfAbsent(cluster.getId(), new Semaphore(1)); Line 263: try { Line 264: log.debug("Scheduling started, correlation Id: {}", correlationId); -- To view, visit https://gerrit.ovirt.org/37931 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I58c7aaa7a5a7160132418c397840583209aa8371 Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
