7:49 a.m.
Returning both virStorageSourcePtr and its path member does not make a
lot of sense.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/qemu/qemu_driver.c | 7 +++----
src/util/virstoragefile.c | 16 +++++++---------
src/util/virstoragefile.h | 7 +++----
tests/virstoragetest.c | 39 +++++++++++++++++++++++----------------
4 files changed, 36 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 35ab2f0..0283d99 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15340,9 +15340,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) {
@@ -15354,7 +15353,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 b2d8f62..4fdbb8b 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1510,9 +1510,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;
@@ -1545,24 +1545,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 a0adb9b..82198e5 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 5320c78..edab03a 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)) {
--
1.9.2