Hello Maor Lipchuk,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/36090
to review the following change.
Change subject: core: filter candidate block Storage Domains with existing LUNs.
......................................................................
core: filter candidate block Storage Domains with existing LUNs.
Filter out candidates of imported Storage Domain if we have external
LUNs which are part of the vg.
Change-Id: Id82e5e1d3df8ce265833efb3fc4b9050ca73b679
Bug-Url: https://bugzilla.redhat.com/1157243
Signed-off-by: Maor Lipchuk <mlipc...@redhat.com>
Signed-off-by: Tal Nisan <tni...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingBlockStorageDomainCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQuery.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQueryTest.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Entities.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
9 files changed, 81 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/90/36090/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingBlockStorageDomainCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingBlockStorageDomainCommand.java
index 27fab79..439fad6 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingBlockStorageDomainCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddExistingBlockStorageDomainCommand.java
@@ -5,9 +5,11 @@
import java.util.List;
import java.util.Map;
+import org.apache.commons.collections.CollectionUtils;
import org.ovirt.engine.core.bll.LockMessagesMatchUtil;
import org.ovirt.engine.core.common.action.LockProperties;
import org.ovirt.engine.core.common.action.StorageDomainManagementParameter;
+import org.ovirt.engine.core.common.businessentities.Entities;
import org.ovirt.engine.core.common.businessentities.LUNs;
import org.ovirt.engine.core.common.errors.VdcBllMessages;
import org.ovirt.engine.core.common.locks.LockingGroup;
@@ -53,6 +55,14 @@
if (getStorageDomainStaticDAO().get(getStorageDomain().getId()) !=
null) {
return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST);
}
+
+ List<LUNs> physicalVolumeIdsOnStorage =
getLUNsFromVgInfo(getStorageDomain().getStorage());
+ if
(CollectionUtils.containsAny(Entities.getPhysicalVolumesFromLunsList(physicalVolumeIdsOnStorage),
+
Entities.getPhysicalVolumesFromLunsList(getDbFacade().getLunDao().getAll()))) {
+ log.infoFormat("There are existing luns in the system which are
part of VG id '{0}'",
+ getStorageDomain().getStorage());
+ return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_IMPORT_STORAGE_DOMAIN_EXTERNAL_LUN_DISK_EXIST);
+ }
return true;
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQuery.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQuery.java
index 0180220..e525b19 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQuery.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQuery.java
@@ -31,6 +31,7 @@
import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.dao.LunDAO;
import org.ovirt.engine.core.dao.StorageDomainDAO;
import org.ovirt.engine.core.utils.linq.LinqUtils;
import org.ovirt.engine.core.utils.linq.Predicate;
@@ -206,6 +207,10 @@
@SuppressWarnings("unchecked")
protected List<StorageDomain>
getStorageDomainsByVolumeGroupIds(List<String> vgIDs) {
List<StorageDomain> storageDomains = new ArrayList<>();
+
+ // Get existing PhysicalVolumes.
+ List<String> existingPhysicalVolumeIds =
Entities.getPhysicalVolumesFromLunsList(getLunDAO().getAll());
+
for (String vgID : vgIDs) {
VDSReturnValue returnValue = null;
try {
@@ -218,6 +223,11 @@
}
ArrayList<LUNs> luns = (ArrayList<LUNs>)
returnValue.getReturnValue();
+ List<String> physicalVolumeIdsOnStorage =
Entities.getPhysicalVolumesFromLunsList(luns);
+ if (CollectionUtils.containsAny(physicalVolumeIdsOnStorage,
existingPhysicalVolumeIds)) {
+ log.infoFormat("There are existing luns in the system which
are part of VG id '{0}'", vgID);
+ continue;
+ }
// Get storage domain ID by a representative LUN
LUNs lun = luns.get(0);
@@ -273,6 +283,9 @@
return getDbFacade().getStorageDomainDao();
}
+ protected LunDAO getLunDAO() {
+ return getDbFacade().getLunDao();
+ }
/* Execute wrappers (for testing/mocking necessities) */
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQueryTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQueryTest.java
index a04fb05..e9b1c55 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQueryTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/GetUnregisteredBlockStorageDomainsQueryTest.java
@@ -23,6 +23,7 @@
import
org.ovirt.engine.core.common.vdscommands.HSMGetStorageDomainInfoVDSCommandParameters;
import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.dao.LunDAO;
import org.ovirt.engine.core.dao.StorageDomainDAO;
import java.util.ArrayList;
@@ -50,6 +51,9 @@
@Mock
private StorageDomainDAO storageDomainDAO;
+ @Mock
+ private LunDAO lunDAO;
+
@Override
@Before
public void setUp() throws Exception {
@@ -67,6 +71,7 @@
doReturn(storageDomainDAO).when(getQuery()).getStorageDomainDAO();
doReturn(getExistingStorageDomains()).when(storageDomainDAO).getAll();
+ doReturn(lunDAO).when(getQuery()).getLunDAO();
}
@Test
@@ -76,6 +81,7 @@
when(getQueryParameters().getVdsId()).thenReturn(Guid.newGuid());
List<LUNs> luns = getLUNs(storageDomainId, vgId);
+ doReturn(Collections.emptyList()).when(lunDAO).getAll();
doReturn(createSuccessVdcReturnValue()).when(getQuery()).
executeConnectStorageToVds(any(StorageServerConnectionParametersBase.class));
@@ -105,6 +111,40 @@
}
@Test
+ public void testIscsiExternalLunDiskPartOfUnregisteredDomain() {
+
when(getQueryParameters().getStorageType()).thenReturn(StorageType.ISCSI);
+
when(getQueryParameters().getStorageServerConnections()).thenReturn(getConnections());
+ when(getQueryParameters().getVdsId()).thenReturn(Guid.newGuid());
+
+ List<LUNs> luns = getLUNs(storageDomainId, vgId);
+ List<LUNs> externalLuns = new ArrayList<>();
+ externalLuns.add(luns.get(1));
+ doReturn(luns).when(lunDAO).getAll();
+
+ doReturn(createSuccessVdcReturnValue()).when(getQuery()).
+
executeConnectStorageToVds(any(StorageServerConnectionParametersBase.class));
+
+ doReturn(createGetDeviceListReturnValue(luns)).when(getQuery()).
+ executeGetDeviceList(any(GetDeviceListQueryParameters.class));
+
+ doReturn(createGetVGInfoReturnValue(luns)).when(getQuery()).
+ executeGetVGInfo(any(GetVGInfoVDSCommandParameters.class));
+
+
doReturn(createGetStorageDomainInfoReturnValue(storageDomainId)).when(getQuery()).
+
executeHSMGetStorageDomainInfo(any(HSMGetStorageDomainInfoVDSCommandParameters.class));
+
+ // Execute query
+ getQuery().executeQueryCommand();
+
+ // Assert query's results
+ Pair<List<StorageDomain>, List<StorageServerConnections>> returnValue =
+ getQuery().getQueryReturnValue().getReturnValue();
+
+ List<StorageDomain> storageDomains = returnValue.getFirst();
+ assertEquals(storageDomains.size(), 0);
+ }
+
+ @Test
public void testIscsiNotFoundUnregisteredDomain() {
when(getQueryParameters().getStorageType()).thenReturn(StorageType.ISCSI);
when(getQueryParameters().getStorageServerConnections()).thenReturn(getConnections());
@@ -112,6 +152,7 @@
List<LUNs> luns = getLUNs(existingStorageDomainId, existingVgId);
+ doReturn(Collections.emptyList()).when(lunDAO).getAll();
doReturn(createSuccessVdcReturnValue()).when(getQuery()).
executeConnectStorageToVds(any(StorageServerConnectionParametersBase.class));
@@ -142,6 +183,7 @@
List<LUNs> luns = getLUNs(storageDomainId, vgId);
+ doReturn(Collections.emptyList()).when(lunDAO).getAll();
doReturn(createGetDeviceListReturnValue(luns)).when(getQuery()).
executeGetDeviceList(any(GetDeviceListQueryParameters.class));
@@ -168,9 +210,9 @@
when(getQueryParameters().getStorageType()).thenReturn(StorageType.FCP);
when(getQueryParameters().getStorageServerConnections()).thenReturn(getConnections());
when(getQueryParameters().getVdsId()).thenReturn(Guid.newGuid());
-
List<LUNs> luns = getLUNs(existingStorageDomainId, existingVgId);
+ doReturn(Collections.emptyList()).when(lunDAO).getAll();
doReturn(createSuccessVdcReturnValue()).when(getQuery()).
executeConnectStorageToVds(any(StorageServerConnectionParametersBase.class));
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Entities.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Entities.java
index 65e2796..c4818d5 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Entities.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Entities.java
@@ -62,6 +62,14 @@
}
}
+ public static List<String> getPhysicalVolumesFromLunsList(List<LUNs> luns)
{
+ List<String> physicalVolumeIds = new ArrayList<String>();
+ for (LUNs lun : luns) {
+ physicalVolumeIds.add(lun.getphysical_volume_id());
+ }
+ return physicalVolumeIds;
+ }
+
public static <E extends VmNetworkInterface> Map<String, E>
vmInterfacesByNetworkName(List<E> entityList) {
if (entityList != null) {
Map<String, E> map = new HashMap<String, E>();
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 c16b744..8b4e79f 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
@@ -247,6 +247,7 @@
ACTION_TYPE_FAILED_STORAGE_VMS_IN_POOL(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST(ErrorType.CONFLICT),
+
ACTION_TYPE_FAILED_IMPORT_STORAGE_DOMAIN_EXTERNAL_LUN_DISK_EXIST(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST(ErrorType.CONFLICT),
ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN(ErrorType.BAD_PARAMETERS),
ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_EXPORT_DOMAIN(ErrorType.BAD_PARAMETERS),
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 27906c2..48f0525 100644
---
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -264,6 +264,7 @@
ACTION_TYPE_FAILED_STORAGE_VMS_IN_POOL=Cannot ${action} ${type}. The following
VMs/Templates are attached to pool: ${vms}.
ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST=Cannot ${action} ${type}.
The Storage Domain name is already in use.
ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST=Cannot ${action} ${type}. The
Storage Domain already exists.
+ACTION_TYPE_FAILED_IMPORT_STORAGE_DOMAIN_EXTERNAL_LUN_DISK_EXIST=Cannot
${action} ${type}. Part of the Storage Domain LUNs are being used as an
External LUN disk. Please remove them first and try again.
ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST=Cannot ${action} ${type}.
The Data Center name is already in use.
ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN=Cannot ${action}
${type}. The selected Storage Domain does not contain the VM Template.
ACTION_TYPE_FAILED_NO_VDS_IN_POOL=Cannot ${action} ${type}. There is no active
Host in the 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 3de70b1..11330d2 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
@@ -718,6 +718,9 @@
@DefaultStringValue("Cannot ${action} ${type}. The Storage Domain already
exists.")
String ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST();
+ @DefaultStringValue("Cannot ${action} ${type}. Part of the Storage Domain
LUNs are being used as an External LUN disk. Please remove them first and try
again.")
+ String ACTION_TYPE_FAILED_IMPORT_STORAGE_DOMAIN_EXTERNAL_LUN_DISK_EXIST();
+
@DefaultStringValue("Cannot ${action} ${type}. The Data Center name is
already in use.")
String ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST();
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 cf91c49..6f779e0 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
@@ -253,6 +253,7 @@
ACTION_TYPE_FAILED_STORAGE_VMS_IN_POOL=Cannot ${action} ${type}. The following
VMs/Templates are attached to pool: ${vms}.
ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST=Cannot ${action} ${type}.
The Storage Domain name is already in use.
ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST=Cannot ${action} ${type}. The
Storage Domain already exists.
+ACTION_TYPE_FAILED_IMPORT_STORAGE_DOMAIN_EXTERNAL_LUN_DISK_EXIST=Cannot
${action} ${type}. Part of the Storage Domain LUNs are being used as an
External LUN disk. Please remove them first and try again.
ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST=Cannot ${action} ${type}.
The Data Center name is already in use.
ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN=Cannot ${action}
${type}. The selected Storage Domain does not contain the VM Template.
ACTION_TYPE_FAILED_NO_VDS_IN_POOL=Cannot ${action} ${type}. There is no active
Host in the 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 0bd1dc6..600a16e 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
@@ -261,6 +261,7 @@
ACTION_TYPE_FAILED_STORAGE_VMS_IN_POOL=Cannot ${action} ${type}. The following
VMs/Templates are attached to pool: ${vms}.
ACTION_TYPE_FAILED_STORAGE_DOMAIN_NAME_ALREADY_EXIST=Cannot ${action} ${type}.
The Storage Domain name is already in use.
ACTION_TYPE_FAILED_STORAGE_DOMAIN_ALREADY_EXIST=Cannot ${action} ${type}. The
Storage Domain already exists.
+ACTION_TYPE_FAILED_IMPORT_STORAGE_DOMAIN_EXTERNAL_LUN_DISK_EXIST=Cannot
${action} ${type}. Part of the Storage Domain LUNs are being used as an
External LUN disk. Please remove them first and try again.
ACTION_TYPE_FAILED_STORAGE_POOL_NAME_ALREADY_EXIST=Cannot ${action} ${type}.
The Data Center name is already in use.
ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_DESTINATION_DOMAIN=Cannot ${action}
${type}. The selected Storage Domain does not contain the VM Template.
ACTION_TYPE_FAILED_NO_VDS_IN_POOL=Cannot ${action} ${type}. There is no active
Host in the Data Center.
--
To view, visit http://gerrit.ovirt.org/36090
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id82e5e1d3df8ce265833efb3fc4b9050ca73b679
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
