Liron Ar has posted comments on this change. Change subject: engine: Block setting a plugged disk as read only when the vm is running ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/25492/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java: Line 258: } Line 259: Line 260: private boolean validateCanUpdateReadOnly() { Line 261: if (shouldUpdateReadOnly() && getVm().getStatus() != VMStatus.Down && vmDeviceForVm.getIsPlugged()) { Line 262: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN); In my opinion more informative message can be returned (so the user will know that he could unplug it for changing and that he doesn't must to turn off the vm). Line 263: } Line 264: return true; Line 265: } Line 266: http://gerrit.ovirt.org/#/c/25492/1/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java: Line 184: .contains(VdcBllMessages.SHAREABLE_DISK_IS_NOT_SUPPORTED_BY_VOLUME_FORMAT.toString())); Line 185: } Line 186: Line 187: @Test Line 188: public void canDoActionFailedUpdateReadOnly() { please add also test that it doesn't fail when the device is unplugged. Line 189: VmDevice device = createVmDevice(diskImageGuid, vmId); Line 190: doReturn(device).when(vmDeviceDAO).get(device.getId()); Line 191: Line 192: // make sure that device is plugged -- To view, visit http://gerrit.ovirt.org/25492 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0bd16d91e02f26f2d6697b31233669a80a98534c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
