Maor Lipchuk has uploaded a new change for review.

Change subject: core: Add a validation when deactivate ISO domain.
......................................................................

core: Add a validation when deactivate ISO domain.

Adding a CDA when deactivating an ISO storage domain if there
are VMs with an ISO file attached to them and are not down.

Signed-off-by: Maor Lipchuk <mlipc...@redhat.com>
Bug-Url: https://bugzilla.redhat.com/969641
Change-Id: I47c1a8155762ecd0b04bb17676151946982bb919
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
M 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
M 
frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
M 
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
7 files changed, 105 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/62/24462/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
index fc399cd..58f756d 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
@@ -2,9 +2,11 @@
 
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.bll.LockIdNameAttribute;
 import org.ovirt.engine.core.bll.LockMessagesMatchUtil;
 import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute;
@@ -18,6 +20,9 @@
 import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId;
 import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
 import org.ovirt.engine.core.common.businessentities.VDS;
+import org.ovirt.engine.core.common.businessentities.VMStatus;
+import org.ovirt.engine.core.common.businessentities.VmDynamic;
+import org.ovirt.engine.core.common.businessentities.VmStatic;
 import org.ovirt.engine.core.common.errors.VdcBllMessages;
 import org.ovirt.engine.core.common.locks.LockingGroup;
 import org.ovirt.engine.core.common.utils.Pair;
@@ -112,6 +117,9 @@
                 return 
failCanDoAction(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_MASTER_WITH_LOCKED_DOMAINS);
             }
         }
+        if (!isRunningVmsWithIsoAttached()) {
+            return false;
+        }
         if (!getParameters().getIsInternal()
                 && !getVmDAO()
                         
.getAllActiveForStorageDomain(getStorageDomain().getId())
@@ -132,6 +140,31 @@
         return true;
     }
 
