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

From: Eric Benner <ebenner(a)vultr.com&gt; 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(a)vultr.com&gt; --- 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