From 20735beb6d07e1a974b9f2225dee4afd82f67ac0 Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Wed, 7 Aug 2024 12:02:37 +0200 Subject: [PATCH] kvm: ref-count secondary storage pool usage If a secondary storage pool is used by e.g. 2 concurrent snapshot->template actions, if the first action finished it removed the netfs mount point for the other action. Now the storage pools are usage ref-counted and will only deleted if there are no more users. --- .../kvm/storage/IscsiAdmStorageAdaptor.java | 2 +- .../kvm/storage/KVMStoragePoolManager.java | 2 +- .../kvm/storage/LibvirtStorageAdaptor.java | 54 ++++++++++++++++++- .../kvm/storage/ManagedNfsStorageAdaptor.java | 2 +- .../kvm/storage/MultipathSCSIAdapterBase.java | 2 +- .../kvm/storage/ScaleIOStorageAdaptor.java | 2 +- .../kvm/storage/StorageAdaptor.java | 2 +- .../storage/LibvirtStorageAdaptorTest.java | 2 +- .../kvm/storage/LinstorStorageAdaptor.java | 2 +- .../kvm/storage/StorPoolStorageAdaptor.java | 2 +- 10 files changed, 62 insertions(+), 10 deletions(-) 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 f980cd295bd2..6c3ac8ed20af 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 @@ -43,7 +43,7 @@ public class IscsiAdmStorageAdaptor implements StorageAdaptor { private static final Map MapStorageUuidToStoragePool = new HashMap<>(); @Override - public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details) { + public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details, boolean isPrimaryStorage) { IscsiAdmStoragePool storagePool = new IscsiAdmStoragePool(uuid, host, port, storagePoolType, this); MapStorageUuidToStoragePool.put(uuid, storagePool); 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 4f25cfa08d59..ee23e489951d 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 @@ -361,7 +361,7 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri //Note: due to bug CLOUDSTACK-4459, createStoragepool can be called in parallel, so need to be synced. private synchronized KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map details, boolean primaryStorage) { StorageAdaptor adaptor = getStorageAdaptor(type); - KVMStoragePool pool = adaptor.createStoragePool(name, host, port, path, userInfo, type, details); + KVMStoragePool pool = adaptor.createStoragePool(name, host, port, path, userInfo, type, details, primaryStorage); // LibvirtStorageAdaptor-specific statement if (pool.isPoolSupportHA() && primaryStorage) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index cad9d429969c..57a492452e5b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -72,6 +72,7 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; @@ -80,6 +81,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { private StorageLayer _storageLayer; private String _mountPoint = "/mnt"; private String _manageSnapshotPath; + private static final ConcurrentHashMap storagePoolRefCounts = new ConcurrentHashMap<>(); private String rbdTemplateSnapName = "cloudstack-base-snap"; private static final int RBD_FEATURE_LAYERING = 1; @@ -637,8 +639,44 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) { } } + /** + * adjust refcount + */ + private int adjustStoragePoolRefCount(String uuid, int adjustment) { + final String mutexKey = storagePoolRefCounts.keySet().stream() + .filter(k -> k.equals(uuid)) + .findFirst() + .orElse(uuid); + synchronized (mutexKey) { + // some access on the storagePoolRefCounts.key(mutexKey) element + int refCount = storagePoolRefCounts.computeIfAbsent(mutexKey, k -> 0); + refCount += adjustment; + if (refCount < 1) { + storagePoolRefCounts.remove(mutexKey); + } else { + storagePoolRefCounts.put(mutexKey, refCount); + } + return refCount; + } + } + /** + * Thread-safe increment storage pool usage refcount + * @param uuid UUID of the storage pool to increment the count + */ + private void incStoragePoolRefCount(String uuid) { + adjustStoragePoolRefCount(uuid, 1); + } + /** + * Thread-safe decrement storage pool usage refcount for the given uuid and return if storage pool still in use. + * @param uuid UUID of the storage pool to decrement the count + * @return true if the storage pool is still used, else false. + */ + private boolean decStoragePoolRefCount(String uuid) { + return adjustStoragePoolRefCount(uuid, -1) > 0; + } + @Override - public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map details) { + public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map details, boolean isPrimaryStorage) { s_logger.info("Attempting to create storage pool " + name + " (" + type.toString() + ") in libvirt"); StoragePool sp = null; @@ -744,6 +782,12 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri } try { + if (!isPrimaryStorage) { + // only ref count storage pools for secondary storage, as primary storage is assumed + // to be always mounted, as long the primary storage isn't fully deleted. + incStoragePoolRefCount(name); + } + if (sp.isActive() == 0) { s_logger.debug("Attempting to activate pool " + name); sp.create(0); @@ -755,6 +799,7 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri return getStoragePool(name); } catch (LibvirtException e) { + decStoragePoolRefCount(name); String error = e.toString(); if (error.contains("Storage source conflict")) { throw new CloudRuntimeException("A pool matching this location already exists in libvirt, " + @@ -805,6 +850,13 @@ private boolean destroyStoragePoolHandleException(Connect conn, String uuid) @Override public boolean deleteStoragePool(String uuid) { s_logger.info("Attempting to remove storage pool " + uuid + " from libvirt"); + + // decrement and check if storage pool still in use + if (decStoragePoolRefCount(uuid)) { + s_logger.info(String.format("deleteStoragePool: Storage pool %s still in use", uuid)); + return true; + } + Connect conn; try { conn = LibvirtConnection.getConnection(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java index b23dd9a17904..cb285383b9bc 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java @@ -55,7 +55,7 @@ public ManagedNfsStorageAdaptor(StorageLayer storagelayer) { } @Override - public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details) { + public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details, boolean isPrimaryStorage) { LibvirtStoragePool storagePool = new LibvirtStoragePool(uuid, path, StoragePoolType.ManagedNFS, this, null); storagePool.setSourceHost(host); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java index 5bcb6e48d974..0d58bdaa1777 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java @@ -168,7 +168,7 @@ private KVMPhysicalDisk getPhysicalDisk(AddressInfo address, KVMStoragePool pool } @Override - public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map details) { + public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map details, boolean isPrimaryStorage) { LOGGER.info(String.format("createStoragePool(uuid,host,port,path,type) called with args (%s, %s, %s, %s, %s)", uuid, host, ""+port, path, type)); MultipathSCSIPool storagePool = new MultipathSCSIPool(uuid, host, port, path, type, details, this); MapStorageUuidToStoragePool.put(uuid, storagePool); 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 60986f198a8b..bb5ba08a4215 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 @@ -143,7 +143,7 @@ public KVMPhysicalDisk getPhysicalDisk(String volumePath, KVMStoragePool pool) { } @Override - public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map details) { + public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map details, boolean isPrimaryStorage) { ScaleIOStoragePool storagePool = new ScaleIOStoragePool(uuid, host, port, path, type, details, this); MapStorageUuidToStoragePool.put(uuid, storagePool); return storagePool; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java index 80e73e01a863..d1a1ed9a2ecd 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java @@ -38,7 +38,7 @@ public interface StorageAdaptor { // it with info from local disk, and return it public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool); - public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map details); + public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map details, boolean isPrimaryStorage); public boolean deleteStoragePool(String uuid); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java index 3a7491ec542e..c2bbff7efb0c 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java @@ -86,6 +86,6 @@ public void testCreateStoragePoolWithNFSMountOpts() throws Exception { Map details = new HashMap<>(); details.put("nfsmountopts", "vers=4.1, nconnect=4"); - KVMStoragePool pool = libvirtStorageAdaptor.createStoragePool(uuid, null, 0, dir, null, Storage.StoragePoolType.NetworkFilesystem, details); + KVMStoragePool pool = libvirtStorageAdaptor.createStoragePool(uuid, null, 0, dir, null, Storage.StoragePoolType.NetworkFilesystem, details, true); } } 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 index fb00875bb3d1..bc2a3a42d971 100644 --- 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 @@ -149,7 +149,7 @@ public KVMPhysicalDisk getPhysicalDisk(String name, KVMStoragePool pool) @Override public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, - Storage.StoragePoolType type, Map details) + Storage.StoragePoolType type, Map details, boolean isPrimaryStorage) { s_logger.debug(String.format( "Linstor createStoragePool: name: '%s', host: '%s', path: %s, userinfo: %s", name, host, path, userInfo)); diff --git a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java index 273f088d77f9..c7711c05ef57 100644 --- a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java +++ b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java @@ -57,7 +57,7 @@ public static void SP_LOG(String fmt, Object... args) { private static final Map storageUuidToStoragePool = new HashMap(); @Override - public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details) { + public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map details, boolean isPrimaryStorage) { SP_LOG("StorPoolStorageAdaptor.createStoragePool: uuid=%s, host=%s:%d, path=%s, userInfo=%s, type=%s", uuid, host, port, path, userInfo, storagePoolType); StorPoolStoragePool storagePool = new StorPoolStoragePool(uuid, host, port, storagePoolType, this);