[libvirt] [PATCH 3/6] storage: Handle transient pool removal in driver startup

[Ján Tomko]

On Wed, Jun 22, 2016 at 08:12:13PM -0400, Cole Robinson wrote:
>Currently transient storage pools can end up in a shutoff state
>via driver startup
>
>https://bugzilla.redhat.com/show_bug.cgi?id=1227475
>
>Use storagePoolSetInactive to handle transient pool removal in
>this case. There's some weirdness here though because this can
>happen while we are iterating over the storage pool list
>---
> src/storage/storage_driver.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
>diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
>index c7ffea8..45f00e0 100644
>--- a/src/storage/storage_driver.c
>+++ b/src/storage/storage_driver.c
>@@ -102,12 +102,16 @@ storagePoolSetInactive(virStoragePoolObjPtr pool)
>     return ret;
> }
>
>-static void
>+/*
>+ * Returns true if pool was removed from driver->pools, via
>+ * storagePoolSetInactive
>+ */
>+static bool

> storagePoolUpdateState(virStoragePoolObjPtr pool)
> {
>-    bool active;
>+    bool active = false;
>     virStorageBackendPtr backend;
>-    int ret = -1;
>+    bool ret = false;
>     char *stateFile;
>
>     if (!(stateFile = virFileBuildPath(driver->stateDir,
>@@ -123,7 +127,6 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
>     /* Backends which do not support 'checkPool' are considered
>      * inactive by default.
>      */
>-    active = false;
>     if (backend->checkPool &&
>         backend->checkPool(pool, &active) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>@@ -149,15 +152,15 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
>     }
>
>     pool->active = active;
>-    ret = 0;
>  error:
>-    if (ret < 0) {
>+    if (!active) {
>+        ret = storagePoolSetInactive(pool);
>         if (stateFile)
>             unlink(stateFile);
>     }
>     VIR_FREE(stateFile);
>
>-    return;
>+    return ret;
> }
>
> static void
>@@ -169,7 +172,13 @@ storagePoolUpdateAllState(void)
>         virStoragePoolObjPtr pool = driver->pools.objs[i];
>
>         virStoragePoolObjLock(pool);
>-        storagePoolUpdateState(pool);
>+        if (storagePoolUpdateState(pool)) {

UpdateState should update the state, not remove pools from the list.

If it marks it as inactive, then we'd call storagePoolSetInactive
here, or even better, a subset of it since we do not need to iterate
over the pool array to find the pool index - we already have it.

Jan
>+            /* The transient pool was removed from the list, while we
>+               are iterating over the list. Adjust the counter backwards
>+               to match */
>+            --i;
>+            continue;
>+        }
>         virStoragePoolObjUnlock(pool);
>     }
> }
>-- 
>2.7.4
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list