rp- commented on code in PR #10280:
URL: https://github.com/apache/cloudstack/pull/10280#discussion_r1931611829


##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -475,16 +486,39 @@ public boolean deletePhysicalDisk(String name, 
KVMStoragePool pool, Storage.Imag
     {
         s_logger.debug("Linstor: deletePhysicalDisk " + name);
         final DevelopersApi api = getLinstorAPI(pool);
-
+        final String rscName = getLinstorRscName(name);
+        final LinstorStoragePool linstorPool = (LinstorStoragePool) pool;
+        String rscGrpName = linstorPool.getResourceGroup();
+        boolean deleted = false;
         try {
-            final String rscName = getLinstorRscName(name);
-            s_logger.debug("Linstor: delete resource definition " + rscName);
-            ApiCallRcList answers = api.resourceDefinitionDelete(rscName);
-            handleLinstorApiAnswers(answers, "Linstor: Unable to delete 
resource definition " + rscName);
+            List<ResourceDefinition> existingRDs = 
LinstorUtil.getRDListStartingWith(api, rscName);
+
+            for (ResourceDefinition rd : existingRDs) {
+                int expectedProps = 0; // if it is a non template resource, we 
don't expect any _cs-template-for- prop
+                String propKey = 
LinstorUtil.getTemplateForAuxPropKey(rscGrpName);
+                if (rd.getProps().containsKey(propKey)) {
+                    ResourceDefinitionModify rdm = new 
ResourceDefinitionModify();
+                    rdm.deleteProps(Collections.singletonList(propKey));
+                    api.resourceDefinitionModify(rd.getName(), rdm);
+                    expectedProps = 1;
+                }
+
+                // if there is only one template-for property left for 
templates, the template isn't needed anymore
+                // or if it isn't a template anyway, it will not have this Aux 
property
+                // _cs-template-for- poperties work like a ref-count.
+                if (rd.getProps().keySet().stream()
+                        .filter(key -> key.startsWith("Aux/" + 
LinstorUtil.CS_TEMPLATE_FOR_PREFIX))
+                        .count() == expectedProps) {
+                    ApiCallRcList answers = 
api.resourceDefinitionDelete(rd.getName());
+                    checkLinstorAnswersThrow(answers);
+                    deleted = true;
+                }
+            }

Review Comment:
   done



##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -569,17 +628,29 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk 
disk, String name, KVMSt
 
         final KVMPhysicalDisk dstDisk = destPools.createPhysicalDisk(
             name, QemuImg.PhysicalDiskFormat.RAW, provisioningType, 
disk.getVirtualSize(), null);
+        final String rscName = getLinstorRscName(name);
 
         final DevelopersApi api = getLinstorAPI(destPools);
-        applyAuxProps(api, name, disk.getDispName(), disk.getVmName());
+        // if it is the initial systemvm disk copy, we need to apply the 
_cs-template-for property.
+        if (isSystemTemplate(disk)) {
+            applyAuxProps(api, name, "SystemVM Template", null);
+            LinstorStoragePool linPool = (LinstorStoragePool) destPools;
+            try {
+                LinstorUtil.setAuxTemplateForProperty(api, rscName, 
linPool.getResourceGroup());
+            } catch (ApiException apiExc) {
+                s_logger.error(String.format("Error setting aux template for 
property for %s", rscName));
+                logLinstorAnswers(apiExc.getApiCallRcList());
+            }
+        } else {
+            applyAuxProps(api, name, disk.getDispName(), disk.getVmName());
+        }

Review Comment:
   done



##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -393,22 +398,76 @@ private String getRscGrp(StoragePoolVO storagePoolVO) {
             storagePoolVO.getUserInfo() : "DfltRscGrp";
     }
 
-    private String createResourceBase(
-        String rscName, long sizeInBytes, String volName, String vmName, 
DevelopersApi api, String rscGrp) {
-        ResourceGroupSpawn rscGrpSpawn = new ResourceGroupSpawn();
-        rscGrpSpawn.setResourceDefinitionName(rscName);
-        rscGrpSpawn.addVolumeSizesItem(sizeInBytes / 1024);
-
+    /**
+     * Creates a new Linstor resource.
+     * @param rscName
+     * @param sizeInBytes
+     * @param volName
+     * @param vmName
+     * @param api
+     * @param rscGrp
+     * @param poolId
+     * @param isTemplate indicates if the resource is a template
+     * @return true if a new resource was created, false if it already existed 
or was reused.
+     */
+    private boolean createResourceBase(
+        String rscName, long sizeInBytes, String volName, String vmName, 
DevelopersApi api,
+        String rscGrp, long poolId, boolean isTemplate)
+    {
         try
         {
-            s_logger.info("Linstor: Spawn resource " + rscName);
-            ApiCallRcList answers = api.resourceGroupSpawn(rscGrp, 
rscGrpSpawn);
-            checkLinstorAnswersThrow(answers);
-
-            answers = LinstorUtil.applyAuxProps(api, rscName, volName, vmName);
-            checkLinstorAnswersThrow(answers);
+            s_logger.debug(String.format("createRscBae: %s :: %s :: %b", 
rscName, rscGrp, isTemplate));
+            List<Pair<ResourceDefinition, ResourceGroup>> existingRDs = 
LinstorUtil.getRDAndRGListStartingWith(api, rscName);
+
+            String fullRscName = String.format("%s-%d", rscName, poolId);
+            boolean alreadyCreated = existingRDs.stream()
+                    .anyMatch(p -> 
p.first().getName().equalsIgnoreCase(fullRscName)) ||
+                    existingRDs.stream().anyMatch(p -> 
p.first().getProps().containsKey(LinstorUtil.getTemplateForAuxPropKey(rscGrp)));
+            if (!alreadyCreated) {
+                boolean createNewRsc = true;
+                if (!existingRDs.isEmpty()) {
+                    ResourceGroup tgtRscGrp = api.resourceGroupList(
+                            Collections.singletonList(rscGrp), null, null, 
null).get(0);
+                    List<String> tgtLayerStack = tgtRscGrp.getSelectFilter() 
!= null ?
+                            tgtRscGrp.getSelectFilter().getLayerStack() : null;
+
+                    // check if there is already a template copy, that we 
could reuse
+                    // this means if select filters are similar enough to 
allow cloning from
+                    for (Pair<ResourceDefinition, ResourceGroup> rdPair : 
existingRDs) {
+                        ResourceGroup rg = rdPair.second();
+                        List<String> rgLayerStack = rg.getSelectFilter() != 
null ?
+                                rg.getSelectFilter().getLayerStack() : null;
+                        if (Objects.equals(tgtLayerStack, rgLayerStack) &&
+                                
Objects.equals(tgtRscGrp.getSelectFilter().getStoragePoolList(),
+                                        
rg.getSelectFilter().getStoragePoolList())) {
+                            LinstorUtil.setAuxTemplateForProperty(api, 
rscName, rscGrp);
+                            createNewRsc = false;
+                            break;
+                        }
+                    }
+                }
 
-            return LinstorUtil.getDevicePath(api, rscName);
+                if (createNewRsc) {
+                    String newRscName = existingRDs.isEmpty() ? rscName : 
fullRscName;
+                    ResourceGroupSpawn rscGrpSpawn = new ResourceGroupSpawn();
+                    rscGrpSpawn.setResourceDefinitionName(newRscName);
+                    rscGrpSpawn.addVolumeSizesItem(sizeInBytes / 1024);
+                    if (isTemplate) {
+                        Properties props = new Properties();
+                        
props.put(LinstorUtil.getTemplateForAuxPropKey(rscGrp), "true");
+                        rscGrpSpawn.setResourceDefinitionProps(props);
+                    }
+
+                    s_logger.info("Linstor: Spawn resource " + newRscName);
+                    ApiCallRcList answers = api.resourceGroupSpawn(rscGrp, 
rscGrpSpawn);
+                    checkLinstorAnswersThrow(answers);
+
+                    answers = LinstorUtil.applyAuxProps(api, newRscName, 
volName, vmName);
+                    checkLinstorAnswersThrow(answers);
+                }
+                return createNewRsc;
+            }
+            return false;

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to