[PATCH] storage: netfs: Handle backend errors

[John Ferlan <jferlan@xxxxxxxxxx>]

Commit id '18642d10' caused a virt-test regression for NFS backend
storage error path checks when running the command:

    'virsh find-storage-pool-sources-as netfs Unknown  '

when the host did not have Gluster installed. Prior to the commit,
the test would fail with the error:

    error: internal error: Child process (/usr/sbin/showmount --no-headers
    --exports Unknown) unexpected exit status 1: clnt_create: RPC: Unknown host

After the commit, the error would be ignored, the call would succeed,
and an empty list of pool sources returned. This was tucked into the
commit message as an expected outcome.

When the target host does not have a GLUSTER_CLI this is a regression
over the previous release. Furthermore, even if Gluster CLI was present,
but had a failure to get devices, the API would return a failure even if
the NFS backend had found devices.

Add code to handle the error conditions.
  - If NFS fails
    - If there is no Gluster CLI, then jump to cleanup/failure.
    - If there is a Gluster CLI, then message the NFS
      failure and continue with the Gluster checks.

  - If Gluster fails
    - If NFS had failed, then jump to cleanup/failure.
    - Message the Gluster failure and continue on.

  - If only one fails, fetch and return a list of source devices
    even if it's empty

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/storage/storage_backend.c    | 14 +++++++++++++
 src/storage/storage_backend.h    |  1 +
 src/storage/storage_backend_fs.c | 43 +++++++++++++++++++++++++++++++++++-----
 3 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index b56fefe..4d73902 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1643,6 +1643,13 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool,
 }
 
 #ifdef GLUSTER_CLI
+bool
+virStorageBackendHaveGlusterCLI(void)
+{
+    return true;
+}
+
+
 int
 virStorageBackendFindGlusterPoolSources(const char *host,
                                         int pooltype,
@@ -1721,6 +1728,13 @@ virStorageBackendFindGlusterPoolSources(const char *host,
     return ret;
 }
 #else /* #ifdef GLUSTER_CLI */
+bool
+virStorageBackendHaveGlusterCLI(void)
+{
+    return false;
+}
+
+
 int
 virStorageBackendFindGlusterPoolSources(const char *host ATTRIBUTE_UNUSED,
                                         int pooltype ATTRIBUTE_UNUSED,
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 4f95000..41eed97 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -87,6 +87,7 @@ int virStorageBackendFindFSImageTool(char **tool);
 virStorageBackendBuildVolFrom
 virStorageBackendFSImageToolTypeToFunc(int tool_type);
 
+bool virStorageBackendHaveGlusterCLI(void);
 int virStorageBackendFindGlusterPoolSources(const char *host,
                                             int pooltype,
                                             virStoragePoolSourceListPtr list);
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4e4a7ae..f3d4a6d 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -238,9 +238,11 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(char **const groups,
 }
 
 
-static void
+static int
 virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state)
 {
+    int ret = -1;
+
     /*
      *  # showmount --no-headers -e HOSTNAME
      *  /tmp   *
@@ -267,9 +269,13 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state)
     if (virCommandRunRegex(cmd, 1, regexes, vars,
                            virStorageBackendFileSystemNetFindPoolSourcesFunc,
                            state, NULL) < 0)
-        virResetLastError();
+        goto cleanup;
 
+    ret = 0;
+
+ cleanup:
     virCommandFree(cmd);
+    return ret;
 }
 
 
@@ -289,6 +295,7 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE
     virStoragePoolSourcePtr source = NULL;
     char *ret = NULL;
     size_t i;
+    bool failNFS = false;
 
     virCheckFlags(0, NULL);
 
@@ -310,12 +317,38 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE
 
     state.host = source->hosts[0].name;
 
-    virStorageBackendFileSystemNetFindNFSPoolSources(&state);
+    if (virStorageBackendFileSystemNetFindNFSPoolSources(&state) < 0) {
+        virErrorPtr err;
+        /* If no Gluster CLI, then force error right here */
+        if (!virStorageBackendHaveGlusterCLI())
+            goto cleanup;
+
+        /* If we have a Gluster CLI, then message the error, clean it out,
+         * and move onto the Gluster code
+         */
+        err = virGetLastError();
+        VIR_ERROR(_("Failed to get NFS pool sources: '%s'"),
+                  err != NULL ? err->message: _("unknown error"));
+        virResetLastError();
+        failNFS = true;
+    }
 
     if (virStorageBackendFindGlusterPoolSources(state.host,
                                                 VIR_STORAGE_POOL_NETFS_GLUSTERFS,
-                                                &state.list) < 0)
-        goto cleanup;
+                                                &state.list) < 0) {
+        virErrorPtr err;
+        /* If NFS failed as well, then force the error right here */
+        if (failNFS)
+            goto cleanup;
+
+        /* Otherwise, NFS passed, so we message the Gluster error, clean
+         * it out, and generate the source list (even if it's empty)
+         */
+        err = virGetLastError();
+        VIR_ERROR(_("Failed to get Gluster pool sources: '%s'"),
+                  err != NULL ? err->message: _("unknown error"));
+        virResetLastError();
+    }
 
     if (!(ret = virStoragePoolSourceListFormat(&state.list)))
         goto cleanup;
-- 
1.9.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list