[Engine-patches] Change in ovirt-engine[ovirt-engine-3.4]: engine: Block setting a plugged disk as read only when the v...

[mlipchuk]

Hello Sergey Gotliv,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/25492

to review the following change.

Change subject: engine: Block setting a plugged disk as read only when the vm 
is running
......................................................................

engine: Block setting a plugged disk as read only when the vm is running

Change-Id: I0bd16d91e02f26f2d6697b31233669a80a98534c
Bug-Url: https://bugzilla.redhat.com/1050840
Signed-off-by: Sergey Gotliv <sgot...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java
2 files changed, 29 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/92/25492/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
index 385fecf..4224bae 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
@@ -258,7 +258,7 @@
     }
 
     private boolean validateCanUpdateReadOnly() {
-        if (shouldUpdateReadOnly() && getVm().getStatus() != VMStatus.Down) {
+        if (shouldUpdateReadOnly() && getVm().getStatus() != VMStatus.Down && 
vmDeviceForVm.getIsPlugged()) {
             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN);
         }
         return true;
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java
index 3ff4da7..f5b74bc 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmDiskCommandTest.java
@@ -185,6 +185,25 @@
     }
 
     @Test
+    public void canDoActionFailedUpdateReadOnly() {
+        VmDevice device = createVmDevice(diskImageGuid, vmId);
+        doReturn(device).when(vmDeviceDAO).get(device.getId());
+
+        // make sure that device is plugged
+        assertEquals(true, device.getIsPlugged());
+
+        DiskImage disk = createDiskImage();
+        disk.setReadOnly(true);
+
+        when(diskDao.get(diskImageGuid)).thenReturn(disk);
+
+        UpdateVmDiskParameters parameters = createParameters();
+        initializeCommand(parameters, Arrays.asList(createVm(VMStatus.Up)));
+
+        CanDoActionTestUtils.runAndAssertCanDoActionFailure(command, 
VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN);
+    }
+
+    @Test
     public void canDoActionFailedShareableDiskOnGlusterDomain() throws 
Exception {
         UpdateVmDiskParameters parameters = createParameters();
         DiskImage disk = createShareableDisk(VolumeFormat.RAW);
@@ -303,10 +322,10 @@
         when(diskDao.get(diskImageGuid)).thenAnswer(new Answer() {
             @Override
             public Object answer(InvocationOnMock invocationOnMock) throws 
Throwable {
-            final DiskImage oldDisk = createDiskImage();
-            oldDisk.setDiskInterface(DiskInterface.VirtIO);
-            assert(oldDisk.getDiskInterface() != 
parameters.getDiskInfo().getDiskInterface());
-            return oldDisk;
+                final DiskImage oldDisk = createDiskImage();
+                oldDisk.setDiskInterface(DiskInterface.VirtIO);
+                assert (oldDisk.getDiskInterface() != 
parameters.getDiskInfo().getDiskInterface());
+                return oldDisk;
             }
         });
 
@@ -520,8 +539,12 @@
     }
 
     protected VM createVmStatusDown(VM... otherPluggedVMs) {
+        return createVm(VMStatus.Down);
+    }
+
+    protected VM createVm(VMStatus status) {
         VM vm = new VM();
-        vm.setStatus(VMStatus.Down);
+        vm.setStatus(status);
         vm.setGuestOs("rhel6");
         vm.setId(vmId);
         vm.setVdsGroupCompatibilityVersion(Version.v3_1);


-- 
To view, visit http://gerrit.ovirt.org/25492
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
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: Sergey Gotliv <sgot...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches