Liron Aravot has uploaded a new change for review.

Change subject: core: OVFs that aren't stored on any domain should be stored on 
all
......................................................................

core: OVFs that aren't stored on any domain should be stored on all

When the VM/Template has no non shareable image disks, its OVF file won't be
stored on any domain. This patches changes this behavior by storing
those OVFs on all the domains.

Change-Id: Iaeecc6c4526800656315842384d83830fdf0b72b
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1138134
Signed-off-by: Liron Aravot <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStorageDomainCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStoragePoolCommand.java
M 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java
M 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAODbFacadeImpl.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/VmStaticDAOTest.java
M packaging/dbscripts/vms_sp.sql
7 files changed, 87 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/18/35818/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStorageDomainCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStorageDomainCommand.java
index 57e8825..ffb7fff 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStorageDomainCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStorageDomainCommand.java
@@ -221,6 +221,8 @@
                         false,
                         false);
 
+        
vmAndTemplatesIds.addAll(getVmStaticDAO().getVmAndTemplatesIdsWithoutAttachedImageDisks(false));
+
         byte[] bytes = buildOvfInfoFileByteArray(vmAndTemplatesIds);
 
         Pair<StorageDomainOvfInfo, DiskImage> lastOvfStoreForUpdate = 
domainOvfStoresInfoForUpdate.getLast();
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStoragePoolCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStoragePoolCommand.java
index 4770c88..d22ca69 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStoragePoolCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessOvfUpdateForStoragePoolCommand.java
@@ -47,6 +47,7 @@
     private HashSet<Guid> proccessedDomains;
     private List<Guid> removedOvfIdsInfo;
     private OvfUpdateProcessHelper ovfUpdateProcessHelper;
+    private List<Guid> activeDataDomainsIds;
 
     public ProcessOvfUpdateForStoragePoolCommand(T parameters) {
         this(parameters, null);
@@ -56,6 +57,7 @@
         super(parameters, commandContext);
         setStoragePoolId(parameters.getStoragePoolId());
         ovfUpdateProcessHelper = new OvfUpdateProcessHelper();
+        activeDataDomainsIds = new LinkedList<>();
     }
 
     protected OvfUpdateProcessHelper getOvfUpdateProcessHelper() {
@@ -123,6 +125,7 @@
                 continue;
             }
 
+            activeDataDomainsIds.add(domain.getId());
             Integer ovfStoresCountForDomain = Config.<Integer> 
getValue(ConfigValues.StorageDomainOvfStoreCount);
             List<StorageDomainOvfInfo> storageDomainOvfInfos = 
getStorageDomainOvfInfoDAO().getAllForDomain(domain.getId());
 
