DaanHoogland commented on code in PR #9498:
URL: https://github.com/apache/cloudstack/pull/9498#discussion_r1760878307
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java:
##########
@@ -637,6 +638,38 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid,
KVMStoragePool pool) {
}
}
+ /**
+ * Thread-safe increment storage pool usage refcount
+ * @param uuid UUID of the storage pool to increment the count
+ */
+ private void incStoragePoolRefCount(String uuid) {
+ synchronized (storagePoolRefCounts) {
+ int refCount = storagePoolRefCounts.computeIfAbsent(uuid, k -> 0);
+ refCount += 1;
+ storagePoolRefCounts.put(uuid, refCount);
+ }
+ }
+
+ /**
+ * 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) {
+ synchronized (storagePoolRefCounts) {
+ Integer refCount = storagePoolRefCounts.get(uuid);
+ if (refCount != null && refCount > 1) {
+ s_logger.debug(String.format("Storage pool %s still in use,
refCount %d", uuid, refCount));
+ refCount -= 1;
+ storagePoolRefCounts.put(uuid, refCount);
+ return true;
+ } else {
+ storagePoolRefCounts.remove(uuid);
+ return false;
+ }
+ }
+ }
Review Comment:
@rp- , should inc and dec maybe both call the same bit of synchronised code
so that no inc and dec can happen at the same time as well? The
`storagePoolRefCounts` can be a synchronised map to prevent strangeties
happening on the map, but we also want to prevent multiple access on the
elements.
```suggestion
/**
* adjust refcount
*/
private int adjustStoragePoolRefCount(Sting uuid, int adjustment) {
synchronised(uuid) {
// some access on the storagePoolRefCounts.key(uuid) element
Integer refCount = storagePoolRefCounts.computeIfAbsent(uuid, k
-> 0);
refCount += adjustment;
storagePoolRefCounts.put(uuid, refCount);
if (refCount < 1) {
storagePoolRefCounts.remove(uuid);
} else {
storagePoolRefCounts.put(uuid, 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;
}
```
???
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
