diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b7de28ad2c..700310013c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15329,9 +15329,8 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, if (!top) { topSource = &disk->src; - } else if (!(virStorageFileChainLookup(&disk->src, - top, &topSource, - &top_parent))) { + } else if (!(topSource = virStorageFileChainLookup(disk->src.backingStore, + top, &top_parent))) { goto endjob; } if (!topSource->backingStore) { @@ -15343,7 +15342,7 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base, if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) baseSource = topSource->backingStore; - else if (!(virStorageFileChainLookup(topSource, base, &baseSource, NULL))) + else if (!(baseSource = virStorageFileChainLookup(topSource, base, NULL))) goto endjob; if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) && diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 3ba9d93f14..7ceb8ff695 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1516,9 +1516,9 @@ int virStorageFileGetSCSIKey(const char *path, * Since the results point within CHAIN, they must not be * independently freed. Reports an error and returns NULL if NAME is * not found. */ -const char * +virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain, - const char *name, virStorageSourcePtr *meta, + const char *name, const char **parent) { const char *start = chain->path; @@ -1551,24 +1551,22 @@ virStorageFileChainLookup(virStorageSourcePtr chain, parentDir = chain->relDir; chain = chain->backingStore; } + if (!chain) goto error; - if (meta) - *meta = chain; - return chain->path; + return chain; error: - if (name) + if (name) { virReportError(VIR_ERR_INVALID_ARG, _("could not find image '%s' in chain for '%s'"), name, start); - else + } else { virReportError(VIR_ERR_INVALID_ARG, _("could not find base image in chain for '%s'"), start); + } *parent = NULL; - if (meta) - *meta = NULL; return NULL; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index a0adb9be1b..82198e5edb 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -281,10 +281,9 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, int virStorageFileChainGetBroken(virStorageSourcePtr chain, char **broken_file); -const char *virStorageFileChainLookup(virStorageSourcePtr chain, - const char *name, - virStorageSourcePtr *meta, - const char **parent) +virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain, + const char *name, + const char **parent) ATTRIBUTE_NONNULL(1); int virStorageFileResize(const char *path, diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index c02d866d80..e730b8e4d7 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -424,16 +424,11 @@ testStorageLookup(const void *args) { const struct testLookupData *data = args; int ret = 0; - const char *actualResult; - virStorageSourcePtr actualMeta; + virStorageSourcePtr result; const char *actualParent; - /* This function is documented as giving results within chain, but - * as the same string may be duplicated into more than one field, - * we rely on STREQ rather than pointer equality. Test twice to - * ensure optional parameters don't cause NULL deref. */ - actualResult = virStorageFileChainLookup(data->chain, data->name, - NULL, NULL); + /* Test twice to ensure optional parameter doesn't cause NULL deref. */ + result = virStorageFileChainLookup(data->chain, data->name, NULL); if (!data->expResult) { if (!virGetLastError()) { @@ -448,24 +443,36 @@ testStorageLookup(const void *args) } } - if (STRNEQ_NULLABLE(data->expResult, actualResult)) { + if (!result) { + if (data->expResult) { + fprintf(stderr, "result 1: expected %s, got NULL\n", + data->expResult); + ret = -1; + } + } else if (STRNEQ_NULLABLE(data->expResult, result->path)) { fprintf(stderr, "result 1: expected %s, got %s\n", - NULLSTR(data->expResult), NULLSTR(actualResult)); + NULLSTR(data->expResult), NULLSTR(result->path)); ret = -1; } - actualResult = virStorageFileChainLookup(data->chain, data->name, - &actualMeta, &actualParent); + result = virStorageFileChainLookup(data->chain, data->name, &actualParent); if (!data->expResult) virResetLastError(); - if (STRNEQ_NULLABLE(data->expResult, actualResult)) { + + if (!result) { + if (data->expResult) { + fprintf(stderr, "result 2: expected %s, got NULL\n", + data->expResult); + ret = -1; + } + } else if (STRNEQ_NULLABLE(data->expResult, result->path)) { fprintf(stderr, "result 2: expected %s, got %s\n", - NULLSTR(data->expResult), NULLSTR(actualResult)); + NULLSTR(data->expResult), NULLSTR(result->path)); ret = -1; } - if (data->expMeta != actualMeta) { + if (data->expMeta != result) { fprintf(stderr, "meta: expected %p, got %p\n", - data->expMeta, actualMeta); + data->expMeta, result); ret = -1; } if (STRNEQ_NULLABLE(data->expParent, actualParent)) {