Tal Nisan has uploaded a new change for review. Change subject: core: Changed storage type checks in commands ......................................................................
core: Changed storage type checks in commands As a part of the effort to remove the type of storage pool the following changes are introduced in this patch: - Removed unused methods in StorageDomainStaticDao concerning pool types and their tests - Replaced StoragerHelper getter in ConnectAllHostsToLun command from the storage pool type to static ISCSI type as this is the only option for this command - Changed connection to new master in RecoveryStoragePool command to use the storage helper of the type of new master, until now the old master type was used since it didn't make any difference since the types of the old and new were always the same Relates-To: https://bugzilla.redhat.com/1038053 Change-Id: Iab29347dd82ea47bf6e0d1d5542db9d6d52c0b08 Signed-off-by: Tal Nisan <tni...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectAllHostsToLunCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAODbFacadeImpl.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StorageDomainStaticDAOTest.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java 8 files changed, 3 insertions(+), 129 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/95/23295/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectAllHostsToLunCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectAllHostsToLunCommand.java index bdabf10..131eef2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectAllHostsToLunCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectAllHostsToLunCommand.java @@ -10,6 +10,7 @@ import org.ovirt.engine.core.common.action.ExtendSANStorageDomainParameters; import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.businessentities.LUNs; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VdsSpmStatus; import org.ovirt.engine.core.common.errors.VdcBLLException; @@ -100,7 +101,7 @@ for (Map.Entry<String, List<Guid>> entry : processed.entrySet()) { for (Guid vdsId : entry.getValue()) { LUNs lun = lunsMap.get(entry.getKey()); - StorageHelperDirector.getInstance().getItem(getStoragePool().getStorageType()) + StorageHelperDirector.getInstance().getItem(StorageType.ISCSI) .disconnectStorageFromLunByVdsId(getStorageDomain(), vdsId, lun); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java index 0cdb54a..5f976e7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java @@ -68,9 +68,6 @@ if (domain.getStorageDomainSharedStatus() != StorageDomainSharedStatus.Unattached) { addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL); returnValue = false; - } else if (domain.getStorageType() != getStoragePool().getStorageType()) { - addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_RECOVERY_STORAGE_POOL_STORAGE_TYPE_MISSMATCH); - returnValue = false; } } } @@ -90,7 +87,7 @@ @Override protected void executeCommand() { - if (StorageHelperDirector.getInstance().getItem(getStorageDomain().getStorageType()) + if (StorageHelperDirector.getInstance().getItem(getNewMaster(false).getStorageType()) .connectStorageToDomainByVdsId(getNewMaster(false), getVds().getId())) { ((EventQueue) EjbUtils.findBean(BeanType.EVENTQUEUE_MANAGER, BeanProxyType.LOCAL)).submitEventSync(new Event(getParameters().getStoragePoolId(), diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAO.java index 116389b..3e39135 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAO.java @@ -3,7 +3,6 @@ import java.util.List; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; -import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.compat.Guid; @@ -19,18 +18,6 @@ StorageDomainStatic getByName(String name); /** - * Retrieves all domains of the specified type for the specified pool. - * - * @param type - * the domain type - * @param pool - * the storage pool - * @return the list of domains - */ - List<StorageDomainStatic> getAllForStoragePoolOfStorageType( - StorageType type, Guid pool); - - /** * Retrieves all storage domains for the given storage pool. * * @param pool @@ -38,15 +25,6 @@ * @return the list of domains */ List<StorageDomainStatic> getAllForStoragePool(Guid pool); - - /** - * Retrieves all domains for the given type. - * - * @param type - * the domain type - * @return the list of domains - */ - List<StorageDomainStatic> getAllOfStorageType(StorageType type); /** * Return all the domains of the given status which belong to the given pool. diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAODbFacadeImpl.java index 7fc8064..2cc19af 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainStaticDAODbFacadeImpl.java @@ -30,24 +30,6 @@ } @Override - public List<StorageDomainStatic> getAllOfStorageType( - StorageType type) { - return getCallsHandler().executeReadList("Getstorage_domain_staticBystorage_pool_type", - StorageDomainStaticRowMapper.instance, - getCustomMapSqlParameterSource() - .addValue("storage_pool_type", type)); - } - - @Override - public List<StorageDomainStatic> getAllForStoragePoolOfStorageType( - StorageType type, Guid pool) { - return getCallsHandler().executeReadList("Getstorage_domain_staticBystorage_type_and_storage_pool_id", - StorageDomainStaticRowMapper.instance, - getStoragePoolIdParameterSource(pool) - .addValue("storage_type", type)); - } - - @Override public List<StorageDomainStatic> getAllForStoragePool(Guid id) { return getCallsHandler().executeReadList("Getstorage_domain_staticBystorage_pool_id", StorageDomainStaticRowMapper.instance, diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java index 0f7c92f..9afeb49 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAO.java @@ -4,7 +4,6 @@ import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; -import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.compat.Guid; @@ -89,15 +88,6 @@ * @return The list of storage pools with the given status */ List<StoragePool> getAllByStatus(StoragePoolStatus status); - - /** - * Retrieves the list of all storage pools of a given type. - * - * @param type - * the storage pool type - * @return the list of storage pool - */ - List<StoragePool> getAllOfType(StorageType type); /** * Retrieves all storage pools for the given storage domain. diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java index 281506e..d567acd 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StoragePoolDAODbFacadeImpl.java @@ -115,13 +115,6 @@ } @Override - public List<StoragePool> getAllOfType(StorageType type) { - MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() - .addValue("storage_pool_type", type); - return getCallsHandler().executeReadList("Getstorage_poolsByType", mapper, parameterSource); - } - - @Override public List<StoragePool> getAllForStorageDomain(Guid id) { MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() .addValue("storage_domain_id", id); diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StorageDomainStaticDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StorageDomainStaticDAOTest.java index b6add69..58d370d 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StorageDomainStaticDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StorageDomainStaticDAOTest.java @@ -13,7 +13,6 @@ import org.junit.Test; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; -import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.compat.Guid; @@ -117,46 +116,6 @@ assertNotNull(result); assertFalse(result.isEmpty()); - } - - /** - * Ensures the right set is returned. - */ - @Test - public void testGetAllForStoragePoolOfStorageType() { - List<StorageDomainStatic> result = dao.getAllForStoragePoolOfStorageType(StorageType.ISCSI, EXISTING_POOL_ID); - - assertNotNull(result); - assertFalse(result.isEmpty()); - for (StorageDomainStatic domain : result) { - assertEquals(StorageType.ISCSI, domain.getStorageType()); - } - } - - /** - * Ensures that an empty collection is returned when no static domains of the specified type exist. - */ - @Test - public void testGetAllOfStorageTypeWithInvalidType() { - List<StorageDomainStatic> result = dao.getAllOfStorageType(StorageType.FCP); - - assertNotNull(result); - assertTrue(result.isEmpty()); - } - - /** - * Ensures the right collection of domains is returned. - */ - @Test - public void testGetAllOfStorageType() { - List<StorageDomainStatic> result = dao - .getAllOfStorageType(StorageType.ISCSI); - - assertNotNull(result); - assertFalse(result.isEmpty()); - for (StorageDomainStatic domain : result) { - assertEquals(StorageType.ISCSI, domain.getStorageType()); - } } @Test diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java index e4d713e..6d4f43c 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java @@ -12,7 +12,6 @@ import org.ovirt.engine.core.common.businessentities.QuotaEnforcementTypeEnum; import org.ovirt.engine.core.common.businessentities.StorageFormatType; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; -import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; @@ -199,31 +198,6 @@ assertNotNull(result); assertFalse(result.isEmpty()); - } - - /** - * Ensures that an empty collection is returned for a type that's not in the database. - */ - @Test - public void testGetAllOfTypeForUnrepresentedType() { - List<StoragePool> result = dao.getAllOfType(StorageType.UNKNOWN); - - assertNotNull(result); - assertTrue(result.isEmpty()); - } - - /** - * Ensures that only pools of the given type are returned. - */ - @Test - public void testGetAllOfType() { - List<StoragePool> result = dao.getAllOfType(StorageType.ISCSI); - - assertNotNull(result); - assertFalse(result.isEmpty()); - for (StoragePool pool : result) { - assertEquals(StorageType.ISCSI, pool.getStorageType()); - } } /** -- To view, visit http://gerrit.ovirt.org/23295 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iab29347dd82ea47bf6e0d1d5542db9d6d52c0b08 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tal Nisan <tni...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
