Merge remote-tracking branch 'origin/4.18' into 4.19

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java

@@ -482,19 +482,19 @@ public class SystemVmTemplateRegistration {
             templateZoneVO = vmTemplateZoneDao.persist(templateZoneVO);
         } else {
             templateZoneVO.setLastUpdated(new java.util.Date());
-            if (vmTemplateZoneDao.update(templateZoneVO.getId(), templateZoneVO)) {
+            if (!vmTemplateZoneDao.update(templateZoneVO.getId(), templateZoneVO)) {
                 templateZoneVO = null;
             }
         }
         return templateZoneVO;
     }
 
-    private void createCrossZonesTemplateZoneRefEntries(VMTemplateVO template) {
+    private void createCrossZonesTemplateZoneRefEntries(Long templateId) {
         List<DataCenterVO> dcs = dataCenterDao.listAll();
         for (DataCenterVO dc : dcs) {
-            VMTemplateZoneVO templateZoneVO = createOrUpdateTemplateZoneEntry(dc.getId(), template.getId());
+            VMTemplateZoneVO templateZoneVO = createOrUpdateTemplateZoneEntry(dc.getId(), templateId);
             if (templateZoneVO == null) {
-                throw new CloudRuntimeException(String.format("Failed to create template_zone_ref record for the systemVM template for hypervisor: %s and zone: %s", template.getHypervisorType().name(), dc));
+                throw new CloudRuntimeException(String.format("Failed to create template_zone_ref record for the systemVM template (id: %s) and zone: %s", templateId, dc));
             }
         }
     }
@@ -624,8 +624,9 @@ public class SystemVmTemplateRegistration {
                 throw new CloudRuntimeException(String.format("Failed to register template for hypervisor: %s", hypervisor.name()));
             }
             templateId = template.getId();
-            createCrossZonesTemplateZoneRefEntries(template);
         }
+        createCrossZonesTemplateZoneRefEntries(templateId);
+
         details.setId(templateId);
         String destTempFolderName = String.valueOf(templateId);
         String destTempFolder = filePath + PARTIAL_TEMPLATE_FOLDER + destTempFolderName;

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java

@@ -972,6 +972,10 @@ public class LibvirtVMDef {
             return _diskLabel;
         }
 
+        public void setDiskLabel(String label) {
+            _diskLabel = label;
+        }
+
         public DiskType getDiskType() {
             return _diskType;
         }

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java

@@ -18,13 +18,20 @@
  */
 package com.cloud.hypervisor.kvm.resource;
 
+import java.util.Iterator;
+import java.util.Set;
 import java.util.concurrent.Callable;
 
+import org.apache.log4j.Logger;
 import org.libvirt.Connect;
 import org.libvirt.Domain;
 import org.libvirt.LibvirtException;
