Vered Volansky has posted comments on this change. Change subject: engine: Do not add disk in snapshot preview ......................................................................
Patch Set 4: I would prefer that you didn't submit this (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java Line 223: Line 224: protected boolean isVmNotInPreviewSnapshot() { Line 225: return Line 226: getVmId() != null && Line 227: validate(getSnapshotsValidator().vmNotDuringSnapshot(getVmId())) && I don't like creating same two SnapshotsValidator objects one after the other. Line 228: validate(getSnapshotsValidator().vmNotInPreview(getVmId())); Line 229: } Line 230: Line 231: protected boolean isVmNotLocked() { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java Line 128: if (getDiskLunMapDao().getDiskIdByLunId(lun.getLUN_id()) != null) { Line 129: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_LUN_IS_ALREADY_IN_USE); Line 130: } Line 131: Line 132: if (getVm() != null) { You can just return getVm == null || ( isVmNotInPreviewSnapshot() && isVmNotLocked() ) ) Line 133: if (!isVmNotInPreviewSnapshot()) { Line 134: return false; Line 135: } Line 136: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java Line 169: boolean isDiskUpdatedToShareable = newDisk.isShareable(); Line 170: boolean isOldDiskShareable = oldDisk.isShareable(); Line 171: Line 172: // Check if VM is not during snapshot. Line 173: if (!isVmNotInPreviewSnapshot()) { Why the removal of the CDA message here? Line 174: return false; Line 175: } Line 176: Line 177: if (!isOldDiskShareable && isDiskUpdatedToShareable) { -- To view, visit http://gerrit.ovirt.org/13327 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibfde0540f1d77456ed1412b015a0963f4682039f Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Libor Spevak <lspe...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Libor Spevak <lspe...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches