[KVM] Disconnect the volumes with the proper storage adaptor. (apache#6029)

* [KVM] Disconnect the volumes with the proper storage adaptor.

* Improved / Added logs

Author: sureshanaparti

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -1681,7 +1681,7 @@ private List<Map<String, String>> getVolumesToDisconnect(VirtualMachine vm) {
             StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
 
             if (storagePool != null && storagePool.isManaged()) {
-                Map<String, String> info = new HashMap<>(3);
+                Map<String, String> info = new HashMap<>();
 
                 info.put(DiskTO.STORAGE_HOST, storagePool.getHostAddress());
                 info.put(DiskTO.STORAGE_PORT, String.valueOf(storagePool.getPort()));

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

@@ -3024,7 +3024,7 @@ public boolean cleanupDisk(Map<String, String> volumeToDisconnect) {
     public boolean cleanupDisk(final DiskDef disk) {
         final String path = disk.getDiskPath();
 
-        if (path == null) {
+        if (org.apache.commons.lang.StringUtils.isBlank(path)) {
             s_logger.debug("Unable to clean up disk with null path (perhaps empty cdrom drive):" + disk);
             return false;
         }

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java

@@ -331,12 +331,6 @@ public boolean disconnectPhysicalDisk(String volumeUuid, KVMStoragePool pool) {
 
     @Override
     public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect) {
-        String poolType = volumeToDisconnect.get(DiskTO.PROTOCOL_TYPE);
-        // Unsupported pool types
-        if (poolType != null && poolType.equalsIgnoreCase(StoragePoolType.PowerFlex.toString())) {
-            return false;
-        }
-
         String host = volumeToDisconnect.get(DiskTO.STORAGE_HOST);
         String port = volumeToDisconnect.get(DiskTO.STORAGE_PORT);
         String path = volumeToDisconnect.get(DiskTO.IQN);

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

@@ -29,6 +29,7 @@
 import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
 import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
+import org.apache.commons.collections.MapUtils;
 import org.apache.log4j.Logger;
 import org.reflections.Reflections;
 
@@ -165,10 +166,27 @@ public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec) {
     }
 
     public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect) {
+        s_logger.debug(String.format("Disconnect physical disks using volume map: %s", volumeToDisconnect.toString()));
+        if (MapUtils.isEmpty(volumeToDisconnect)) {
+            return false;
+        }
+
+        if (volumeToDisconnect.get(DiskTO.PROTOCOL_TYPE) != null) {
+            String poolType = volumeToDisconnect.get(DiskTO.PROTOCOL_TYPE);
+            StorageAdaptor adaptor = _storageMapper.get(poolType);
+            if (adaptor != null) {
+                s_logger.info(String.format("Disconnecting physical disk using the storage adaptor found for pool type: %s", poolType));
+                return adaptor.disconnectPhysicalDisk(volumeToDisconnect);
+            }
+
+            s_logger.debug(String.format("Couldn't find the storage adaptor for pool type: %s to disconnect the physical disk, trying with others", poolType));
+        }
+
         for (Map.Entry<String, StorageAdaptor> set : _storageMapper.entrySet()) {
             StorageAdaptor adaptor = set.getValue();
 
             if (adaptor.disconnectPhysicalDisk(volumeToDisconnect)) {
+                s_logger.debug(String.format("Disconnected physical disk using the storage adaptor for pool type: %s", set.getKey()));
                 return true;
             }
         }
@@ -177,10 +195,12 @@ public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect) {
     }
 
     public boolean disconnectPhysicalDiskByPath(String path) {
+        s_logger.debug(String.format("Disconnect physical disk by path: %s", path));
         for (Map.Entry<String, StorageAdaptor> set : _storageMapper.entrySet()) {
             StorageAdaptor adaptor = set.getValue();
 
             if (adaptor.disconnectPhysicalDiskByPath(path)) {
+                s_logger.debug(String.format("Disconnected physical disk by local path: %s, using the storage adaptor for pool type: %s", path, set.getKey()));
                 return true;
             }
         }

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

@@ -288,8 +288,7 @@ public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool)
     @Override
     public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect)
     {
-        s_logger.debug("Linstor: disconnectPhysicalDisk map");
-        return true;
+        return false;
     }
 
     private Optional<ResourceWithVolumes> getResourceByPath(final List<ResourceWithVolumes> resources, String path) {
@@ -309,10 +308,10 @@ private Optional<ResourceWithVolumes> getResourceByPath(final List<ResourceWithV
     @Override
     public boolean disconnectPhysicalDiskByPath(String localPath)
     {
-        s_logger.debug("Linstor: disconnectPhysicalDiskByPath " + localPath);
         // get first storage pool from the map, as we don't know any better:
         if (!MapStorageUuidToStoragePool.isEmpty())
         {
+            s_logger.debug("Linstor: disconnectPhysicalDiskByPath " + localPath);
             String firstKey = MapStorageUuidToStoragePool.keySet().stream().findFirst().get();
             final KVMStoragePool pool = MapStorageUuidToStoragePool.get(firstKey);
 

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java

@@ -214,12 +214,12 @@ public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool) {
 
     @Override
     public boolean disconnectPhysicalDisk(Map<String, String> volumeToDisconnect) {
-        return true;
+        return false;
     }
 
     @Override
     public boolean disconnectPhysicalDiskByPath(String localPath) {
-        return true;
+        return false;
     }
 
     @Override