Liron Aravot has posted comments on this change. Change subject: core: Check attached VMs snapshot status when moving a disk ......................................................................
Patch Set 4: (2 comments) https://gerrit.ovirt.org/#/c/40080/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java: Line 181: } Line 182: return false; Line 183: } Line 184: Line 185: private boolean validateVmSnapshotStatus() { rethinking about that, if we preview a snapshot that doesn't include the disk, do we want to block moving it? basically we can check if the disk is part of the previewed snapshot and block just on that case. maybe checking only if there is a preview is too wide check. also, what about blocking in the UI? if we have the info there we can block it as well so the user won't fail after clicking. Line 186: SnapshotsValidator snapshotsValidator = getSnapshotsValidator(); Line 187: for (Pair<VM, VmDevice> pair : getVmsWithVmDeviceInfoForDiskId()) { Line 188: VmDevice vmDevice = pair.getSecond(); Line 189: if (vmDevice.getSnapshotId() == null) { // Skip check for VMs with connected snapshot https://gerrit.ovirt.org/#/c/40080/4/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java: Line 234: CanDoActionTestUtils.runAndAssertCanDoActionSuccess(command); Line 235: } Line 236: Line 237: @Test Line 238: public void canDoActionVmInPreview() { what about test for the other check you've added? Line 239: initializeCommand(ImageOperation.Move); Line 240: initSnapshotValidator(); Line 241: initVmForSpace(); Line 242: initVmDiskImage(false); -- To view, visit https://gerrit.ovirt.org/40080 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I80feffc9f11aed1bc8fa7a6f80562d05ce815c56 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Idan Shaby <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Tal Nisan <[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
