From 062bb06baac7f5f068539d8f9d7a7b57d6021ec2 Mon Sep 17 00:00:00 2001 From: Ramgopal Nagaboina Date: Wed, 9 Sep 2026 13:00:44 -0400 Subject: [PATCH] kvm: remount NFS primary storage left unmounted after cancelling maintenance When primary storage is taken out of maintenance, the KVM agent can find the libvirt storage pool still defined and active and reuse it, even though the NFS share was unmounted while in maintenance. The pool then stays logically Up while its mount point is gone on the host, and operations fail with "libvirt failed to mount storage pool". For a NetworkFilesystem pool that libvirt reports active, also check that the target path is actually mounted; if it is not, undefine the pool so the existing create path recreates and remounts it. Fixes: #12690 --- .../kvm/storage/LibvirtStorageAdaptor.java | 16 +++++++-- .../storage/LibvirtStorageAdaptorTest.java | 36 +++++++++++++++++-- 2 files changed, 46 insertions(+), 6 deletions(-) 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