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 8be5e9658fac..d783c1e11055 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 @@ -277,10 +277,14 @@ public void storagePoolRefresh(StoragePool pool) { } } - private void checkNetfsStoragePoolMounted(String uuid) { + private boolean isStoragePoolMounted(String uuid) { String targetPath = _mountPoint + File.separator + uuid; - int mountpointResult = Script.runSimpleBashScriptForExitValue("mountpoint -q " + targetPath); - if (mountpointResult != 0) { + return Script.runSimpleBashScriptForExitValue("mountpoint -q " + targetPath) == 0; + } + + private void checkNetfsStoragePoolMounted(String uuid) { + if (!isStoragePoolMounted(uuid)) { + String targetPath = _mountPoint + File.separator + uuid; String errMsg = String.format("libvirt failed to mount storage pool %s at %s", uuid, targetPath); logger.error(errMsg); throw new CloudRuntimeException(errMsg); @@ -749,6 +753,12 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri sp = null; logger.info("Found existing defined storage pool " + name + ". It wasn't running, so we undefined it."); } + if (sp != null && type == StoragePoolType.NetworkFilesystem && !isStoragePoolMounted(name)) { + sp.destroy(); + sp.undefine(); + sp = null; + logger.info("Found existing defined storage pool " + name + " but it is not mounted, so we undefined it to recreate and remount."); + } if (sp != null) { logger.info("Found existing defined storage pool " + name + ", using it."); } 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..f70e722ffbf3 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; @@ -76,7 +77,7 @@ public void tearDown() throws Exception { closeable.close(); } - @Test(expected = CloudRuntimeException.class) + @Test public void testCreateStoragePoolWithNFSMountOpts() throws Exception { LibvirtStoragePoolDef.PoolType type = LibvirtStoragePoolDef.PoolType.NETFS; String name = "Primary1"; @@ -97,11 +98,40 @@ public void testCreateStoragePoolWithNFSMountOpts() throws Exception { Mockito.when(conn.storagePoolLookupByUUIDString(uuid)).thenReturn(sp); Mockito.when(sp.isActive()).thenReturn(1); Mockito.when(sp.getXMLDesc(0)).thenReturn(poolXml); - Mockito.when(Script.runSimpleBashScriptForExitValue(anyString())).thenReturn(-1); + // pool is mounted, so it is reused (matching mount options) + Mockito.when(Script.runSimpleBashScriptForExitValue(anyString())).thenReturn(0); 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, true); + try { + libvirtStorageAdaptor.createStoragePool(uuid, null, 0, dir, null, Storage.StoragePoolType.NetworkFilesystem, details, true); + } catch (Exception ignored) { + // building the returned KVMStoragePool needs libvirt internals not mocked here + } + Mockito.verify(sp, Mockito.never()).destroy(); + Mockito.verify(sp, Mockito.never()).undefine(); + } + + @Test + public void testCreateStoragePoolRemountsActiveButUnmountedNfsPool() throws Exception { + String uuid = String.valueOf(UUID.randomUUID()); + String dir = "/export/primary"; + + 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); + // libvirt reports the pool active, but the mountpoint check fails: it is not actually mounted + Mockito.when(Script.runSimpleBashScriptForExitValue(anyString())).thenReturn(-1); + + Map details = new HashMap<>(); + // the recreate path is not fully mockable here; we only assert the recovery decision + Assert.assertThrows(Exception.class, () -> + libvirtStorageAdaptor.createStoragePool(uuid, null, 0, dir, null, Storage.StoragePoolType.NetworkFilesystem, details, true)); + + Mockito.verify(sp).destroy(); + Mockito.verify(sp).undefine(); } @Test