Laszlo Hornyak has posted comments on this change.
Change subject: core: refactoring VdsSelector API
......................................................................
Patch Set 3: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/VdsSelector.java
Line 62: setDestinationVdsId(destinationVdsId);
Line 63: this.memoryChecker = memoryChecker;
Line 64: }
Line 65:
Line 66: public Guid getVdsToRunOn(List<VDS> vdsList,
out of this name I would believe that it does not alter the state of the
object, however it sets the vdsBlackList, so maybe a other name would be nice?
What about selectVdsToRunOn?
Line 67: List<Guid> vdsBlackList,
Line 68: boolean isMigrate) {
Line 69: this.vdsBlackList = vdsBlackList;
Line 70: Guid result = Guid.Empty;
Line 95: public boolean canFindVdsToRunOn(List<VDS> vdsList,
Line 96: List<Guid> vdsBlackList,
Line 97: List<String> messages,
Line 98: boolean isMigrate) {
Line 99: this.vdsBlackList = vdsBlackList;
Another cancidate for renaming?
Line 100: boolean returnValue = false;
Line 101: if (getDestinationVdsId() != null) {
Line 102: VDS targetVds = null;
Line 103: if (vdsList != null) {
Line 300
Line 301
Line 302
Line 303
Line 304
Nice to see this method going away
....................................................
Commit Message
Line 5: CommitDate: 2013-06-07 16:59:06 +0200
Line 6:
Line 7: core: refactoring VdsSelector API
Line 8:
Line 9: Host selection should become stateless,
This sounds like you are intending to make VdsSelector stateless while you are
adding one more stateful field to it. Could you clarify this?
Line 10: therefore moving failed host list out of this class (vdsBlackList).
Line 11: In addition, each vdsSelector call should supply a list of hosts
Line 12: that the selector should 'work' on.
Line 13:
--
To view, visit http://gerrit.ovirt.org/14997
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d936e1b96628e426e944d77730e3ca08a13570d
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