+    protected boolean isRunningVmsWithIsoAttached() {
+        if (!getParameters().getIsInternal() && 
getStorageDomain().getStorageDomainType() == StorageDomainType.ISO) {
+            List<String> vmNames = getVmsWithAttachedISO();
+            if (!vmNames.isEmpty()) {
+                return 
failCanDoAction(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED,
+                        String.format("$VmNames %1$s", 
StringUtils.join(vmNames, ",")));
+            }
+        }
+        return true;
+    }
+
+    protected List<String> getVmsWithAttachedISO() {
+        List<VmStatic> vms = 
getVmStaticDAO().getAllByStoragePoolId(getStorageDomain().getStoragePoolId());
+        List<String> vmNames = new LinkedList<>();
+        for (VmStatic vmStatic : vms) {
+            VmDynamic vmDynamic = getVmDynamicDAO().get(vmStatic.getId());
+            if (getVmDynamicDAO().get(vmStatic.getId()).getStatus() != 
VMStatus.Down
+                    && !StringUtils.isEmpty(vmDynamic.getCurrentCd() != null ? 
vmDynamic.getCurrentCd()
+                            : vmStatic.getIsoPath())) {
+                vmNames.add(vmStatic.getName());
+            }
+        }
+        return vmNames;
+    }
+
     /**
      * Filter out the domains with the requested status from the given domains 
list, excluding the domain which the
      * command is run for.
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java
index 2160dca..374b1fe 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommandTest.java
@@ -1,5 +1,6 @@
 package org.ovirt.engine.core.bll.storage;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doReturn;
@@ -8,7 +9,9 @@
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
+import java.util.List;
 
+import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -17,13 +20,16 @@
 import org.ovirt.engine.core.bll.context.CompensationContext;
 import org.ovirt.engine.core.bll.interfaces.BackendInternal;
 import org.ovirt.engine.core.common.action.StorageDomainPoolParametersBase;
+import org.ovirt.engine.core.common.businessentities.StorageDomain;
 import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
+import org.ovirt.engine.core.common.businessentities.StorageDomainType;
+import org.ovirt.engine.core.common.businessentities.StoragePool;
+import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap;
 import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId;
 import org.ovirt.engine.core.common.businessentities.VDS;
 import org.ovirt.engine.core.common.businessentities.VDSStatus;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
-import org.ovirt.engine.core.common.businessentities.StoragePool;
-import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMap;
+import org.ovirt.engine.core.common.businessentities.VmStatic;
+import org.ovirt.engine.core.common.errors.VdcBllMessages;
 import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.common.vdscommands.VDSParametersBase;
@@ -35,6 +41,8 @@
 import org.ovirt.engine.core.dao.StoragePoolDAO;
 import org.ovirt.engine.core.dao.StoragePoolIsoMapDAO;
 import org.ovirt.engine.core.dao.VdsDAO;
+import org.ovirt.engine.core.dao.VmDynamicDAO;
+import org.ovirt.engine.core.dao.VmStaticDAO;
 import org.ovirt.engine.core.utils.MockEJBStrategyRule;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -58,24 +66,34 @@
     private AsyncTaskDAO asyncTaskDAO;
     @Mock
     private VDS vds;
+    @Mock
+    private VmStaticDAO vmStaticDAO;
+    @Mock
+    private VmDynamicDAO vmDynamicDAO;
+
     StoragePoolIsoMap map = new StoragePoolIsoMap();
+
+    private StorageDomain domain = null;
 
     @ClassRule
     public static MockEJBStrategyRule mockEjbRule = new MockEJBStrategyRule();
 
+    StorageDomainPoolParametersBase params = new 
StorageDomainPoolParametersBase(Guid.newGuid(), Guid.newGuid());
+    DeactivateStorageDomainCommand<StorageDomainPoolParametersBase> cmd =
+            spy(new 
DeactivateStorageDomainCommand<StorageDomainPoolParametersBase>(params));
+
+    @Before
+    public void setup() {
+        doReturn(dbFacade).when(cmd).getDbFacade();
+        when(dbFacade.getStoragePoolDao()).thenReturn(storagePoolDAO);
+        when(dbFacade.getStorageDomainDao()).thenReturn(storageDomainDAO);
+    }
+
     @Test
     public void statusSetInMap() {
-        StorageDomainPoolParametersBase params = new 
StorageDomainPoolParametersBase(Guid.newGuid(), Guid.newGuid());
-        DeactivateStorageDomainCommand<StorageDomainPoolParametersBase> cmd =
-                spy(new 
DeactivateStorageDomainCommand<StorageDomainPoolParametersBase>(params));
-
         
doReturn(mock(IStorageHelper.class)).when(cmd).getStorageHelper(any(StorageDomain.class));
-        doReturn(dbFacade).when(cmd).getDbFacade();
-
         when(dbFacade.getStoragePoolIsoMapDao()).thenReturn(isoMapDAO);
-        when(dbFacade.getStoragePoolDao()).thenReturn(storagePoolDAO);
         when(dbFacade.getVdsDao()).thenReturn(vdsDAO);
-        when(dbFacade.getStorageDomainDao()).thenReturn(storageDomainDAO);
         when(dbFacade.getAsyncTaskDao()).thenReturn(asyncTaskDAO);
         when(storagePoolDAO.get(any(Guid.class))).thenReturn(new 
StoragePool());
         when(isoMapDAO.get(any(StoragePoolIsoMapId.class))).thenReturn(map);
@@ -95,4 +113,40 @@
         cmd.executeCommand();
         assertTrue(map.getStatus() == StorageDomainStatus.Maintenance);
     }
+
+    @Test
+    public void testVmsWithNoIsoAttached() {
+        mockDomain();
+        doReturn(domain).when(cmd).getStorageDomain();
+        when(dbFacade.getVmStaticDao()).thenReturn(vmStaticDAO);
+        List<VmStatic> listVMs = new ArrayList<>();
+        
when(vmStaticDAO.getAllByStoragePoolId(any(Guid.class))).thenReturn(listVMs);
+        assertTrue(cmd.isRunningVmsWithIsoAttached());
+        assertTrue(cmd.getReturnValue().getCanDoActionMessages().isEmpty());
+    }
+
+    @Test
+    public void testVmsWithIsoAttached() {
+        setup();
+        mockDomain();
+        doReturn(domain).when(cmd).getStorageDomain();
+        when(dbFacade.getVmStaticDao()).thenReturn(vmStaticDAO);
+        when(dbFacade.getVmDynamicDao()).thenReturn(vmDynamicDAO);
+
+        List<VmStatic> listVMs = new ArrayList<>();
+        VmStatic vmStatic = new VmStatic();
+        vmStatic.setName("TestVM");
+        vmStatic.setId(Guid.newGuid());
+        listVMs.add(vmStatic);
+        doReturn(listVMs).when(cmd).getVmsWithAttachedISO();
+        assertFalse(cmd.isRunningVmsWithIsoAttached());
+        assertTrue(cmd.getReturnValue()
+                .getCanDoActionMessages()
+                
.contains(VdcBllMessages.ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED.toString()));
+    }
+
+    private void mockDomain() {
+        domain = new StorageDomain();
+        domain.setStorageDomainType(StorageDomainType.ISO);
+    }
 }
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
index 397bff5..2d89f60 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
@@ -563,6 +563,7 @@
     ERROR_CANNOT_DEACTIVATE_MASTER_WITH_NON_DATA_DOMAINS(ErrorType.CONFLICT),
     ERROR_CANNOT_DEACTIVATE_MASTER_WITH_LOCKED_DOMAINS(ErrorType.CONFLICT),
     ERROR_CANNOT_DEACTIVATE_DOMAIN_WITH_TASKS(ErrorType.CONFLICT),
+    
ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED(ErrorType.CONFLICT),
     
ERROR_CANNOT_DEACTIVATE_MASTER_DOMAIN_WITH_TASKS_ON_POOL(ErrorType.CONFLICT),
     
ERROR_CANNOT_ADD_STORAGE_POOL_WITHOUT_DATA_AND_ISO_DOMAINS(ErrorType.BAD_PARAMETERS),
     ERROR_CANNOT_ADD_STORAGE_POOL_WITHOUT_DATA_DOMAIN(ErrorType.CONFLICT),
diff --git 
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties 
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
index f3e1147..f3688da 100644
--- 
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++ 
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -818,6 +818,7 @@
 VAR__TYPE__VM__CLUSTER=$type VM Cluster
 VDS_FENCE_OPERATION_FAILED=Cannot ${action} ${type}. Fence operation failed.
 VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO=Cannot ${action} without active 
ISO domain.
+ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED=Cannot ${action} 
${type}. The ISO Storage Domain is being used by the following VMs: ${VmNames}.
 MAC_ADDRESS_IS_IN_USE=MAC Address is in use.
 CAN_DO_ACTION_GENERAL_FAILURE=General command validation failure.
 ERROR_CANNOT_REMOVE_ACTIVE_STORAGE_POOL=Cannot remove an active Data Center.
diff --git 
a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
 
b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
index 0367d8e..7dbe433 100644
--- 
a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
+++ 
b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
@@ -2212,6 +2212,9 @@
     @DefaultStringValue("Cannot ${action} without active ISO domain.")
     String VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO();
 
+    @DefaultStringValue("Cannot ${action} ${type}. The ISO Storage Domain is 
being used by the following VMs: ${VmNames}.")
+    String ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED();
+
     @DefaultStringValue("MAC Address is in use.")
     String MAC_ADDRESS_IS_IN_USE();
 
diff --git 
a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
 
b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
index 85b5cab..31b5648 100644
--- 
a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
+++ 
b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -792,6 +792,7 @@
 VAR__TYPE__VM__CLUSTER=$type VM Cluster
 VDS_FENCE_OPERATION_FAILED=Cannot ${action} ${type}. Fence operation failed.
 VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO=Cannot ${action} without active 
ISO domain.
+ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED=Cannot ${action} 
${type}. The ISO Storage Domain is being used by the following VMs: ${VmNames}.
 MAC_ADDRESS_IS_IN_USE=Mac Address is in use.
 CAN_DO_ACTION_GENERAL_FAILURE=General command validation failure.
 ERROR_CANNOT_REMOVE_ACTIVE_STORAGE_POOL=Cannot remove an active Data Center.
diff --git 
a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
 
b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
index 7757a9d..dcd5759 100644
--- 
a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
+++ 
b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -815,6 +815,7 @@
 VAR__TYPE__VM__CLUSTER=$type VM Cluster
 VDS_FENCE_OPERATION_FAILED=Cannot ${action} ${type}. Fence operation failed.
 VM_CANNOT_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO=Cannot ${action} without active 
ISO domain.
+ERROR_CANNOT_DEACTIVATE_STORAGE_DOMAIN_WITH_ISO_ATTACHED=Cannot ${action} 
${type}. The ISO Storage Domain is being used by the following VMs: ${VmNames}.
 MAC_ADDRESS_IS_IN_USE=Mac Address is in use.
 CAN_DO_ACTION_GENERAL_FAILURE=General command validation failure.
 ERROR_CANNOT_REMOVE_ACTIVE_STORAGE_POOL=Cannot remove an active Data Center.


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I47c1a8155762ecd0b04bb17676151946982bb919
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.4
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to