Yair Zaslavsky has posted comments on this change. Change subject: getAllTasksList\Status with spUUID retrieves info only if host is the SPM ......................................................................
Patch Set 7: (4 inline comments) .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/GetTaskListCommandParameters.java Line 3: import org.ovirt.engine.core.compat.Guid; Line 4: Line 5: public class GetTaskListCommandParameters extends VdsIdVDSCommandParametersBase { Line 6: public GetTaskListCommandParameters(Guid vdsId) { Line 7: _vdsId = vdsId; I would prefer not to have underscores :) Line 8: _spUUID = null; Line 9: } Line 10: Line 11: public GetTaskListCommandParameters(Guid vdsId, Guid spUUID) { Line 12: _vdsId = vdsId; Line 13: _spUUID = spUUID; Line 14: } Line 15: Line 16: private Guid _vdsId; _vdsId is already defined at base class, right? Line 17: private Guid _spUUID; Line 18: Line 19: public Guid getVdsId() { Line 20: return _vdsId; .................................................... File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/SPMGetAllTasksStatusesVDSCommand.java Line 15: Line 16: @Override Line 17: protected void ExecuteIrsBrokerCommand() { Line 18: setVDSReturnValue(ResourceManager.getInstance().runVdsCommand(VDSCommandType.HSMGetAllTasksStatuses, Line 19: new GetTaskListCommandParameters(getCurrentIrsProxyData().getCurrentVdsId(), No backward compatbility is required here? You're executing the same verb. Line 20: getParameters().getStoragePoolId()))); Line 21: } .................................................... File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HSMGetAllTasksStatusesVDSCommand.java Line 26: @Override Line 27: protected void ExecuteVdsBrokerCommand() { Line 28: _result = null; Line 29: if (getParameters().getStoragePoolId() != null) { Line 30: VDS vds = DbFacade.getInstance().getVdsDao().get(getParameters().getVdsId()); Hi, I would prefer if you will have at base class a method VdsDao getVdsDao (protected method) that returns DbFacade.getInstance().getVdsDao This reason for this as it will help us mock vds broker commands one day. Line 31: if (FeatureSupported.saferGetAllTasksRequest(vds.getVdsGroupCompatibilityVersion())) { Line 32: _result = getBroker().getAllTasksStatuses(getParameters().getStoragePoolId().toString()); Line 33: } Line 34: } -- To view, visit http://gerrit.ovirt.org/13450 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff7d8db4e4ad6b3f809085aff7216ac8a457b633 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: liron aravot <liron.ara...@gmail.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches