rp- commented on code in PR #9498:
URL: https://github.com/apache/cloudstack/pull/9498#discussion_r1762595557
##########
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:
1. Sure I can also use concurrenthashmap
2. Sure if you leak the elements out of the synchronization blocks they are
not thread safe anymore, but the current code doesn't do that.
3. As far as I read, concurrenthashmap only locks on element level, so
switching to that would already be good.
--
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
