linstor: Fix using multiple primary storage with same linstor-controller (apache#10280)

rp- · dhslove · commit cf061cac3974 · 2025-02-19T10:58:47.000+09:00
diff --git a/plugins/storage/volume/linstor/CHANGELOG.md b/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
diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java
@@ -61,6 +61,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;
 
 public class LinstorStorageAdaptor implements StorageAdaptor {
     protected Logger logger = LogManager.getLogger(getClass());
@@ -202,10 +207,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
@@ -215,22 +220,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();
                 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 {
                 logger.error("Linstor: viewResources didn't return resources or volumes.");
@@ -473,21 +484,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- properties 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)
     {
         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);
-            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) {
+            logger.error("Linstor: ApiEx - " + apiEx.getMessage());
             throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
         }
-        return true;
     }
 
     @Override
@@ -561,6 +607,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) {
+                logger.error("Error setting aux template for property for {}", 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)
     {
@@ -574,15 +670,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);
 
         logger.debug("Linstor.copyPhysicalDisk: dstPath: {}", 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);
@@ -735,4 +830,4 @@ public boolean isNodeOnline(LinstorStoragePool pool, String nodeName) {
             throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
         }
     }
-}
+}
diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java
@@ -26,6 +26,7 @@
 import com.linbit.linstor.api.model.Properties;
 import com.linbit.linstor.api.model.ProviderKind;
 import com.linbit.linstor.api.model.Resource;
+import com.linbit.linstor.api.model.ResourceDefinition;
 import com.linbit.linstor.api.model.ResourceDefinitionModify;
 import com.linbit.linstor.api.model.ResourceGroup;
 import com.linbit.linstor.api.model.ResourceWithVolumes;
@@ -37,8 +38,11 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
+import com.cloud.utils.Pair;
 import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
@@ -49,6 +53,7 @@ public class LinstorUtil {
     public final static String PROVIDER_NAME = "Linstor";
     public static final String RSC_PREFIX = "cs-";
     public static final String RSC_GROUP = "resourceGroup";
+    public static final String CS_TEMPLATE_FOR_PREFIX = "_cs-template-for-";
 
     public static final String TEMP_VOLUME_ID = "tempVolumeId";
 
@@ -288,4 +293,114 @@ public static ApiCallRcList applyAuxProps(DevelopersApi api, String rscName, Str
         }
         return answers;
     }
+
+    /**
+     * Returns all resource definitions that start with the given `startWith` name.
+     * @param api
+     * @param startWith startWith String
+     * @return a List with all ResourceDefinition starting with `startWith`
+     * @throws ApiException
+     */
+    public static List<ResourceDefinition> getRDListStartingWith(DevelopersApi api, String startWith)
+            throws ApiException
+    {
+        List<ResourceDefinition> rscDfns = api.resourceDefinitionList(null, null, null, null);
+
+        return rscDfns.stream()
+                .filter(rscDfn -> rscDfn.getName().toLowerCase().startsWith(startWith.toLowerCase()))
+                .collect(Collectors.toList());
+    }
+
+    /**
+     * Returns a pair list of resource-definitions with ther 1:1 mapped resource-group objects that start with the
+     * resource name `startWith`
+     * @param api
+     * @param startWith
+     * @return
+     * @throws ApiException
+     */
+    public static List<Pair<ResourceDefinition, ResourceGroup>> getRDAndRGListStartingWith(DevelopersApi api, String startWith)
+            throws ApiException
+    {
+        List<ResourceDefinition> foundRDs = getRDListStartingWith(api, startWith);
+
+        List<String> rscGrpStrings = foundRDs.stream()
+                .map(ResourceDefinition::getResourceGroupName)
+                .collect(Collectors.toList());
+
+        Map<String, ResourceGroup> rscGrps = api.resourceGroupList(rscGrpStrings, null, null, null).stream()
+                .collect(Collectors.toMap(ResourceGroup::getName, rscGrp -> rscGrp));
+
+        return foundRDs.stream()
+                .map(rd -> new Pair<>(rd, rscGrps.get(rd.getResourceGroupName())))
+                .collect(Collectors.toList());
+    }
+
+    /**
+     * The full name of template-for aux property key.
+     * @param rscGrpName
+     * @return
+     */
+    public static String getTemplateForAuxPropKey(String rscGrpName) {
+        return String.format("Aux/%s%s", CS_TEMPLATE_FOR_PREFIX, rscGrpName);
+    }
+
+    /**
+     * Template resource should have a _cs-template-for-... property, that indicates to which resource-group
+     * this template belongs, it works like a refcount to keep it alive if there are still such properties on the
+     * template resource. That methods set the correct property on the given resource.
+     * @param api
+     * @param rscName Resource name to set the property.
+     * @param rscGrpName Resource group this template should belong too.
+     * @throws ApiException
+     */
+    public static void setAuxTemplateForProperty(DevelopersApi api, String rscName, String rscGrpName)
+            throws ApiException
+    {
+        ResourceDefinitionModify rdm = new ResourceDefinitionModify();
+        Properties props = new Properties();
+        String propKey = LinstorUtil.getTemplateForAuxPropKey(rscGrpName);
+        props.put(propKey, "true");
+        rdm.setOverrideProps(props);
+        ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
+
+        if (answers.hasError()) {
+            String bestError = LinstorUtil.getBestErrorMessage(answers);
+            s_logger.error(String.format("Set %s on %s error: %s", propKey, rscName, bestError));
+            throw new CloudRuntimeException(bestError);
+        } else {
+            s_logger.info(String.format("Set %s property on %s", propKey, rscName));
+        }
+    }
+
+    /**
+     * Find the correct resource definition to clone from.
+     * There could be multiple resource definitions for the same template, with the same prefix.
+     * This method searches for which resource group the resource definition was intended and returns that.
+     * If no exact resource definition could be found, we return the first with a similar name as a fallback.
+     * If there is not even one with the correct prefix, we return null.
+     * @param api
+     * @param rscName
+     * @param rscGrpName
+     * @return The resource-definition to clone from, if no template and no match, return null.
+     * @throws ApiException
+     */
+    public static ResourceDefinition findResourceDefinition(DevelopersApi api, String rscName, String rscGrpName)
+            throws ApiException {
+        List<ResourceDefinition> rscDfns = api.resourceDefinitionList(null, null, null, null);
+
+        List<ResourceDefinition> rdsStartingWith = rscDfns.stream()
+                .filter(rscDfn -> rscDfn.getName().toLowerCase().startsWith(rscName.toLowerCase()))
+                .collect(Collectors.toList());
+
+        if (rdsStartingWith.isEmpty()) {
+            return null;
+        }
+
+        Optional<ResourceDefinition> rd = rdsStartingWith.stream()
+                .filter(rscDfn -> rscDfn.getProps().containsKey(LinstorUtil.getTemplateForAuxPropKey(rscGrpName)))
+                .findFirst();
+
+        return rd.orElseGet(() -> rdsStartingWith.get(0));
+    }
 }
