[PATCH] Re: storage: Cleaning up pools is not always ideal

[ebenner at vultr.com]

From: Eric Benner <ebenner at vultr.com>

Didn't have a sign off and was formatted by a mail client. This is a functional copy of this patch with the requirements.

Signed-off-by: Eric Benner <ebenner at vultr.com>
---
 src/remote/libvirtd.conf.in  |  7 +++++++
 src/storage/storage_driver.c | 38 ++++++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in
index 34741183cc..5600c26eca 100644
--- a/src/remote/libvirtd.conf.in
+++ b/src/remote/libvirtd.conf.in
@@ -506,3 +506,10 @@
 # potential infinite waits blocking libvirt.
 #
 #ovs_timeout = 5
+
+###################################################################
+# This decides whether to disable pools that errors in some way such as during a refresh
+# This can negatively affect LVM pools. 1 = disable the pools, 2 = don't disable the pools
+# Do not use this if you are not using only LVM pools
+#
+cleanup_pools = 1
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 0bb116cf08..5f8bc8a835 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -55,6 +55,7 @@
 VIR_LOG_INIT("storage.storage_driver");
 
 static virStorageDriverStatePtr driver;
+static int cleanup_pools = -1;
 
 static int storageStateCleanup(void);
 
@@ -74,6 +75,20 @@ static void storageDriverUnlock(void)
     virMutexUnlock(&driver->lock);
 }
 
+static int cleanupPools(void)
+{
+    if (cleanup_pools == -1) {
+        g_autoptr(virConf) libvirtConf = NULL;
+        virConfLoadConfig(&libvirtConf, "libvirtd.conf");
+
+        if (!virConfGetValueInt(libvirtConf, "cleanup_pools", &cleanup_pools)
+            || (cleanup_pools != 0 && cleanup_pools != 1))
+            cleanup_pools = 1;
+    }
+
+    return cleanup_pools;
+}
+
 
 static void
 storagePoolRefreshFailCleanup(virStorageBackendPtr backend,
@@ -81,14 +96,26 @@ storagePoolRefreshFailCleanup(virStorageBackendPtr backend,
                               const char *stateFile)
 {
     virErrorPtr orig_err;
-
     virErrorPreserveLast(&orig_err);
     virStoragePoolObjClearVols(obj);
 
     if (stateFile)
         unlink(stateFile);
-    if (backend->stopPool)
-        backend->stopPool(obj);
+
+    if (!cleanupPools()) {
+        virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to refresh storage pool '%s'. Would have disabled this pool but clean_pools = 0: %s"),
+                               def->name, virGetLastErrorMessage());
+    } else {
+        virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj);
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to refresh storage pool '%s'. Disabled this pool because clean_pools = 1: %s"),
+                               def->name, virGetLastErrorMessage());
+        if (backend->stopPool) {
+            backend->stopPool(obj);
+        }
+    }
     virErrorRestore(&orig_err);
 }
 
@@ -101,7 +128,10 @@ storagePoolRefreshImpl(virStorageBackendPtr backend,
     virStoragePoolObjClearVols(obj);
     if (backend->refreshPool(obj) < 0) {
         storagePoolRefreshFailCleanup(backend, obj, stateFile);
-        return -1;
+        if (!cleanupPools())
+            return 0;
+        else
+            return -1;
     }
 
     return 0;
-- 
2.24.1