Allon Mureinik has posted comments on this change. Change subject: core: adding SpmCommand ......................................................................
Patch Set 2: (5 comments) I really don't like the name SpmCommand (since, well, it isn't). How about ImageStreamingCommand? http://gerrit.ovirt.org/#/c/32543/2//COMMIT_MSG Commit Message: Line 5: CommitDate: 2014-09-07 12:53:50 +0300 Line 6: Line 7: core: adding SpmCommand Line 8: Line 9: This patch adds SpmCommand as a parent class of UploadStreamCommand to s/adds/extracts/ Line 10: contain shared logic between this command and the to-be-added Line 11: RetrieveImageDataCommand. Line 12: Line 13: Change-Id: I32166ee993bce33680f6b2feb5359666e414c777 http://gerrit.ovirt.org/#/c/32543/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SpmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SpmCommand.java: Line 17: import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; Line 18: import org.ovirt.engine.core.compat.Guid; Line 19: Line 20: public abstract class SpmCommand<T extends ImagesContainterParametersBase> extends BaseImagesCommand<T> { Line 21: Guid cachedSpmId; should be private Line 22: Line 23: public SpmCommand(T parameters, CommandContext cmdContext) { Line 24: super(parameters, cmdContext); Line 25: setStoragePoolId(getParameters().getStoragePoolId()); Line 47: if (getPoolSpmId() == null) { Line 48: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_SPM); Line 49: } Line 50: Line 51: setStoragePool(null); why do you need this? Line 52: if (getStoragePool() == null) { Line 53: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST); Line 54: } Line 55: Line 48: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_SPM); Line 49: } Line 50: Line 51: setStoragePool(null); Line 52: if (getStoragePool() == null) { Actually, StoragePoolValidator.isUp() should cover this (perhaps it should be renamed to existsAndUp()? Or split to two methods? (not the scope of this patch) Line 53: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST); Line 54: } Line 55: Line 56: if (!validate(new StoragePoolValidator(getStoragePool()).isUp())) { Line 105: } Line 106: Line 107: @Override Line 108: protected void endWithFailure() { Line 109: setSucceeded(true); ? Line 110: } Line 111: Line 112: @Override Line 113: protected Map<String, Pair<String, String>> getSharedLocks() { -- To view, visit http://gerrit.ovirt.org/32543 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I32166ee993bce33680f6b2feb5359666e414c777 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: [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
