Liron Aravot has uploaded a new change for review. Change subject: core: WIP: issues when reusing LUNs ......................................................................
core: WIP: issues when reusing LUNs This patch fixes the following issues 1. When removing a domain that used lun storage and there's a disk used by that lun - LUNs volume_group_id still contains the removed domain id which doesn't exist anymore. 2. When extending a domain to use an existing lun, the domain id isn't set to that lun. 3. When creating a domain from an existing lun, the domain id isn't set to this lun. 4. When removing lun disk which isn't used as storage domain (and therefore - not used anymore), the lun remained in the DB. those issues caused that a created vg existed on vdsm side while not in db which caused to an NPE during execution of GetLunsByVgIdQuery. Change-Id: I410974389bcab24dd42fedb03be7dde5ffbd99e0 Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=875909 Signed-off-by: Liron Aravot <lara...@redhat.com> --- M backend/manager/dbscripts/storages_san_sp.sql M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddSANStorageDomainCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ExtendSANStorageDomainCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAO.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAODbFacadeImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/LunDAOTest.java 8 files changed, 48 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/28/9228/1 diff --git a/backend/manager/dbscripts/storages_san_sp.sql b/backend/manager/dbscripts/storages_san_sp.sql index dad6c9a..aeea92a 100644 --- a/backend/manager/dbscripts/storages_san_sp.sql +++ b/backend/manager/dbscripts/storages_san_sp.sql @@ -23,6 +23,19 @@ +Create or replace FUNCTION UpdateLUNsVolumeGroupId(v_LUN_id VARCHAR(50), +v_volume_group_id VARCHAR(50)) +RETURNS VOID + AS $procedure$ +BEGIN +UPDATE LUNs set volume_group_id = v_volume_group_id where LUN_id = v_LUN_ID; +END; $procedure$ +LANGUAGE plpgsql; + + + + + Create or replace FUNCTION DeleteLUN(v_LUN_id VARCHAR(50)) RETURNS VOID diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddSANStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddSANStorageDomainCommand.java index 59ed51e..41245b4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddSANStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddSANStorageDomainCommand.java @@ -65,7 +65,7 @@ @Override public Void runInTransaction() { for (LUNs lun : luns) { - proceedLUNInDb(lun, getStorageDomain().getstorage_type()); + proceedLUNInDb(lun, getStorageDomain().getstorage_type(), getStorageDomain().getstorage()); } return null; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ExtendSANStorageDomainCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ExtendSANStorageDomainCommand.java index cd36ee8..4bf6b1e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ExtendSANStorageDomainCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ExtendSANStorageDomainCommand.java @@ -25,7 +25,7 @@ protected void executeCommand() { setStorageDomainStatus(StorageDomainStatus.Locked, null); for (LUNs lun : getParameters().getLunsList()) { - proceedLUNInDb(lun, getStorageDomain().getstorage_type()); + proceedLUNInDb(lun, getStorageDomain().getstorage_type(), getStorageDomain().getstorage()); } if (Backend .getInstance() diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java index 59057d1..17815f7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ISCSIStorageHelper.java @@ -196,6 +196,8 @@ if (DbFacade.getInstance().getDiskLunMapDao().getDiskIdByLunId(lun.getLUN_id()) == null) { DbFacade.getInstance().getLunDao().remove(lun.getLUN_id()); numOfRemovedLuns++; + } else { + DbFacade.getInstance().getLunDao().updateLUNsVolumeGroupId(lun.getLUN_id(), ""); } } if (numOfRemovedLuns > 0) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java index b03719b..d984468 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java @@ -227,9 +227,16 @@ } public static void proceedLUNInDb(final LUNs lun, StorageType storageType) { + proceedLUNInDb(lun,storageType,""); + } + + public static void proceedLUNInDb(final LUNs lun, StorageType storageType, String volumeGroupId) { if (DbFacade.getInstance().getLunDao().get(lun.getLUN_id()) == null) { DbFacade.getInstance().getLunDao().save(lun); + } else if (!volumeGroupId.isEmpty()){ + DbFacade.getInstance().getLunDao().updateLUNsVolumeGroupId(lun.getLUN_id(), volumeGroupId); } + for (storage_server_connections connection : lun.getLunConnections()) { List<storage_server_connections> connections = DbFacade.getInstance() .getStorageServerConnectionDao().getAllForConnection(connection); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAO.java index 938b680..45b12ba 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAO.java @@ -45,6 +45,11 @@ List<LUNs> getAll(); /** + * Updates the lun volume group id + */ + void updateLUNsVolumeGroupId(String id, String volumeGroupId); + + /** * Saves the supplied LUN instance. * * @param lun diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAODbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAODbFacadeImpl.java index ebab1c1..daa6820 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAODbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/LunDAODbFacadeImpl.java @@ -46,6 +46,14 @@ return getCallsHandler().executeRead("GetLUNByLUNId", MAPPER, parameterSource); } + @Override + public void updateLUNsVolumeGroupId(String id, String volumeGroupId) { + MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource().addValue("LUN_id", id) + .addValue("volume_group_id", volumeGroupId); + + getCallsHandler().executeModification("UpdateLUNsVolumeGroupId", parameterSource); + } + @SuppressWarnings("unchecked") @Override public List<LUNs> getAllForStorageServerConnection(String id) { @@ -55,7 +63,6 @@ return getCallsHandler().executeReadList("GetLUNsBystorage_server_connection", MAPPER, parameterSource); } - @SuppressWarnings("unchecked") @Override public List<LUNs> getAllForVolumeGroup(String id) { MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/LunDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/LunDAOTest.java index 2c4436d..71c4c69 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/LunDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/LunDAOTest.java @@ -70,6 +70,17 @@ } /** + * Ensures that LUNs are returned for the connection. + */ + @Test + public void testUpdateLUNsVolumeGroupId() { + String testGroupId = "testvolgroupId"; + dao.updateLUNsVolumeGroupId(existingLUN.getLUN_id(), testGroupId); + LUNs dbLun = dao.get(existingLUN.getLUN_id()); + assertEquals("LUNs volume group id wasn't updated", testGroupId, dbLun.getvolume_group_id()); + } + + /** * Ensures that an empty collection is returned. */ @Test -- To view, visit http://gerrit.ovirt.org/9228 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I410974389bcab24dd42fedb03be7dde5ffbd99e0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
