[libvirt] [PATCH 0/6] Gluster pool lookup and few gluster related fixes
Peter Krempa (6): storage: pool: Fix XML indentation in pool source lookup storage: netfs: Split up and tidy up NFS storage pool source function storage: netfs: Support lookup of glusterfs pool sources storage: gluster: Implement storage pool lookup storage: gluster: Fix crash when initialization of storage backend fails util: storagefile: Don't pursue backing chain of NULL image configure.ac | 6 +++ src/conf/storage_conf.c | 2 + src/storage/storage_backend.c | 86 +++++++++++++++++++++++++++++++++++ src/storage/storage_backend.h | 4 ++ src/storage/storage_backend_fs.c | 54 ++++++++++++++-------- src/storage/storage_backend_gluster.c | 56 ++++++++++++++++++++++- src/util/virstoragefile.c | 2 +- 7 files changed, 187 insertions(+), 23 deletions(-) -- 1.9.1
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
On 03/28/2014 04:01 PM, Peter Krempa wrote:
The <source> elements need to be indented from <sources> elements. --- src/conf/storage_conf.c | 2 ++ 1 file changed, 2 insertions(+)
ACK; safe for 1.2.3
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))
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/28/14 23:07, Eric Blake wrote:
On 03/28/2014 04:01 PM, Peter Krempa wrote:
The <source> elements need to be indented from <sources> elements. --- src/conf/storage_conf.c | 2 ++ 1 file changed, 2 insertions(+)
ACK; safe for 1.2.3
Pushed; Thanks. Peter
Extract the NFS related stuff into a separate function and tidy up the rest of the code so we can reuse it to add gluster backend detection. Additionally avoid reporting of errors from "showmount" and return an empty source list instead. This will help when adding other detection backends. --- src/storage/storage_backend_fs.c | 53 +++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0f8da06..8fb03c5 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -242,10 +242,8 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(char **const groups, } -static char * -virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *srcSpec, - unsigned int flags) +static void +virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) { /* * # showmount --no-headers -e HOSTNAME @@ -261,6 +259,29 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE int vars[] = { 1 }; + + virCommandPtr cmd = NULL; + + cmd = virCommandNewArgList(SHOWMOUNT, + "--no-headers", + "--exports", + state->host, + NULL); + + if (virCommandRunRegex(cmd, 1, regexes, vars, + virStorageBackendFileSystemNetFindPoolSourcesFunc, + &state, NULL) < 0) + virResetLastError(); + + virCommandFree(cmd); +} + + +static char * +virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, + const char *srcSpec, + unsigned int flags) +{ virNetfsDiscoverState state = { .host = NULL, .list = { @@ -270,15 +291,14 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE } }; virStoragePoolSourcePtr source = NULL; - char *retval = NULL; + char *ret = NULL; size_t i; - virCommandPtr cmd = NULL; virCheckFlags(0, NULL); if (!srcSpec) { - virReportError(VIR_ERR_INVALID_ARG, - "%s", _("hostname must be specified for netfs sources")); + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("hostname must be specified for netfs sources")); return NULL; } @@ -294,19 +314,9 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE state.host = source->hosts[0].name; - cmd = virCommandNewArgList(SHOWMOUNT, - "--no-headers", - "--exports", - source->hosts[0].name, - NULL); + virStorageBackendFileSystemNetFindNFSPoolSources(&state); - if (virCommandRunRegex(cmd, 1, regexes, vars, - virStorageBackendFileSystemNetFindPoolSourcesFunc, - &state, NULL) < 0) - goto cleanup; - - retval = virStoragePoolSourceListFormat(&state.list); - if (retval == NULL) + if (!(ret = virStoragePoolSourceListFormat(&state.list))) goto cleanup; cleanup: @@ -315,8 +325,7 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE VIR_FREE(state.list.sources); virStoragePoolSourceFree(source); - virCommandFree(cmd); - return retval; + return ret; } -- 1.9.1
On 03/28/2014 04:01 PM, Peter Krempa wrote:
Extract the NFS related stuff into a separate function and tidy up the rest of the code so we can reuse it to add gluster backend detection.
Additionally avoid reporting of errors from "showmount" and return an empty source list instead. This will help when adding other detection backends. --- src/storage/storage_backend_fs.c | 53 +++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 22 deletions(-)
Mostly mechanical, but it's borderline (used by patch 4/6, which feels more like a feature than bug fix) and big enough that I'd feel better waiting for 1.2.4 before pushing this one. But once that happens, ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/28/14 23:13, Eric Blake wrote:
On 03/28/2014 04:01 PM, Peter Krempa wrote:
Extract the NFS related stuff into a separate function and tidy up the rest of the code so we can reuse it to add gluster backend detection.
Additionally avoid reporting of errors from "showmount" and return an empty source list instead. This will help when adding other detection backends. --- src/storage/storage_backend_fs.c | 53 +++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 22 deletions(-)
Mostly mechanical, but it's borderline (used by patch 4/6, which feels more like a feature than bug fix) and big enough that I'd feel better waiting for 1.2.4 before pushing this one. But once that happens,
ACK
Pushed now; Thanks Peter
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 04:01 PM, Peter Krempa wrote:
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(+)
New feature rather than bug fix; please wait until after 1.2.3 to push.
+ cmd = virCommandNewArgList(GLUSTER_CLI, + "--xml",
At least it's machine-parseable, and not free-form regex prone to mistakes on funky names :)
+ if (virCommandRun(cmd, &rc) < 0) + goto cleanup; + + if (rc != 0) { + VIR_INFO("failed to query host '%s' for gluster volumes: %s", + host, outbuf);
Fair enough to suppress log messages on failure. ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/28/2014 11:01 PM, Peter Krempa wrote:
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/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;
These three are never freed. Jan
On 03/28/14 23:57, Ján Tomko wrote:
On 03/28/2014 11:01 PM, Peter Krempa wrote:
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/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;
These three are never freed.
I've fixed these leaks and pushed this patch according to Eric's ACK.
Jan
Peter
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:
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(+)
+ + if (!srcSpec) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("hostname must be specified for gluster sources"));
Would it make sense to check localhost if srcSpec is NULL? Again, post-1.2.3, but looks reasonable. ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/28/14 23:22, Eric Blake wrote:
On 03/28/2014 04:01 PM, Peter Krempa wrote:
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(+)
+ + if (!srcSpec) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("hostname must be specified for gluster sources"));
Would it make sense to check localhost if srcSpec is NULL?
Well AFAIK the netfs pool doesn't have that semantic so I didn't bother.
Again, post-1.2.3, but looks reasonable. ACK
Pushed; Thanks Peter
The libgfapi function glfs_fini doesn't tolerate NULL pointers. Add a check on the error paths as it's possible to crash libvirtd if the gluster volume can't be initialized. --- src/storage/storage_backend_gluster.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 5b79146..fe92f15 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -548,7 +548,8 @@ virStorageFileBackendGlusterDeinit(virStorageFilePtr file) file, file->hosts[0].name, file->path); virStorageFileBackendGlusterPrivPtr priv = file->priv; - glfs_fini(priv->vol); + if (priv->vol) + glfs_fini(priv->vol); VIR_FREE(priv->volname); VIR_FREE(priv); @@ -621,7 +622,8 @@ virStorageFileBackendGlusterInit(virStorageFilePtr file) error: VIR_FREE(priv->volname); - glfs_fini(priv->vol); + if (priv->vol) + glfs_fini(priv->vol); VIR_FREE(priv); return -1; -- 1.9.1
On 03/28/2014 04:01 PM, Peter Krempa wrote:
The libgfapi function glfs_fini doesn't tolerate NULL pointers. Add a check on the error paths as it's possible to crash libvirtd if the gluster volume can't be initialized. --- src/storage/storage_backend_gluster.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
ACK; safe for 1.2.3
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 5b79146..fe92f15 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -548,7 +548,8 @@ virStorageFileBackendGlusterDeinit(virStorageFilePtr file) file, file->hosts[0].name, file->path); virStorageFileBackendGlusterPrivPtr priv = file->priv;
- glfs_fini(priv->vol); + if (priv->vol) + glfs_fini(priv->vol); VIR_FREE(priv->volname);
VIR_FREE(priv); @@ -621,7 +622,8 @@ virStorageFileBackendGlusterInit(virStorageFilePtr file)
error: VIR_FREE(priv->volname); - glfs_fini(priv->vol); + if (priv->vol) + glfs_fini(priv->vol); VIR_FREE(priv);
return -1;
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/28/14 23:08, Eric Blake wrote:
On 03/28/2014 04:01 PM, Peter Krempa wrote:
The libgfapi function glfs_fini doesn't tolerate NULL pointers. Add a check on the error paths as it's possible to crash libvirtd if the gluster volume can't be initialized. --- src/storage/storage_backend_gluster.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
ACK; safe for 1.2.3
Pushed; Thanks Peter
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
On 03/28/2014 04:01 PM, Peter Krempa wrote:
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(-)
ACK; safe for 1.2.3
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)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/28/14 23:09, Eric Blake wrote:
On 03/28/2014 04:01 PM, Peter Krempa wrote:
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(-)
ACK; safe for 1.2.3
Pushed; Thanks Peter
participants (3)
-
Eric Blake -
Ján Tomko -
Peter Krempa