linstor: Do not pretend handling disconnect paths that are non Linstor (apache#8897)

Author: rp-

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

@@ -35,13 +35,15 @@
 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;
 import com.linbit.linstor.api.model.ApiCallRc;
 import com.linbit.linstor.api.model.ApiCallRcList;
 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.ResourceGroupSpawn;
@@ -80,6 +82,10 @@ private void logLinstorAnswer(@Nonnull ApiCallRc answer) {
         }
     }
 
+    private void logLinstorAnswers(@Nonnull ApiCallRcList answers) {
+        answers.forEach(this::logLinstorAnswer);
+    }
+
     private void checkLinstorAnswersThrow(@Nonnull ApiCallRcList answers) {
         answers.forEach(this::logLinstorAnswer);
         if (answers.hasError())
@@ -239,6 +245,28 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
         }
     }
 
+    /**
+     * Checks if the given resource is in use by drbd on any host and
+     * if so set the drbd option allow-two-primaries
+     * @param api linstor api object
+     * @param rscName resource name to set allow-two-primaries if in use
+     * @throws ApiException if any problem connecting to the Linstor controller
+     */
+    private void allow2PrimariesIfInUse(DevelopersApi api, String rscName) throws ApiException {
+        if (LinstorUtil.isResourceInUse(api, rscName)) {
+            // allow 2 primaries for live migration, should be removed by disconnect on the other end
+            ResourceDefinitionModify rdm = new ResourceDefinitionModify();
+            Properties props = new Properties();
+            props.put("DrbdOptions/Net/allow-two-primaries", "yes");
+            rdm.setOverrideProps(props);
+            ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
+            if (answers.hasError()) {
+                logger.error("Unable to set 'allow-two-primaries' on {} ", rscName);
+                // do not fail here as adding allow-two-primaries property is only a problem while live migrating
+            }
+        }
+    }
+
     @Override
     public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<String, String> details)
     {
@@ -265,40 +293,98 @@ public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<S
 
         try
         {
-            // allow 2 primaries for live migration, should be removed by disconnect on the other end
-            ResourceDefinitionModify rdm = new ResourceDefinitionModify();
-            Properties props = new Properties();
-            props.put("DrbdOptions/Net/allow-two-primaries", "yes");
-            rdm.setOverrideProps(props);
-            ApiCallRcList answers = api.resourceDefinitionModify(rscName, rdm);
-            if (answers.hasError()) {
-                logger.error("Unable to set 'allow-two-primaries' on " + rscName);
-                // do not fail here as adding allow-two-primaries property is only a problem while live migrating
-            }
+            allow2PrimariesIfInUse(api, rscName);
         } catch (ApiException apiEx) {
             logger.error(apiEx);
             // do not fail here as adding allow-two-primaries property is only a problem while live migrating
         }
         return true;
     }
 
+    private boolean tryDisconnectLinstor(String volumePath, KVMStoragePool pool)
+    {
+        if (volumePath == null) {
+            return false;
+        }
+
+        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
+            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()) {
+                    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) {
+                logger.error(apiEx.getBestMessage());
+                // do not fail here as removing allow-two-primaries property or deleting diskless isn't fatal
+            }
+
+            return true;
+        }
+
+        logger.warn("Linstor: Couldn't find resource for this path: " + volumePath);
+        return false;
+    }
+
     @Override
     public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool)
     {
-        logger.debug("Linstor: disconnectPhysicalDisk " + pool.getUuid() + ":" + volumePath);
-        return true;
+        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
+        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();
     }
 
@@ -319,42 +405,9 @@ public boolean disconnectPhysicalDiskByPath(String localPath)
             logger.debug("Linstor: disconnectPhysicalDiskByPath " + localPath);
             final KVMStoragePool pool = optFirstPool.get();
 
-            logger.debug("Linstor: Using storpool: " + pool.getUuid());
-            final DevelopersApi api = getLinstorAPI(pool);
-
-            try
-            {
-                List<ResourceWithVolumes> resources = api.viewResources(
-                    Collections.singletonList(localNodeName),
-                    null,
-                    null,
-                    null,
-                    null,
-                    null);
-
-                Optional<ResourceWithVolumes> rsc = getResourceByPath(resources, localPath);
-
-                if (rsc.isPresent())
-                {
-                    ResourceDefinitionModify rdm = new ResourceDefinitionModify();
-                    rdm.deleteProps(Collections.singletonList("DrbdOptions/Net/allow-two-primaries"));
-                    ApiCallRcList answers = api.resourceDefinitionModify(rsc.get().getName(), rdm);
-                    if (answers.hasError()) {
-                        logger.error(
-                                String.format("Failed to remove 'allow-two-primaries' on %s: %s",
-                                        rsc.get().getName(), LinstorUtil.getBestErrorMessage(answers)));
-                        // do not fail here as removing allow-two-primaries property isn't fatal
-                    }
-
-                    return true;
-                }
-                logger.warn("Linstor: Couldn't find resource for this path: " + localPath);
-            } catch (ApiException apiEx) {
-                logger.error(apiEx.getBestMessage());
-                // do not fail here as removing allow-two-primaries property isn't fatal
-            }
+            return tryDisconnectLinstor(localPath, pool);
         }
-        return true;
+        return false;
     }
 
     @Override
@@ -520,4 +573,4 @@ public long getUsed(LinstorStoragePool pool) {
             throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
         }
     }
-}
+}