The <source> elements need to be indented from <sources> elements. --- src/conf/storage_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a300476..65504b4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -2006,11 +2006,13 @@ virStoragePoolSourceListFormat(virStoragePoolSourceListPtr def) } virBufferAddLit(&buf, "<sources>\n"); + virBufferAdjustIndent(&buf, 2); for (i = 0; i < def->nsources; i++) { virStoragePoolSourceFormat(&buf, options, &def->sources[i]); } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</sources>\n"); if (virBufferError(&buf)) -- 1.9.1

https://bugzilla.redhat.com/show_bug.cgi?id=1072714 Use the "gluster" command line tool to retrieve information about remote volumes on a gluster server to allow storage pool source lookup. Unfortunately gluster doesn't provide a management library so that we could use that directly, instead the RPC calls are hardcoded in the command line tool. --- configure.ac | 6 +++ src/storage/storage_backend.c | 86 ++++++++++++++++++++++++++++++++++++++++ src/storage/storage_backend.h | 4 ++ src/storage/storage_backend_fs.c | 5 +++ 4 files changed, 101 insertions(+) diff --git a/configure.ac b/configure.ac index 73efffa..b60da1c 100644 --- a/configure.ac +++ b/configure.ac @@ -1936,6 +1936,12 @@ if test "$with_storage_gluster" = "yes"; then fi AM_CONDITIONAL([WITH_STORAGE_GLUSTER], [test "$with_storage_gluster" = "yes"]) +if test "$with_storage_fs" = "yes" || + test "$with_storage_gluster" = "yes"; then + AC_PATH_PROG([GLUSTER_CLI], [gluster], [], [$PATH:/sbin:/usr/sbin]) + AC_DEFINE_UNQUOTED([GLUSTER_CLI], ["$GLUSTER_CLI"], + [Location or name of the gluster command line tool]) +fi LIBPARTED_CFLAGS= LIBPARTED_LIBS= diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5b3b536..c7e4688 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1640,3 +1640,89 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, return stablepath; } + +#ifdef GLUSTER_CLI +int +virStorageBackendFindGlusterPoolSources(const char *host, + int pooltype, + virStoragePoolSourceListPtr list) +{ + char *outbuf = NULL; + virCommandPtr cmd = NULL; + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; + xmlNodePtr *nodes = NULL; + virStoragePoolSource *src = NULL; + size_t i; + int nnodes; + int rc; + + int ret = -1; + + cmd = virCommandNewArgList(GLUSTER_CLI, + "--xml", + "--log-file=/dev/null", + "volume", "info", "all", NULL); + + virCommandAddArgFormat(cmd, "--remote-host=%s", host); + virCommandSetOutputBuffer(cmd, &outbuf); + + if (virCommandRun(cmd, &rc) < 0) + goto cleanup; + + if (rc != 0) { + VIR_INFO("failed to query host '%s' for gluster volumes: %s", + host, outbuf); + ret = 0; + goto cleanup; + } + + if (!(doc = virXMLParseStringCtxt(outbuf, _("(gluster_cli_output)"), + &ctxt))) + goto cleanup; + + if ((nnodes = virXPathNodeSet("//volumes/volume", ctxt, &nodes)) <= 0) { + VIR_INFO("no gluster volumes available on '%s'", host); + ret = 0; + goto cleanup; + } + + for (i = 0; i < nnodes; i++) { + ctxt->node = nodes[i]; + + if (!(src = virStoragePoolSourceListNewSource(list))) + goto cleanup; + + if (!(src->dir = virXPathString("string(//name)", ctxt))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to extract gluster volume name")); + goto cleanup; + } + + if (VIR_ALLOC_N(src->hosts, 1) < 0) + goto cleanup; + src->nhost = 1; + + if (VIR_STRDUP(src->hosts[0].name, host) < 0) + goto cleanup; + + src->format = pooltype; + } + + ret = 0; + + cleanup: + VIR_FREE(outbuf); + virCommandFree(cmd); + 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 */ diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 2034a22..042ecfa 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -87,6 +87,10 @@ int virStorageBackendFindFSImageTool(char **tool); virStorageBackendBuildVolFrom virStorageBackendFSImageToolTypeToFunc(int tool_type); +int virStorageBackendFindGlusterPoolSources(const char *host, + int pooltype, + virStoragePoolSourceListPtr list); + typedef struct _virStorageBackend virStorageBackend; typedef virStorageBackend *virStorageBackendPtr; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 8fb03c5..fd53691 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -316,6 +316,11 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE virStorageBackendFileSystemNetFindNFSPoolSources(&state); + if (virStorageBackendFindGlusterPoolSources(state.host, + VIR_STORAGE_POOL_NETFS_GLUSTERFS, + &state.list) < 0) + goto cleanup; + if (!(ret = virStoragePoolSourceListFormat(&state.list))) goto cleanup; -- 1.9.1

On 03/28/2014 11:01 PM, Peter Krempa wrote: These three are never freed. Jan

Use the previously implemented function to lookup glusterfs source pools for the netfs pool to lookup native gluster pools too. --- src/storage/storage_backend_gluster.c | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 9a6180e..5b79146 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -472,10 +472,60 @@ virStorageBackendGlusterVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, } +static char * +virStorageBackendGlusterFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *srcSpec, + unsigned int flags) +{ + virStoragePoolSourceList list = { .type = VIR_STORAGE_POOL_GLUSTER, + .nsources = 0, + .sources = NULL + }; + virStoragePoolSourcePtr source = NULL; + char *ret = NULL; + size_t i; + + virCheckFlags(0, NULL); + + if (!srcSpec) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("hostname must be specified for gluster sources")); + return NULL; + } + + if (!(source = virStoragePoolDefParseSourceString(srcSpec, + VIR_STORAGE_POOL_GLUSTER))) + return NULL; + + if (source->nhost != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Expected exactly 1 host for the storage pool")); + goto cleanup; + } + + if (virStorageBackendFindGlusterPoolSources(source->hosts[0].name, + 0, /* currently ignored */ + &list) < 0) + goto cleanup; + + if (!(ret = virStoragePoolSourceListFormat(&list))) + goto cleanup; + + cleanup: + for (i = 0; i < list.nsources; i++) + virStoragePoolSourceClear(&list.sources[i]); + VIR_FREE(list.sources); + + virStoragePoolSourceFree(source); + return ret; +} + + virStorageBackend virStorageBackendGluster = { .type = VIR_STORAGE_POOL_GLUSTER, .refreshPool = virStorageBackendGlusterRefreshPool, + .findPoolSources = virStorageBackendGlusterFindPoolSources, .deleteVol = virStorageBackendGlusterVolDelete, }; -- 1.9.1

On 03/28/2014 04:01 PM, Peter Krempa wrote: ACK; safe for 1.2.3 -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

When virStorageFileGetMetadata is called with NULL path argument, the invalid pointer boils down through the recursive worker and is caught by virHashAddEntry which is thankfully resistant to NULL arguments. As it doesn't make sense to pursue backing chains of NULL volumes, exit earlier. This was noticed in the virt-aahelper-test with a slightly modified codebase. --- src/util/virstoragefile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c7384d1..8370d23 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1118,7 +1118,7 @@ virStorageFileGetMetadata(const char *path, int format, virHashTablePtr cycle = virHashCreate(5, NULL); virStorageFileMetadataPtr ret; - if (!cycle) + if (!cycle || !path) return NULL; if (format <= VIR_STORAGE_FILE_NONE) -- 1.9.1