In commit 4090e15399 we went back from reporting no errors if no storage pools were found on a given host to reporting a bad error. And only in cases when gluster was not installed. Report a less bad error in case there are no volumes. Also report the error when gluster is installed but no volumes were found, since virStorageBackendFindGlusterPoolSources would return success in that case. --- src/storage/storage_backend.c | 19 ++++++++++++------- src/storage/storage_backend_fs.c | 20 ++++++++++++++------ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 18433e9c7..6c9706897 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2549,6 +2549,16 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, #ifdef GLUSTER_CLI +/** + * virStorageBackendFindGlusterPoolSources: + * @host: host to detect volumes on + * @pooltype: src->format is set to this value + * @list: list of storage pool sources to be filled + * + * Looks up gluster volumes on @host and fills them to @list. + * + * Returns number of volumes on the host on success, or -1 on error. + */ int virStorageBackendFindGlusterPoolSources(const char *host, int pooltype, @@ -2578,8 +2588,6 @@ virStorageBackendFindGlusterPoolSources(const char *host, goto cleanup; if (rc != 0) { - VIR_INFO("failed to query host '%s' for gluster volumes: %s", - host, outbuf); ret = 0; goto cleanup; } @@ -2588,11 +2596,8 @@ virStorageBackendFindGlusterPoolSources(const char *host, &ctxt))) goto cleanup; - if ((nnodes = virXPathNodeSet("//volumes/volume", ctxt, &nodes)) <= 0) { - VIR_INFO("no gluster volumes available on '%s'", host); - ret = 0; + if ((nnodes = virXPathNodeSet("//volumes/volume", ctxt, &nodes)) < 0) goto cleanup; - } for (i = 0; i < nnodes; i++) { ctxt->node = nodes[i]; @@ -2616,7 +2621,7 @@ virStorageBackendFindGlusterPoolSources(const char *host, src->format = pooltype; } - ret = 0; + ret = nnodes; cleanup: VIR_FREE(nodes); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index f4341f32c..b915fd58b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -281,7 +281,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE virStoragePoolSourcePtr source = NULL; char *ret = NULL; size_t i; - int retNFS = -1, retGluster = -1; + int retNFS = -1; + int retGluster = 0; virCheckFlags(0, NULL); @@ -306,14 +307,21 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE retNFS = virStorageBackendFileSystemNetFindNFSPoolSources(&state); # ifdef GLUSTER_CLI - retGluster = - virStorageBackendFindGlusterPoolSources(state.host, - VIR_STORAGE_POOL_NETFS_GLUSTERFS, - &state.list); + retGluster = virStorageBackendFindGlusterPoolSources(state.host, + VIR_STORAGE_POOL_NETFS_GLUSTERFS, + &state.list); + + if (retGluster < 0) + goto cleanup; + # endif /* If both fail, then we won't return an empty list - return an error */ - if (retNFS < 0 && retGluster < 0) + if (retNFS < 0 && retGluster == 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("no storage pools were found on host '%s'"), + state.host); goto cleanup; + } if (!(ret = virStoragePoolSourceListFormat(&state.list))) goto cleanup; -- 2.11.0

Similarly to the 'netfs' pool, return an error if the host does not have any volumes. --- src/storage/storage_backend_gluster.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index ae0611543..0bd40f742 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -490,6 +490,7 @@ virStorageBackendGlusterFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, }; virStoragePoolSourcePtr source = NULL; char *ret = NULL; + int rc; size_t i; virCheckFlags(0, NULL); @@ -510,11 +511,18 @@ virStorageBackendGlusterFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - if (virStorageBackendFindGlusterPoolSources(source->hosts[0].name, - 0, /* currently ignored */ - &list) < 0) + if ((rc = virStorageBackendFindGlusterPoolSources(source->hosts[0].name, + 0, /* currently ignored */ + &list)) < 0) goto cleanup; + if (rc == 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("no storage pools were found on host '%s'"), + source->hosts[0].name); + goto cleanup; + } + if (!(ret = virStoragePoolSourceListFormat(&list))) goto cleanup; -- 2.11.0

The tool is used for pool discovery. Since we call an external binary we don't really need to compile out the code that uses it. We can check whether it exists at runtime. --- configure.ac | 2 +- src/storage/storage_backend.c | 29 ++++++++++++++++------------- src/storage/storage_backend.h | 3 ++- src/storage/storage_backend_fs.c | 4 +--- src/storage/storage_backend_gluster.c | 2 +- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/configure.ac b/configure.ac index c67ba79c7..9e41f8983 100644 --- a/configure.ac +++ b/configure.ac @@ -605,7 +605,7 @@ LIBVIRT_STORAGE_CHECK_ZFS if test "$with_storage_fs" = "yes" || test "$with_storage_gluster" = "yes"; then - AC_PATH_PROG([GLUSTER_CLI], [gluster], [], [$LIBVIRT_SBIN_PATH]) + AC_PATH_PROG([GLUSTER_CLI], [gluster], [gluster], [$LIBVIRT_SBIN_PATH]) if test "x$GLUSTER_CLI" != "x"; then AC_DEFINE_UNQUOTED([GLUSTER_CLI], ["$GLUSTER_CLI"], [Location or name of the gluster command line tool]) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 6c9706897..65a893679 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2548,12 +2548,12 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, } -#ifdef GLUSTER_CLI /** * virStorageBackendFindGlusterPoolSources: * @host: host to detect volumes on * @pooltype: src->format is set to this value * @list: list of storage pool sources to be filled + * @report: report error if the 'gluster' cli tool is missing * * Looks up gluster volumes on @host and fills them to @list. * @@ -2562,8 +2562,10 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, int virStorageBackendFindGlusterPoolSources(const char *host, int pooltype, - virStoragePoolSourceListPtr list) + virStoragePoolSourceListPtr list, + bool report) { + char *glusterpath = NULL; char *outbuf = NULL; virCommandPtr cmd = NULL; xmlDocPtr doc = NULL; @@ -2576,7 +2578,17 @@ virStorageBackendFindGlusterPoolSources(const char *host, int ret = -1; - cmd = virCommandNewArgList(GLUSTER_CLI, + if (!(glusterpath = virFindFileInPath(GLUSTER_CLI))) { + if (report) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("'gluster' command line tool not found")); + return -1; + } else { + return 0; + } + } + + cmd = virCommandNewArgList(glusterpath, "--xml", "--log-file=/dev/null", "volume", "info", "all", NULL); @@ -2629,18 +2641,9 @@ virStorageBackendFindGlusterPoolSources(const char *host, xmlFreeDoc(doc); VIR_FREE(outbuf); virCommandFree(cmd); + VIR_FREE(glusterpath); return ret; } -#else /* #ifdef GLUSTER_CLI */ -int -virStorageBackendFindGlusterPoolSources(const char *host ATTRIBUTE_UNUSED, - int pooltype ATTRIBUTE_UNUSED, - virStoragePoolSourceListPtr list ATTRIBUTE_UNUSED) -{ - VIR_INFO("gluster cli tool not installed"); - return 0; -} -#endif /* #ifdef GLUSTER_CLI */ #if WITH_BLKID diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 7ae8d58db..3f0403907 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -135,7 +135,8 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, int virStorageBackendFindGlusterPoolSources(const char *host, int pooltype, - virStoragePoolSourceListPtr list); + virStoragePoolSourceListPtr list, + bool report); int virStorageBackendVolUploadLocal(virConnectPtr conn, virStoragePoolObjPtr pool, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index b915fd58b..53c34335f 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -306,15 +306,13 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE retNFS = virStorageBackendFileSystemNetFindNFSPoolSources(&state); -# ifdef GLUSTER_CLI retGluster = virStorageBackendFindGlusterPoolSources(state.host, VIR_STORAGE_POOL_NETFS_GLUSTERFS, - &state.list); + &state.list, false); if (retGluster < 0) goto cleanup; -# endif /* If both fail, then we won't return an empty list - return an error */ if (retNFS < 0 && retGluster == 0) { virReportError(VIR_ERR_OPERATION_FAILED, diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 0bd40f742..e819f6299 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -513,7 +513,7 @@ virStorageBackendGlusterFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, if ((rc = virStorageBackendFindGlusterPoolSources(source->hosts[0].name, 0, /* currently ignored */ - &list)) < 0) + &list, true)) < 0) goto cleanup; if (rc == 0) { -- 2.11.0

The gluster command line tool that is used to lookup storage pool sources moved from the gluster-client package to gluster-cli. Unfortunately the location differs between fedora and rhel so depend on the package rather than binary. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1349441 --- libvirt.spec.in | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 51ffbb55c..a76f6a9cf 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -595,6 +595,10 @@ Requires: util-linux %if 0%{?fedora} Requires: glusterfs-client >= 2.0.1 %endif +# gluster cli tool for pool discovery +%if (0%{?fedora} || 0%{?with_storage_gluster}) +Requires: gluster-cli +%endif # For LVM drivers Requires: lvm2 # For ISCSI driver -- 2.11.0