+import org.libvirt.TypedParameter;
+import org.libvirt.TypedStringParameter;
+import org.libvirt.TypedUlongParameter;
 
 public class MigrateKVMAsync implements Callable<Domain> {
+    protected Logger logger = Logger.getLogger(getClass());
 
     private final LibvirtComputingResource libvirtComputingResource;
 
@@ -37,6 +44,8 @@ public class MigrateKVMAsync implements Callable<Domain> {
     private boolean migrateNonSharedInc;
     private boolean autoConvergence;
 
+    protected Set<String> migrateDiskLabels;
+
     // Libvirt Migrate Flags reference:
     // https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMigrateFlags
 
@@ -87,7 +96,7 @@ public class MigrateKVMAsync implements Callable<Domain> {
     private static final int LIBVIRT_VERSION_SUPPORTS_AUTO_CONVERGE = 1002003;
 
     public MigrateKVMAsync(final LibvirtComputingResource libvirtComputingResource, final Domain dm, final Connect dconn, final String dxml,
-            final boolean migrateStorage, final boolean migrateNonSharedInc, final boolean autoConvergence, final String vmName, final String destIp) {
+            final boolean migrateStorage, final boolean migrateNonSharedInc, final boolean autoConvergence, final String vmName, final String destIp, Set<String> migrateDiskLabels) {
         this.libvirtComputingResource = libvirtComputingResource;
 
         this.dm = dm;
@@ -98,6 +107,7 @@ public class MigrateKVMAsync implements Callable<Domain> {
         this.autoConvergence = autoConvergence;
         this.vmName = vmName;
         this.destIp = destIp;
+        this.migrateDiskLabels = migrateDiskLabels;
     }
 
     @Override
@@ -121,6 +131,37 @@ public class MigrateKVMAsync implements Callable<Domain> {
             flags |= VIR_MIGRATE_AUTO_CONVERGE;
         }
 
-        return dm.migrate(dconn, flags, dxml, vmName, "tcp:" + destIp, libvirtComputingResource.getMigrateSpeed());
+        TypedParameter [] parameters = createTypedParameterList();
+
+        logger.debug(String.format("Migrating [%s] with flags [%s], destination [%s] and speed [%s]. The disks with the following labels will be migrated [%s].", vmName, flags,
+                destIp, libvirtComputingResource.getMigrateSpeed(), migrateDiskLabels));
+
+        return dm.migrate(dconn, parameters, flags);
+
     }
+
+    protected TypedParameter[] createTypedParameterList() {
+        int sizeOfMigrateDiskLabels = 0;
+        if (migrateDiskLabels != null) {
+            sizeOfMigrateDiskLabels = migrateDiskLabels.size();
+        }
+
+        TypedParameter[] parameters = new TypedParameter[4 + sizeOfMigrateDiskLabels];
+        parameters[0] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_NAME, vmName);
+        parameters[1] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_XML, dxml);
+        parameters[2] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_URI, "tcp:" + destIp);
+        parameters[3] = new TypedUlongParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_BANDWIDTH, libvirtComputingResource.getMigrateSpeed());
+
+        if (sizeOfMigrateDiskLabels == 0) {
+            return parameters;
+        }
+
+        Iterator<String> iterator = migrateDiskLabels.iterator();
+        for (int i = 0; i < sizeOfMigrateDiskLabels; i++) {
+            parameters[4 + i] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_MIGRATE_DISKS, iterator.next());
+        }
+
+        return parameters;
+    }
+
 }

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java

@@ -24,6 +24,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.net.URISyntaxException;
 import java.nio.charset.StandardCharsets;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -190,6 +191,7 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper<MigrateCo
             // migrateStorage's value should always only be associated with the initial state of mapMigrateStorage.
             final boolean migrateStorage = MapUtils.isNotEmpty(mapMigrateStorage);
             final boolean migrateStorageManaged = command.isMigrateStorageManaged();
+            Set<String> migrateDiskLabels = null;
 
             if (migrateStorage) {
                 if (s_logger.isDebugEnabled()) {
@@ -199,6 +201,7 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper<MigrateCo
                 if (s_logger.isDebugEnabled()) {
                     s_logger.debug(String.format("Changed VM [%s] XML configuration of used storage. New XML configuration is [%s].", vmName, xmlDesc));
                 }
+                migrateDiskLabels = getMigrateStorageDeviceLabels(disks, mapMigrateStorage);
             }
 
             Map<String, DpdkTO> dpdkPortsMapping = command.getDpdkInterfaceMapping();
@@ -227,7 +230,7 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper<MigrateCo
 
             final Callable<Domain> worker = new MigrateKVMAsync(libvirtComputingResource, dm, dconn, xmlDesc,
                     migrateStorage, migrateNonSharedInc,
-                    command.isAutoConvergence(), vmName, command.getDestinationIp());
+                    command.isAutoConvergence(), vmName, command.getDestinationIp(), migrateDiskLabels);
             final Future<Domain> migrateThread = executor.submit(worker);
             executor.shutdown();
             long sleeptime = 0;
@@ -365,6 +368,30 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper<MigrateCo
         return new MigrateAnswer(command, result == null, result, null);
     }
 
