Allon Mureinik has uploaded a new change for review. Change subject: core: Disabling an SD with unplugged disk of a VM ......................................................................
core: Disabling an SD with unplugged disk of a VM Prior to this patch, when disabling a Storage Domain the canDoAction() would check that it does not contain any disks in that belong to running VM, and if it does, the command is failed. This behavior is overly strict, and we should be able to disable a domain which contains unplugged disks, even if they are attached to a running VM. This patch contains: 1. Changing the GetVmsByStorageDomainId stored procedure to take only plugged disks in to account. 2. In fixtures.xml, plugging disk 1b26a52b-b60f-44cb-9f46-3ef333b04a35 into VM 77296e00-0cad-4e5a-9299-008a7b6f4355, as opposed to just attaching it in order for this fix to be tested. 3. Updating the DAO tests accordingly. Change-Id: I89d119295cf16eb1ea2df365fb22cadde6595e6a Bug-Url: https://bugzilla.redhat.com/969770 Signed-off-by: Allon Mureinik <amure...@redhat.com> --- M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java M backend/manager/modules/dal/src/test/resources/fixtures.xml M packaging/dbscripts/vms_sp.sql 6 files changed, 31 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/03/24803/1 diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java index 7c3a3e3..d2e110c 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java @@ -167,10 +167,12 @@ List<VM> getAllForStorageDomain(Guid storageDomain); /** - * Retrieves all running VMs for the given storage domain. + * Retrieves all running VMs that require the given storage domain to be active. + * In other words, this method returns a list of VMs that have plugged disks that reside + * on the given storage domain. * * @param storageDomain - * the storage domain + * the storage domain's ID * @return the running VMs */ List<VM> getAllActiveForStorageDomain(Guid storageDomain); diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java index bebb77a..71936e6 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java @@ -199,7 +199,7 @@ * The result to check */ private static void assertPluggedGetAllForVMResult(List<Disk> disks) { - Integer numberOfDisks = 2; + Integer numberOfDisks = 5; assertEquals("VM should have " + numberOfDisks + " plugged disk", numberOfDisks.intValue(), disks.size()); } diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java index 287b8eb..3472266 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/FixturesTool.java @@ -54,6 +54,11 @@ protected static final Guid STORAGE_DOAMIN_SCALE_SD5 = new Guid("72e3a666-89e1-4005-a7ca-f7548004a9ab"); /** + * Predefined scale storage domain. + */ + protected static final Guid STORAGE_DOAMIN_SCALE_SD6 = new Guid("72e3a666-89e1-4005-a7ca-f7548004a9ac"); + + /** * Predefined vds group. */ protected static final Guid VDS_GROUP_RHEL6_ISCSI = new Guid("b399944a-81ab-4ec5-8266-e19ba7c3c9d1"); diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java index 78a97b3..1aeb614 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmDAOTest.java @@ -160,7 +160,7 @@ Map<Boolean, List<VM>> result = dao.getForDisk(FixturesTool.IMAGE_GROUP_ID, true); assertNotNull(result); - assertEquals("wrong number of VMs with unplugged image", 1, result.get(false).size()); + assertEquals("wrong number of VMs with unplugged image", 1, result.get(Boolean.TRUE).size()); } /** @@ -269,14 +269,26 @@ } /** - * Ensures that getting all VMs for a storage domain works as expected. + * Ensures that getting all VMs for a storage domain works as expected for a domain without VMs. */ @Test - public void testGetAllForStorageDomain() { - List<VM> result = dao.getAllForStorageDomain(STORAGE_DOMAIN_ID); + public void testGetAllForStorageDomainWithVms() { + List<VM> result = dao.getAllForStorageDomain(FixturesTool.STORAGE_DOAMIN_SCALE_SD5); assertNotNull(result); - assertFalse(result.isEmpty()); + assertEquals(1, result.size()); + assertEquals(FixturesTool.VM_RHEL5_POOL_57, result.get(0).getId()); + } + + /** + * Ensures that getting all VMs for a storage domain works as expected for a domain without VMs. + */ + @Test + public void testGetAllForStorageDomainWithoutVMs() { + List<VM> result = dao.getAllForStorageDomain(FixturesTool.STORAGE_DOAMIN_SCALE_SD6); + + assertNotNull(result); + assertTrue(result.isEmpty()); } /** diff --git a/backend/manager/modules/dal/src/test/resources/fixtures.xml b/backend/manager/modules/dal/src/test/resources/fixtures.xml index 0988b47..4956c5f 100644 --- a/backend/manager/modules/dal/src/test/resources/fixtures.xml +++ b/backend/manager/modules/dal/src/test/resources/fixtures.xml @@ -5054,7 +5054,7 @@ <value>1</value> <null/> <value>true</value> - <value>false</value> + <value>true</value> <value>false</value> <value>alias</value> <value>{ "prop1" : "true", "prop2" : "123456" }</value> diff --git a/packaging/dbscripts/vms_sp.sql b/packaging/dbscripts/vms_sp.sql index 0b6f077..84648cc 100644 --- a/packaging/dbscripts/vms_sp.sql +++ b/packaging/dbscripts/vms_sp.sql @@ -1012,7 +1012,9 @@ INNER JOIN vm_device vd ON vd.vm_id = vms.vm_guid INNER JOIN images i ON i.image_group_id = vd.device_id inner join image_storage_domain_map on i.image_guid = image_storage_domain_map.image_id - WHERE status <> 0 and image_storage_domain_map.storage_domain_id = v_storage_domain_id; + WHERE status <> 0 AND + is_plugged = TRUE AND + image_storage_domain_map.storage_domain_id = v_storage_domain_id; END; $procedure$ LANGUAGE plpgsql; -- To view, visit http://gerrit.ovirt.org/24803 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I89d119295cf16eb1ea2df365fb22cadde6595e6a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Allon Mureinik <amure...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
