Omer Frenkel has posted comments on this change.
Change subject: engine: Move Gluster host
......................................................................
Patch Set 4: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
Line 88: if (getVdsGroup(getTargetClusterId()).supportsGlusterService()
you can use the local targetCluster instead of getting it again from db
and since it is needed also in execute maybe just save it as a member of the
command to reuse
then, the getVdsGroup method will not be needed
Line 103: private boolean hasUpServer(Guid clusterId) {
you could send the cluster object instead of just the id,
so you wouldn't need to get it again from the db just for the name
Line 200: setSucceeded(returnValue.getSucceeded());
why are you setting here the command has succeeded?
what if an error will occur later on the execution?
instead, this method can return boolean for its success status
Line 211: : getVds().gethost_name();
the above logic is not relevant in remove?
Line 217: setSucceeded(returnValue.getSucceeded());
same comment as in remove
--
To view, visit http://gerrit.ovirt.org/5285
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I24bd7d34c2312c34de63371912444c21b2896737
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Selvasundaram <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Selvasundaram <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
