From 027e6030aff74fd4fdf7d031251d2289c7541e8b Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 23 Feb 2022 22:40:14 +0530 Subject: [PATCH] [KVM] Disconnect the volumes with the proper storage adaptor. (#6029) * [KVM] Disconnect the volumes with the proper storage adaptor. * Improved / Added logs --- .../cloud/vm/VirtualMachineManagerImpl.java | 2 +- .../resource/LibvirtComputingResource.java | 2 +- .../kvm/storage/IscsiAdmStorageAdaptor.java | 6 ------ .../kvm/storage/KVMStoragePoolManager.java | 20 +++++++++++++++++++ .../kvm/storage/LinstorStorageAdaptor.java | 5 ++--- .../kvm/storage/ScaleIOStorageAdaptor.java | 4 ++-- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 2fa44b1d308..e090955c147 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1681,7 +1681,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId()); if (storagePool != null && storagePool.isManaged()) { - Map info = new HashMap<>(3); + Map info = new HashMap<>(); info.put(DiskTO.STORAGE_HOST, storagePool.getHostAddress()); info.put(DiskTO.STORAGE_PORT, String.valueOf(storagePool.getPort())); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 60d8e0865b2..cdcbf24b92f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -3024,7 +3024,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv 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; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index fb896834e1b..389a2c717b7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java @@ -331,12 +331,6 @@ public class IscsiAdmStorageAdaptor implements StorageAdaptor { @Override public boolean disconnectPhysicalDisk(Map 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); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index 77579c4eca6..d6c49d295e0 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java @@ -29,6 +29,7 @@ import java.util.concurrent.ConcurrentHashMap; 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 class KVMStoragePoolManager { } public boolean disconnectPhysicalDisk(Map 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 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 class KVMStoragePoolManager { } public boolean disconnectPhysicalDiskByPath(String path) { + s_logger.debug(String.format("Disconnect physical disk by path: %s", path)); for (Map.Entry 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; } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java index 24e4d4654ab..dc00601674b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java @@ -288,8 +288,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { @Override public boolean disconnectPhysicalDisk(Map volumeToDisconnect) { - s_logger.debug("Linstor: disconnectPhysicalDisk map"); - return true; + return false; } private Optional getResourceByPath(final List resources, String path) { @@ -309,10 +308,10 @@ public class LinstorStorageAdaptor implements StorageAdaptor { @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); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java index 4103d76348b..59eaab0f2a1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java @@ -214,12 +214,12 @@ public class ScaleIOStorageAdaptor implements StorageAdaptor { @Override public boolean disconnectPhysicalDisk(Map volumeToDisconnect) { - return true; + return false; } @Override public boolean disconnectPhysicalDiskByPath(String localPath) { - return true; + return false; } @Override