[PATCH 3/4] virStorageFileChainLookup: Return virStorageSourcePtr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Returning both virStorageSourcePtr and its path member does not make a
lot of sense.

Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
---
 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]