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 c949d0882479..85d9d36c3bbd 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 @@ -918,11 +918,15 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri throw new CloudRuntimeException("Failed to create storage pool: " + name); } + boolean refCountIncremented = false; + boolean operationSucceeded = false; + 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); + refCountIncremented = true; } if (sp.isActive() == 0) { @@ -934,17 +938,24 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri checkNetfsStoragePoolMounted(name); } - return getStoragePool(name); + KVMStoragePool storagePool = getStoragePool(name); + operationSucceeded = true; + return storagePool; + } 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, " + - " but has a different UUID/Name. Cannot create new pool without first " + " removing it. Check for inactive pools via 'virsh pool-list --all'. " + + " but has a different UUID/Name. Cannot create new pool without first " + + " removing it. Check for inactive pools via 'virsh pool-list --all'. " + error); } else { throw new CloudRuntimeException(error); } + } finally { + if (refCountIncremented && !operationSucceeded) { + decStoragePoolRefCount(name); + } } } 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 88346abd0176..f2ef7781c558 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 @@ -27,6 +27,7 @@ import java.util.UUID; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -176,4 +177,38 @@ public void testUpdateLocalPoolIops_NullResultFromScript() { Mockito.verify(mockPool, never()).setUsedIops(anyLong()); } + @Test + public void testCreateStoragePoolDecrementsRefCountWhenGetStoragePoolFails() throws Exception { + String uuid = String.valueOf(UUID.randomUUID()); + + Connect conn = Mockito.mock(Connect.class); + StoragePool sp = Mockito.mock(StoragePool.class); + + Mockito.when(LibvirtConnection.getConnection()).thenReturn(conn); + Mockito.when(conn.storagePoolLookupByUUIDString(uuid)).thenReturn(sp); + Mockito.when(sp.isActive()).thenReturn(1); + + LibvirtStorageAdaptor adaptor = Mockito.spy(new LibvirtStorageAdaptor(null)); + + Mockito.doThrow(new CloudRuntimeException("Simulated storage pool failure")) + .when(adaptor).getStoragePool(uuid); + + try { + adaptor.createStoragePoolLocked( + uuid, + "127.0.0.1", + 0, + "/export/secondary", + null, + Storage.StoragePoolType.Filesystem, + null, + false + ); + + Assert.fail("Expected CloudRuntimeException"); + } catch (CloudRuntimeException expected) { + Mockito.verify(adaptor).incStoragePoolRefCount(uuid); + Mockito.verify(adaptor).decStoragePoolRefCount(uuid); + } + } }