linstor: Fix using multiple primary storage with same linstor-controller

It should have been always possible to use multiple primary storages,
with the same linstor-controller, by just using different resource-groups
with different settings.
But if the same template was used on 2 different primary storages,
there would be a name collision on the linstor-controller, as 2 of them
would get allocated to the same name.
This commit fixes this, by intelligently reusing the same template,
as long as possible (if select filter properties match enough).

plugins/storage/volume/linstor/CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to Linstor CloudStack plugin will be documented in this file
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2025-01-27]
+
+### Fixed
+
+- Use of multiple primary storages on the same linstor controller
+
 ## [2025-01-20]
 
 ### Fixed

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java

@@ -60,6 +60,11 @@
 import com.linbit.linstor.api.model.VolumeDefinition;
 
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 
 @StorageAdaptorInfo(storagePoolType=Storage.StoragePoolType.Linstor)
 public class LinstorStorageAdaptor implements StorageAdaptor {
@@ -198,10 +203,10 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
         final DevelopersApi api = getLinstorAPI(pool);
 
         try {
-            List<ResourceDefinition> definitionList = api.resourceDefinitionList(
-                Collections.singletonList(rscName), null, null, null);
+            ResourceDefinition resourceDefinition = LinstorUtil.findResourceDefinition(
+                    api, rscName, lpool.getResourceGroup());
 
-            if (definitionList.isEmpty()) {
+            if (resourceDefinition == null) {
                 ResourceGroupSpawn rgSpawn = new ResourceGroupSpawn();
                 rgSpawn.setResourceDefinitionName(rscName);
                 rgSpawn.addVolumeSizesItem(size / 1024); // linstor uses KiB
@@ -211,22 +216,28 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
                 handleLinstorApiAnswers(answers, "Linstor: Unable to spawn resource.");
             }
 
+            String foundRscName = resourceDefinition != null ? resourceDefinition.getName() : rscName;
+
             // query linstor for the device path
             List<ResourceWithVolumes> resources = api.viewResources(
                 Collections.emptyList(),
-                Collections.singletonList(rscName),
+                Collections.singletonList(foundRscName),
                 Collections.emptyList(),
                 null,
                 null,
                 null);
 
-            makeResourceAvailable(api, rscName, false);
+            makeResourceAvailable(api, foundRscName, false);
 
             if (!resources.isEmpty() && !resources.get(0).getVolumes().isEmpty()) {
                 final String devPath = resources.get(0).getVolumes().get(0).getDevicePath();
                 s_logger.info("Linstor: Created drbd device: " + devPath);
                 final KVMPhysicalDisk kvmDisk = new KVMPhysicalDisk(devPath, name, pool);
                 kvmDisk.setFormat(QemuImg.PhysicalDiskFormat.RAW);
+                long allocatedKib = resources.get(0).getVolumes().get(0).getAllocatedSizeKib() != null ?
+                        resources.get(0).getVolumes().get(0).getAllocatedSizeKib() : 0;
+                kvmDisk.setSize(allocatedKib >= 0 ? allocatedKib * 1024 : 0);
+                kvmDisk.setVirtualSize(size);
                 return kvmDisk;
             } else {
                 s_logger.error("Linstor: viewResources didn't return resources or volumes.");
@@ -470,21 +481,56 @@ public boolean disconnectPhysicalDiskByPath(String localPath)
         return false;
     }
 
+    /**
+     * Decrements the aux property key for template resource and deletes or just deletes if not template resource.
+     * @param api
+     * @param rscName
+     * @param rscGrpName
+     * @return
+     * @throws ApiException
+     */
+    private boolean deRefOrDeleteResource(DevelopersApi api, String rscName, String rscGrpName) throws ApiException {
+        boolean deleted = false;
+        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;
+            }
+        }
+        return deleted;
+    }
+
     @Override
     public boolean deletePhysicalDisk(String name, KVMStoragePool pool, Storage.ImageFormat format)
     {
         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();
 
         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);
+            return deRefOrDeleteResource(api, rscName, rscGrpName);
         } catch (ApiException apiEx) {
+            s_logger.error("Linstor: ApiEx - " + apiEx.getMessage());
             throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
         }
-        return true;
     }
 
     @Override
@@ -558,6 +604,56 @@ private boolean resourceSupportZeroBlocks(KVMStoragePool destPool, String resNam
         return false;
     }
 
+    /**
+     * Checks if the given disk is the SystemVM template, by checking its properties file in the same directory.
+     * The initial systemvm template resource isn't created on the management server, but
+     * we now need to know if the systemvm template is used, while copying.
+     * @param disk
+     * @return True if it is the systemvm template disk, else false.
+     */
+    private static boolean isSystemTemplate(KVMPhysicalDisk disk) {
+        Path diskPath = Paths.get(disk.getPath());
+        Path propFile = diskPath.getParent().resolve("template.properties");
+        if (Files.exists(propFile)) {
+            java.util.Properties templateProps = new java.util.Properties();
+            try {
+                templateProps.load(new FileInputStream(propFile.toFile()));
+                String desc = templateProps.getProperty("description");
+                if (desc.startsWith("SystemVM Template")) {
+                    return true;
+                }
+            } catch (IOException e) {
+                return false;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Conditionally sets the correct aux properties for templates or basic resources.
+     * @param api
+     * @param srcDisk
+     * @param destPool
+     * @param name
+     */
+    private void setRscDfnAuxProperties(
+            DevelopersApi api, KVMPhysicalDisk srcDisk, KVMStoragePool destPool, String name) {
+        // if it is the initial systemvm disk copy, we need to apply the _cs-template-for property.
+        if (isSystemTemplate(srcDisk)) {
+            applyAuxProps(api, name, "SystemVM Template", null);
+            LinstorStoragePool linPool = (LinstorStoragePool) destPool;
+            final String rscName = getLinstorRscName(name);
+            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, srcDisk.getDispName(), srcDisk.getVmName());
+        }
+    }
+
     @Override
     public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMStoragePool destPools, int timeout, byte[] srcPassphrase, byte[] destPassphrase, Storage.ProvisioningType provisioningType)
     {
@@ -571,15 +667,14 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt
             name, QemuImg.PhysicalDiskFormat.RAW, provisioningType, disk.getVirtualSize(), null);
 
         final DevelopersApi api = getLinstorAPI(destPools);
-        applyAuxProps(api, name, disk.getDispName(), disk.getVmName());
+        setRscDfnAuxProperties(api, disk, destPools, name);
 
         s_logger.debug(String.format("Linstor.copyPhysicalDisk: dstPath: %s", dstDisk.getPath()));
         final QemuImgFile destFile = new QemuImgFile(dstDisk.getPath());
         destFile.setFormat(dstDisk.getFormat());
         destFile.setSize(disk.getVirtualSize());
 
-        boolean zeroedDevice = resourceSupportZeroBlocks(destPools, LinstorUtil.RSC_PREFIX + name);
-
+        boolean zeroedDevice = resourceSupportZeroBlocks(destPools, getLinstorRscName(name));
         try {
             final QemuImg qemu = new QemuImg(timeout, zeroedDevice, true);
             qemu.convert(srcFile, destFile);