Arik Hadas has posted comments on this change. Change subject: core: remove RunVmCommandBase#getDestinationVds ......................................................................
Patch Set 3: (3 comments) http://gerrit.ovirt.org/#/c/24255/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java: Line 69: .TranslateErrorTextSingle(migrationErrorCode.name(), true); Line 70: } Line 71: Line 72: protected VDS getDestinationVds() { Line 73: if (destinationVdsId == null) { > wouldn't adding this be simpler instead of rewriting? why is it be better to do this check in the DAO level and not in this command? Line 74: return cachedDestinationVds = null; Line 75: } Line 76: Line 77: if (cachedDestinationVds == null || !cachedDestinationVds.getId().equals(destinationVdsId)) { http://gerrit.ovirt.org/#/c/24255/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java: Line 113: getPredefinedDestinationVdsId > either call this metod getDedicatedVdsId or remove it completely. its just but it doesn't always return the dedicated vds. in run vm once flow, it returns the destination vds that the user set, so we need this method in order to be able to override it in RunVmOnce, and I think that getDedicatedVdsId would be confusing since it is overridden in RunVmOnce Line 614: getPredefinedDestinationVdsId > same here. predefinedDestinationVds isn't any clearer than destinationVds will getPredefinedVdsId be better in your opinion? -- To view, visit http://gerrit.ovirt.org/24255 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie1d8fb852a88203378b3bcd3f1d6f96e469e5cb9 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