+    /**
+     * Gets the disk labels (vda, vdb...) of the disks mapped for migration on mapMigrateStorage.
+     * @param diskDefinitions list of all the disksDefinitions of the VM.
+     * @param mapMigrateStorage map of the disks that should be migrated.
+     * @return set with the labels of the disks that should be migrated.
+     * */
+    protected Set<String> getMigrateStorageDeviceLabels(List<DiskDef> diskDefinitions, Map<String, MigrateCommand.MigrateDiskInfo> mapMigrateStorage) {
+        HashSet<String> setOfLabels = new HashSet<>();
+        s_logger.debug(String.format("Searching for disk labels of disks [%s].", mapMigrateStorage.keySet()));
+        for (String fileName : mapMigrateStorage.keySet()) {
+            for (DiskDef diskDef : diskDefinitions) {
+                String diskPath = diskDef.getDiskPath();
+                if (diskPath != null && diskPath.contains(fileName)) {
+                    setOfLabels.add(diskDef.getDiskLabel());
+                    s_logger.debug(String.format("Found label [%s] for disk [%s].", diskDef.getDiskLabel(), fileName));
+                    break;
+                }
+            }
+        }
+
+        return setOfLabels;
+    }
+
+
     /**
      * Checks if the CPU shares are equal in the source host and destination host.
      *  <ul>

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsyncTest.java

@@ -0,0 +1,83 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.TypedParameter;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Set;
+
+@RunWith(MockitoJUnitRunner.class)
+public class MigrateKVMAsyncTest {
+
+    @Mock
+    private LibvirtComputingResource libvirtComputingResource;
+    @Mock
+    private Connect connect;
+    @Mock
+    private Domain domain;
+
+
+    @Test
+    public void createTypedParameterListTestNoMigrateDiskLabels() {
+        MigrateKVMAsync migrateKVMAsync = new MigrateKVMAsync(libvirtComputingResource, domain, connect, "testxml",
+                false, false, false, "tst", "1.1.1.1", null);
+
+        Mockito.doReturn(10).when(libvirtComputingResource).getMigrateSpeed();
+
+        TypedParameter[] result = migrateKVMAsync.createTypedParameterList();
+
+        Assert.assertEquals(4, result.length);
+
+        Assert.assertEquals("tst", result[0].getValueAsString());
+        Assert.assertEquals("testxml", result[1].getValueAsString());
+        Assert.assertEquals("tcp:1.1.1.1", result[2].getValueAsString());
+        Assert.assertEquals("10", result[3].getValueAsString());
+
+    }
+
+    @Test
+    public void createTypedParameterListTestWithMigrateDiskLabels() {
+        Set<String> labels = Set.of("vda", "vdb");
+        MigrateKVMAsync migrateKVMAsync = new MigrateKVMAsync(libvirtComputingResource, domain, connect, "testxml",
+                false, false, false, "tst", "1.1.1.1", labels);
+
+        Mockito.doReturn(10).when(libvirtComputingResource).getMigrateSpeed();
+
+        TypedParameter[] result = migrateKVMAsync.createTypedParameterList();
+
+        Assert.assertEquals(6, result.length);
+
+        Assert.assertEquals("tst", result[0].getValueAsString());
+        Assert.assertEquals("testxml", result[1].getValueAsString());
+        Assert.assertEquals("tcp:1.1.1.1", result[2].getValueAsString());
+        Assert.assertEquals("10", result[3].getValueAsString());
+
+        Assert.assertEquals(labels, Set.of(result[4].getValueAsString(), result[5].getValueAsString()));
+    }
+
+}

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java

@@ -26,9 +26,11 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -574,6 +576,14 @@ public class LibvirtMigrateCommandWrapperTest {
             "  </devices>\n" +
             "</domain>\n";
 
+    private Map<String, MigrateDiskInfo> createMapMigrateStorage(String sourceText, String path) {
+        Map<String, MigrateDiskInfo> mapMigrateStorage = new HashMap<String, MigrateDiskInfo>();
+
+        MigrateDiskInfo diskInfo = new MigrateDiskInfo("123456", DiskType.BLOCK, DriverType.RAW, Source.FILE, sourceText);
+        mapMigrateStorage.put(path, diskInfo);
+        return mapMigrateStorage;
+    }
+
     @Test
     public void testReplaceIpForVNCInDescFile() {
         final String targetIp = "192.168.22.21";
@@ -750,10 +760,8 @@ public class LibvirtMigrateCommandWrapperTest {
 
     @Test
     public void testReplaceStorage() throws Exception {
-        Map<String, MigrateDiskInfo> mapMigrateStorage = new HashMap<String, MigrateDiskInfo>();
+        Map<String, MigrateDiskInfo> mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6");
 
-        MigrateDiskInfo diskInfo = new MigrateDiskInfo("123456", DiskType.BLOCK, DriverType.RAW, Source.FILE, "sourctest");
-        mapMigrateStorage.put("/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6", diskInfo);
         final String result = libvirtMigrateCmdWrapper.replaceStorage(fullfile, mapMigrateStorage, true);
 
         InputStream in = IOUtils.toInputStream(result, "UTF-8");
@@ -767,7 +775,6 @@ public class LibvirtMigrateCommandWrapperTest {
 
     @Test
     public void testReplaceStorageWithSecrets() throws Exception {
-        Map<String, MigrateDiskInfo> mapMigrateStorage = new HashMap<String, MigrateDiskInfo>();
 
         final String xmlDesc =
             "<domain type='kvm' id='3'>" +
@@ -788,8 +795,7 @@ public class LibvirtMigrateCommandWrapperTest {
 
         final String volumeFile = "3530f749-82fd-458e-9485-a357e6e541db";
         String newDiskPath = "/mnt/2d0435e1-99e0-4f1d-94c0-bee1f6f8b99e/" + volumeFile;
-        MigrateDiskInfo diskInfo = new MigrateDiskInfo("123456", DiskType.BLOCK, DriverType.RAW, Source.FILE, newDiskPath);
-        mapMigrateStorage.put("/mnt/07eb495b-5590-3877-9fb7-23c6e9a40d40/bf8621b3-027c-497d-963b-06319650f048", diskInfo);
+        Map<String, MigrateDiskInfo> mapMigrateStorage = createMapMigrateStorage(newDiskPath, "/mnt/07eb495b-5590-3877-9fb7-23c6e9a40d40/bf8621b3-027c-497d-963b-06319650f048");
         final String result = libvirtMigrateCmdWrapper.replaceStorage(xmlDesc, mapMigrateStorage, false);
         final String expectedSecretUuid = LibvirtComputingResource.generateSecretUUIDFromString(volumeFile);
 
@@ -941,4 +947,64 @@ public class LibvirtMigrateCommandWrapperTest {
 
         Assert.assertEquals(updateShares, newVmCpuShares);
     }
+
+    @Test
+    public void getMigrateStorageDeviceLabelsTestNoDiskDefinitions() {
+        Map<String, MigrateDiskInfo> mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6");
+
+        Set<String> result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(new ArrayList<>(), mapMigrateStorage);
+
+        assertTrue(result.isEmpty());
+    }
+
+    @Test
+    public void getMigrateStorageDeviceLabelsTestNoMapMigrateStorage() {
+        List<DiskDef> disks = new ArrayList<>();
+        DiskDef diskDef0 = new DiskDef();
+
+        diskDef0.setDiskPath("volPath");
+        disks.add(diskDef0);
+
+        Set<String> result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(disks, new HashMap<>());
+
+        assertTrue(result.isEmpty());
+    }
+
+    @Test
+    public void getMigrateStorageDeviceLabelsTestPathIsNotFound() {
+        List<DiskDef> disks = new ArrayList<>();
+        DiskDef diskDef0 = new DiskDef();
+
+        diskDef0.setDiskPath("volPath");
+        disks.add(diskDef0);
+
+        Map<String, MigrateDiskInfo> mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6");
+
+        Set<String> result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(disks, mapMigrateStorage);
+
+        assertTrue(result.isEmpty());
+    }
+
+    @Test
+    public void getMigrateStorageDeviceLabelsTestFindPathAndLabels() {
+        List<DiskDef> disks = new ArrayList<>();
+        DiskDef diskDef0 = new DiskDef();
+        DiskDef diskDef1 = new DiskDef();
+
+        diskDef0.setDiskPath("volPath1");
+        diskDef0.setDiskLabel("vda");
+        disks.add(diskDef0);
+
+        diskDef1.setDiskPath("volPath2");
+        diskDef1.setDiskLabel("vdb");
+        disks.add(diskDef1);
+
+        Map<String, MigrateDiskInfo> mapMigrateStorage = createMapMigrateStorage("sourceTest", "volPath1");
+        mapMigrateStorage.put("volPath2", new MigrateDiskInfo("123457", DiskType.BLOCK, DriverType.RAW, Source.FILE, "sourceText"));
+
+        Set<String> result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(disks, mapMigrateStorage);
+
+        assertTrue(result.containsAll(Arrays.asList("vda", "vdb")));
+    }
+
 }

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

@@ -34,6 +34,7 @@ import org.apache.log4j.Logger;
 import org.libvirt.LibvirtException;
 
 import com.linbit.linstor.api.ApiClient;
+import com.linbit.linstor.api.ApiConsts;
 import com.linbit.linstor.api.ApiException;
 import com.linbit.linstor.api.Configuration;
 import com.linbit.linstor.api.DevelopersApi;
@@ -76,6 +77,10 @@ public class LinstorStorageAdaptor implements StorageAdaptor {
         }
     }
 
+    private void logLinstorAnswers(@Nonnull ApiCallRcList answers) {
+        answers.forEach(this::logLinstorAnswer);
+    }
+
     private void checkLinstorAnswersThrow(@Nonnull ApiCallRcList answers) {
         answers.forEach(this::logLinstorAnswer);
         if (answers.hasError())
@@ -291,23 +296,90 @@ public class LinstorStorageAdaptor implements StorageAdaptor {
         return true;
     }
 
+    private boolean tryDisconnectLinstor(String volumePath, KVMStoragePool pool)
+    {
+        if (volumePath == null) {
+            return false;
+        }
+
+        s_logger.debug("Linstor: Using storage pool: " + pool.getUuid());
+        final DevelopersApi api = getLinstorAPI(pool);
+
+        Optional<ResourceWithVolumes> optRsc;
+        try
+        {
+            List<ResourceWithVolumes> resources = api.viewResources(
+                    Collections.singletonList(localNodeName),
+                    null,
+                    null,
+                    null,
+                    null,
+                    null);
+
+            optRsc = getResourceByPath(resources, volumePath);
+        } catch (ApiException apiEx) {
+            // couldn't query linstor controller
+            s_logger.error(apiEx.getBestMessage());
+            return false;
+        }
+
+
+        if (optRsc.isPresent()) {
+            try {
+                Resource rsc = optRsc.get();
+
+                // if diskless resource remove it, in the worst case it will be transformed to a tiebreaker
+                if (rsc.getFlags() != null &&
+                        rsc.getFlags().contains(ApiConsts.FLAG_DRBD_DISKLESS) &&
+                        !rsc.getFlags().contains(ApiConsts.FLAG_TIE_BREAKER)) {
+                    ApiCallRcList delAnswers = api.resourceDelete(rsc.getName(), localNodeName);
+                    logLinstorAnswers(delAnswers);
+                }
+
+                // remove allow-two-primaries
+                ResourceDefinitionModify rdm = new ResourceDefinitionModify();
+                rdm.deleteProps(Collections.singletonList("DrbdOptions/Net/allow-two-primaries"));
+                ApiCallRcList answers = api.resourceDefinitionModify(rsc.getName(), rdm);
+                if (answers.hasError()) {
+                    s_logger.error(
+                            String.format("Failed to remove 'allow-two-primaries' on %s: %s",
+                                    rsc.getName(), LinstorUtil.getBestErrorMessage(answers)));
+                    // do not fail here as removing allow-two-primaries property isn't fatal
+                }
+            } catch (ApiException apiEx) {
+                s_logger.error(apiEx.getBestMessage());
+                // do not fail here as removing allow-two-primaries property or deleting diskless isn't fatal
+            }
+
+            return true;
+        }
+
+        s_logger.warn("Linstor: Couldn't find resource for this path: " + volumePath);
+        return false;
+    }
+
     @Override
     public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool)
     {
         s_logger.debug("Linstor: disconnectPhysicalDisk " + pool.getUuid() + ":" + volumePath);
+        if (MapStorageUuidToStoragePool.containsValue(pool)) {
+            return tryDisconnectLinstor(volumePath, pool);
+        }
         return false;
     }
 
     @Override
     public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect)
     {
+        // as of now this is only relevant for iscsi targets
+        s_logger.info("Linstor: disconnectPhysicalDisk(Map<String, String> volumeToDisconnect) called?");
         return false;
     }
 
     private Optional<ResourceWithVolumes> getResourceByPath(final List<ResourceWithVolumes> resources, String path) {
         return resources.stream()
             .filter(rsc -> rsc.getVolumes().stream()
-                .anyMatch(v -> v.getDevicePath().equals(path)))
+                .anyMatch(v -> path.equals(v.getDevicePath())))
             .findFirst();
     }
 
@@ -328,46 +400,8 @@ public class LinstorStorageAdaptor implements StorageAdaptor {
             s_logger.debug("Linstor: disconnectPhysicalDiskByPath " + localPath);
             final KVMStoragePool pool = optFirstPool.get();
 
-            s_logger.debug("Linstor: Using storpool: " + pool.getUuid());
-            final DevelopersApi api = getLinstorAPI(pool);
-
-            Optional<ResourceWithVolumes> optRsc;
-            try {
-                List<ResourceWithVolumes> resources = api.viewResources(
-                        Collections.singletonList(localNodeName),
-                        null,
-                        null,
-                        null,
-                        null,
-                        null);
-
-                optRsc = getResourceByPath(resources, localPath);
-            } catch (ApiException apiEx) {
-                // couldn't query linstor controller
-                s_logger.error(apiEx.getBestMessage());
-                return false;
-            }
-
-            if (optRsc.isPresent()) {
-                try {
-                    Resource rsc = optRsc.get();
-                    ResourceDefinitionModify rdm = new ResourceDefinitionModify();
-                    rdm.deleteProps(Collections.singletonList("DrbdOptions/Net/allow-two-primaries"));
-                    ApiCallRcList answers = api.resourceDefinitionModify(rsc.getName(), rdm);
-                    if (answers.hasError()) {
-                        s_logger.error(
-                                String.format("Failed to remove 'allow-two-primaries' on %s: %s",
-                                        rsc.getName(), LinstorUtil.getBestErrorMessage(answers)));
-                        // do not fail here as removing allow-two-primaries property isn't fatal
-                    }
-                } catch(ApiException apiEx){
-                    s_logger.error(apiEx.getBestMessage());
-                    // do not fail here as removing allow-two-primaries property isn't fatal
-                }
-                return true;
-            }
+            return tryDisconnectLinstor(localPath, pool);
         }
-        s_logger.info("Linstor: Couldn't find resource for this path: " + localPath);
         return false;
     }
 

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -4331,7 +4331,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         }
 
         if (offering.getDefaultUse()) {
-            throw new InvalidParameterValueException("Default service offerings cannot be deleted");
+            throw new InvalidParameterValueException(String.format("The system service offering [%s] is marked for default use and cannot be deleted", offering.getDisplayText()));
         }
 
         final User user = _userDao.findById(userId);