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 <[email protected]> --- 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 4 files changed, 25 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/39/24039/1 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 e69950e..e03ffbc 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 @@ -59,6 +59,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 5dfcf91..c88513d 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 @@ -151,7 +151,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()); } /** @@ -260,14 +260,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() { + 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 78dd5cb..3ae4eb1 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 81fb41c..2257595 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 AND i.active = TRUE 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/24039 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I89d119295cf16eb1ea2df365fb22cadde6595e6a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
