Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvm: ref-count storage pool usage #9498

Open
wants to merge 1 commit into
base: 4.19
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
private static final Map<String, KVMStoragePool> MapStorageUuidToStoragePool = new HashMap<>();

@Override
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map<String, String> details) {
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map<String, String> details, boolean isPrimaryStorage) {

Check warning on line 46 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L46 was not covered by tests
IscsiAdmStoragePool storagePool = new IscsiAdmStoragePool(uuid, host, port, storagePoolType, this);

MapStorageUuidToStoragePool.put(uuid, storagePool);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@
//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<String, String> 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);

Check warning on line 364 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L364 was not covered by tests

// LibvirtStorageAdaptor-specific statement
if (pool.isPoolSupportHA() && primaryStorage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand All @@ -80,6 +81,7 @@
private StorageLayer _storageLayer;
private String _mountPoint = "/mnt";
private String _manageSnapshotPath;
private static final ConcurrentHashMap<String, Integer> storagePoolRefCounts = new ConcurrentHashMap<>();

private String rbdTemplateSnapName = "cloudstack-base-snap";
private static final int RBD_FEATURE_LAYERING = 1;
Expand Down Expand Up @@ -637,8 +639,44 @@
}
}

/**
* 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) {

Check warning on line 650 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L645-L650

Added lines #L645 - L650 were not covered by tests
// some access on the storagePoolRefCounts.key(mutexKey) element
int refCount = storagePoolRefCounts.computeIfAbsent(mutexKey, k -> 0);
refCount += adjustment;

Check warning on line 653 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L652-L653

Added lines #L652 - L653 were not covered by tests
if (refCount < 1) {
storagePoolRefCounts.remove(mutexKey);

Check warning on line 655 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L655

Added line #L655 was not covered by tests
} else {
storagePoolRefCounts.put(mutexKey, refCount);

Check warning on line 657 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L657

Added line #L657 was not covered by tests
}
return refCount;

Check warning on line 659 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L659

Added line #L659 was not covered by tests
}
}

Check warning on line 661 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L661

Added line #L661 was not covered by tests
/**
* 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);
}

Check warning on line 668 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L666-L668

Added lines #L666 - L668 were not covered by tests
/**
* 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) {

Check warning on line 674 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L674

Added line #L674 was not covered by tests
return adjustStoragePoolRefCount(uuid, -1) > 0;
}

Check warning on line 676 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L676

Added line #L676 was not covered by tests

@Override
public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map<String, String> details) {
public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map<String, String> details, boolean isPrimaryStorage) {
s_logger.info("Attempting to create storage pool " + name + " (" + type.toString() + ") in libvirt");

StoragePool sp = null;
Expand Down Expand Up @@ -744,6 +782,12 @@
}

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);

Check warning on line 788 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L788

Added line #L788 was not covered by tests
}

if (sp.isActive() == 0) {
s_logger.debug("Attempting to activate pool " + name);
sp.create(0);
Expand All @@ -755,6 +799,7 @@

return getStoragePool(name);
} catch (LibvirtException e) {
decStoragePoolRefCount(name);

Check warning on line 802 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L802

Added line #L802 was not covered by tests
String error = e.toString();
if (error.contains("Storage source conflict")) {
throw new CloudRuntimeException("A pool matching this location already exists in libvirt, " +
Expand Down Expand Up @@ -805,6 +850,13 @@
@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;

Check warning on line 857 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L856-L857

Added lines #L856 - L857 were not covered by tests
}

Connect conn;
try {
conn = LibvirtConnection.getConnection();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
}

@Override
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map<String, String> details) {
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map<String, String> details, boolean isPrimaryStorage) {

Check warning on line 58 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java#L58

Added line #L58 was not covered by tests

LibvirtStoragePool storagePool = new LibvirtStoragePool(uuid, path, StoragePoolType.ManagedNFS, this, null);
storagePool.setSourceHost(host);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
}

@Override
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map<String, String> details) {
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map<String, String> details, boolean isPrimaryStorage) {

Check warning on line 171 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java#L171

Added line #L171 was not covered by tests
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@
}

@Override
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map<String, String> details) {
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, Storage.StoragePoolType type, Map<String, String> details, boolean isPrimaryStorage) {

Check warning on line 146 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L146 was not covered by tests
ScaleIOStoragePool storagePool = new ScaleIOStoragePool(uuid, host, port, path, type, details, this);
MapStorageUuidToStoragePool.put(uuid, storagePool);
return storagePool;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> details);
public KVMStoragePool createStoragePool(String name, String host, int port, String path, String userInfo, StoragePoolType type, Map<String, String> details, boolean isPrimaryStorage);

public boolean deleteStoragePool(String uuid);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,6 @@ public void testCreateStoragePoolWithNFSMountOpts() throws Exception {

Map<String, String> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> details)
Storage.StoragePoolType type, Map<String, String> details, boolean isPrimaryStorage)
{
s_logger.debug(String.format(
"Linstor createStoragePool: name: '%s', host: '%s', path: %s, userinfo: %s", name, host, path, userInfo));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
private static final Map<String, KVMStoragePool> storageUuidToStoragePool = new HashMap<String, KVMStoragePool>();

@Override
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map<String, String> details) {
public KVMStoragePool createStoragePool(String uuid, String host, int port, String path, String userInfo, StoragePoolType storagePoolType, Map<String, String> details, boolean isPrimaryStorage) {

Check warning on line 60 in plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java#L60

Added line #L60 was not covered by tests
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);
Expand Down
Loading