@@ -339,6 +342,11 @@
     }
 
     protected void proccessDisksDomains(List<DiskImage> disks) {
+        if (disks.isEmpty()) {
+            proccessedDomains.addAll(activeDataDomainsIds);
+            return;
+        }
+
         for (DiskImage disk : disks) {
             proccessedDomains.addAll(disk.getStorageIds());
         }
diff --git 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java
 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java
index b2726ed..8dbab0d 100644
--- 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java
+++ 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAO.java
@@ -94,6 +94,12 @@
     public void remove(Guid id, boolean removePermissions);
 
     /**
+     * Retrieves all the ids of the vms and templates that have no attached 
disks matching the provided criteria.
+     * @param shareableDisks  check for attached shareable disks
+     */
+    public List<Guid> getVmAndTemplatesIdsWithoutAttachedImageDisks(boolean 
shareableDisks);
+
+    /**
      * update vm_static.cpu_profile_id for cluster
      *
      * @param clusterId
diff --git 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAODbFacadeImpl.java
 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAODbFacadeImpl.java
index 7f0492e..c4677ca 100644
--- 
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAODbFacadeImpl.java
+++ 
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAODbFacadeImpl.java
@@ -57,6 +57,14 @@
                         .addValue("remove_permissions", removePermissions));
     }
 
+
+    public List<Guid> getVmAndTemplatesIdsWithoutAttachedImageDisks(boolean 
shareableDisks) {
+        MapSqlParameterSource parameterSource = 
getCustomMapSqlParameterSource()
+                .addValue("shareable", shareableDisks);
+        return 
getCallsHandler().executeReadList("GetVmsAndTemplatesIdsWithoutAttachedImageDisks",
+                createGuidMapper(), parameterSource);
+    }
+
     @Override
     protected MapSqlParameterSource createIdParameterMapper(Guid id) {
         return getCustomMapSqlParameterSource().addValue("vm_guid", id);
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 2c6b50a..d882209 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
@@ -182,7 +182,7 @@
     public static final String VM_RHEL5_POOL_50_NAME = "rhel5-pool-50";
 
     /**
-     * Predefined VM for testing with the following properties :
+     * Predefined VM with no attached disks for testing with the following 
properties :
      * <ul>
      * <li>VM name: rhel5-pool-51</li>
      * <li>Vds group: rhel6.iscsi (b399944a-81ab-4ec5-8266-e19ba7c3c9d1)</li>
@@ -240,7 +240,7 @@
     public static final Guid VM_TEMPLATE_RHEL5 = new 
Guid("1b85420c-b84c-4f29-997e-0eb674b40b79");
 
     /**
-     * Predefined template for testing with the following properties :
+     * Predefined template with no attached disks for testing with the 
following properties :
      * <ul>
      * <li>Vds group: rhel6.iscsi (b399944a-81ab-4ec5-8266-e19ba7c3c9d1)</li>
      * </ul>
@@ -285,6 +285,11 @@
     public static final Guid UNREGISTERED_VM = new 
Guid("77296e00-0cad-4e5a-9299-008a7b6f4359");
 
     /**
+     * Predefined VM with no attached disks
+     */
+    public static final Guid VM_WITH_NO_ATTACHED_DISKS = new 
Guid("77296e00-0cad-4e5a-9299-008a7b6f4357");
+
+    /**
      * Predefined vm job for testing with the following properties :
      * <ul>
      * <li>vm_id: VM_RHEL5_POOL_57 (77296e00-0cad-4e5a-9299-008a7b6f4355)</li>
diff --git 
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmStaticDAOTest.java
 
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmStaticDAOTest.java
index 933119d..9f02c5b 100644
--- 
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmStaticDAOTest.java
+++ 
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/VmStaticDAOTest.java
@@ -17,9 +17,13 @@
 import org.ovirt.engine.core.common.businessentities.MigrationSupport;
 import org.ovirt.engine.core.common.businessentities.OriginType;
 import org.ovirt.engine.core.common.businessentities.Snapshot;
+import org.ovirt.engine.core.common.businessentities.VmDevice;
+import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType;
+import org.ovirt.engine.core.common.businessentities.VmDeviceId;
 import org.ovirt.engine.core.common.businessentities.VmStatic;
 import org.ovirt.engine.core.common.businessentities.VmTemplate;
 import org.ovirt.engine.core.common.utils.ObjectUtils;
+import org.ovirt.engine.core.common.utils.VmDeviceType;
 import org.ovirt.engine.core.compat.Guid;
 
 /**
@@ -204,6 +208,38 @@
         assertEquals("vm permissions changed during remove although shouldnt 
have.", numberOfPermissionsBeforeRemove, 
permissionsDao.getAllForEntity(EXISTING_VM_ID).size());
     }
 
+
+    @Test
+    public void getVmAndTemplatesIdsWithoutAttachedImageDi×›sks() {
+        // attaching shareable and snapshots disk to a diskless vm
+        addVmDevice(FixturesTool.VM_WITH_NO_ATTACHED_DISKS, 
FixturesTool.IMAGE_GROUP_ID_2, null);
+        addVmDevice(FixturesTool.VM_WITH_NO_ATTACHED_DISKS, 
FixturesTool.DISK_ID, FixturesTool.EXISTING_SNAPSHOT_ID);
+
+        List<Guid> ids =
+                dao.getVmAndTemplatesIdsWithoutAttachedImageDisks(false);
+        assertTrue(ids.contains(FixturesTool.VM_RHEL5_POOL_51));
+        assertTrue(ids.contains(FixturesTool.VM_TEMPLATE_RHEL5_2));
+        assertTrue(ids.contains(FixturesTool.VM_WITH_NO_ATTACHED_DISKS));
+        assertFalse(ids.contains(FixturesTool.VM_TEMPLATE_RHEL5));
+    }
+
+    private void addVmDevice(Guid vmId, Guid diskId, Guid snapshotId) {
+        VmDevice device = new VmDevice(new VmDeviceId(diskId, vmId),
+                VmDeviceGeneralType.DISK,
+                VmDeviceType.DISK.getName(),
+                "",
+                0,
+                null,
+                true,
+                false,
+                false,
+                "",
+                null,
+                snapshotId,
+                null);
+        dbFacade.getVmDeviceDao().save(device);
+    }
+
     @Test
     public void testGetAllNamesPinnedToHostReturnsNothingForRandomHost() 
throws Exception {
         assertTrue(dao.getAllNamesPinnedToHost(Guid.newGuid()).isEmpty());
diff --git a/packaging/dbscripts/vms_sp.sql b/packaging/dbscripts/vms_sp.sql
index 83dc5b5..163df69 100644
--- a/packaging/dbscripts/vms_sp.sql
+++ b/packaging/dbscripts/vms_sp.sql
@@ -586,6 +586,26 @@
 
 
 
+Create or replace FUNCTION 
GetVmsAndTemplatesIdsWithoutAttachedImageDisks(v_shareable BOOLEAN)
+RETURNS SETOF UUID STABLE
+   AS $procedure$
+BEGIN
+      RETURN QUERY SELECT vs.vm_guid
+      FROM vm_static vs
+      WHERE vs.vm_guid NOT IN (SELECT DISTINCT vd.vm_id
+                               FROM vm_device vd
+                               INNER JOIN base_disks i
+                               ON i.disk_id = vd.device_id
+                               AND vd.snapshot_id IS NULL
+                               WHERE i.shareable = v_shareable);
+END; $procedure$
+LANGUAGE plpgsql;
+
+
+
+
+
+
 Create or replace FUNCTION UpdateVmStatic(v_description VARCHAR(4000) ,
  v_free_text_comment text,
  v_mem_size_mb INTEGER,


-- 
To view, visit http://gerrit.ovirt.org/35818
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaeecc6c4526800656315842384d83830fdf0b72b
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Liron Aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to