conf: drop redundant parameter to chain lookup

The original chain lookup code had to pass in the starting name,
because it was not available in the chain.  But now that we have
added fields to the struct, this parameter is redundant.

* src/util/virstoragefile.h (virStorageFileChainLookup): Alter
signature.
* src/util/virstoragefile.c (virStorageFileChainLookup): Adjust
handling of top of chain.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Adjust caller.
* tests/virstoragetest.c (testStorageLookup, mymain): Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fed7d1cf9a537c4cf56f3f62e084eb17b7858c27..454e39fb112b884df9598ce8f8447117f74f8ea8 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15343,7 +15343,6 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
         top_canon = disk->src.path;
         top_meta = disk->backingChain;
     } else if (!(top_canon = virStorageFileChainLookup(disk->backingChain,
-                                                       disk->src.path,
                                                        top, &top_meta,
                                                        &top_parent))) {
         goto endjob;
@@ -15356,7 +15355,7 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
     }
     if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
         base_canon = top_meta->backingStore;
-    else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon,
+    else if (!(base_canon = virStorageFileChainLookup(top_meta,
                                                       base, NULL, NULL)))
         goto endjob;
 

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 77cc8781f9f91c4ce3950404057388b9d1438789..bc12cea14ff255e8633f461d72284ca7baf95020 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1529,21 +1529,21 @@ int virStorageFileGetSCSIKey(const char *path,
 }
 #endif
 
-/* Given a CHAIN that starts at the named file START, return a string
- * pointing to either START or within CHAIN that gives the preferred
- * name for the backing file NAME within that chain.  Pass NULL for
- * NAME to find the base of the chain.  If META is not NULL, set *META
- * to the point in the chain that describes NAME (or to NULL if the
- * backing element is not a file).  If PARENT is not NULL, set *PARENT
- * to the preferred name of the parent (or to NULL if NAME matches
- * START).  Since the results point within CHAIN, they must not be
+/* Given a CHAIN, look for the backing file NAME within the chain and
+ * return its canonical name.  Pass NULL for NAME to find the base of
+ * the chain.  If META is not NULL, set *META to the point in the
+ * chain that describes NAME (or to NULL if the backing element is not
+ * a file).  If PARENT is not NULL, set *PARENT to the preferred name
+ * of the parent (or to NULL if NAME matches the start of the chain).
+ * 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 *
-virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start,
+virStorageFileChainLookup(virStorageFileMetadataPtr chain,
                           const char *name, virStorageFileMetadataPtr *meta,
                           const char **parent)
 {
+    const char *start = chain->canonPath;
     virStorageFileMetadataPtr owner;
     const char *tmp;
 

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index c747f209286f9eeaefc51460309cc6551f682734..959937c5286701f5ff6c686c77bd3aa978688208 100644 (file)
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -303,11 +303,10 @@ int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,
                                  char **broken_file);
 
 const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain,
-                                      const char *start,
                                       const char *name,
                                       virStorageFileMetadataPtr *meta,
                                       const char **parent)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+    ATTRIBUTE_NONNULL(1);
 
 void virStorageFileFreeMetadata(virStorageFileMetadataPtr meta);
 

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index de37f7fb67cfa1627a23fbfd87d29ebeaa94a77e..5db3cde122b4fdc7b8214f2c92f5f695a5fe5680 100644 (file)
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -381,7 +381,6 @@ testStorageChain(const void *args)
 struct testLookupData
 {
     virStorageFileMetadataPtr chain;
-    const char *start;
     const char *name;
     const char *expResult;
     virStorageFileMetadataPtr expMeta;
@@ -401,8 +400,8 @@ testStorageLookup(const void *args)
      * 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->start,
-                                             data->name, NULL, NULL);
+    actualResult = virStorageFileChainLookup(data->chain, data->name,
+                                             NULL, NULL);
 
     if (!data->expResult) {
         if (!virGetLastError()) {
@@ -423,9 +422,8 @@ testStorageLookup(const void *args)
         ret = -1;
     }
 
-    actualResult = virStorageFileChainLookup(data->chain, data->start,
-                                             data->name, &actualMeta,
-                                             &actualParent);
+    actualResult = virStorageFileChainLookup(data->chain, data->name,
+                                             &actualMeta, &actualParent);
     if (!data->expResult)
         virResetLastError();
     if (STRNEQ_NULLABLE(data->expResult, actualResult)) {
@@ -454,7 +452,6 @@ mymain(void)
     virCommandPtr cmd = NULL;
     struct testChainData data;
     virStorageFileMetadataPtr chain = NULL;
-    const char *start = NULL;
 
     /* Prep some files with qemu-img; if that is not found on PATH, or
      * if it lacks support for qcow2 and qed, skip this test.  */
@@ -849,8 +846,7 @@ mymain(void)
     if (virCommandRun(cmd, NULL) < 0)
         ret = -1;
 
-    /* Test behavior of chain lookups, absolute backing */
-    start = "wrap";
+    /* Test behavior of chain lookups, absolute backing from relative start */
     chain = virStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2,
                                       -1, -1, false);
     if (!chain) {
@@ -860,7 +856,7 @@ mymain(void)
 
 #define TEST_LOOKUP(id, name, result, meta, parent)                  \
     do {                                                             \
-        struct testLookupData data2 = { chain, start, name,          \
+        struct testLookupData data2 = { chain, name,                 \
                                         result, meta, parent, };     \
         if (virtTestRun("Chain lookup " #id,                         \
                         testStorageLookup, &data2) < 0)              \
@@ -868,10 +864,12 @@ mymain(void)
     } while (0)
 
     TEST_LOOKUP(0, "bogus", NULL, NULL, NULL);
-    TEST_LOOKUP(1, "wrap", start, chain, NULL);
-    TEST_LOOKUP(2, abswrap, start, chain, NULL);
-    TEST_LOOKUP(3, "qcow2", chain->backingStore, chain->backingMeta, start);
-    TEST_LOOKUP(4, absqcow2, chain->backingStore, chain->backingMeta, start);
+    TEST_LOOKUP(1, "wrap", chain->canonPath, chain, NULL);
+    TEST_LOOKUP(2, abswrap, chain->canonPath, chain, NULL);
+    TEST_LOOKUP(3, "qcow2", chain->backingStore, chain->backingMeta,
+                chain->canonPath);
+    TEST_LOOKUP(4, absqcow2, chain->backingStore, chain->backingMeta,
+                chain->canonPath);
     TEST_LOOKUP(5, "raw", chain->backingMeta->backingStore,
                 chain->backingMeta->backingMeta, chain->backingStore);
     TEST_LOOKUP(6, absraw, chain->backingMeta->backingStore,
@@ -892,8 +890,7 @@ mymain(void)
     if (virCommandRun(cmd, NULL) < 0)
         ret = -1;
 
-    /* Test behavior of chain lookups, relative backing */
-    start = abswrap;
+    /* Test behavior of chain lookups, relative backing from absolute start */
     virStorageFileFreeMetadata(chain);
     chain = virStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2,
                                       -1, -1, false);
@@ -903,10 +900,12 @@ mymain(void)
     }
 
     TEST_LOOKUP(8, "bogus", NULL, NULL, NULL);
-    TEST_LOOKUP(9, "wrap", start, chain, NULL);
-    TEST_LOOKUP(10, abswrap, start, chain, NULL);
-    TEST_LOOKUP(11, "qcow2", chain->backingStore, chain->backingMeta, start);
-    TEST_LOOKUP(12, absqcow2, chain->backingStore, chain->backingMeta, start);
+    TEST_LOOKUP(9, "wrap", chain->canonPath, chain, NULL);
+    TEST_LOOKUP(10, abswrap, chain->canonPath, chain, NULL);
+    TEST_LOOKUP(11, "qcow2", chain->backingStore, chain->backingMeta,
+                chain->canonPath);
+    TEST_LOOKUP(12, absqcow2, chain->backingStore, chain->backingMeta,
+                chain->canonPath);
     TEST_LOOKUP(13, "raw", chain->backingMeta->backingStore,
                 chain->backingMeta->backingMeta, chain->backingStore);
     TEST_LOOKUP(14, absraw, chain->backingMeta->backingStore,
@@ -922,9 +921,8 @@ mymain(void)
         ret = -1;
 
     /* Test behavior of chain lookups, relative backing */
-    start = "sub/link2";
     virStorageFileFreeMetadata(chain);
-    chain = virStorageFileGetMetadata(start, VIR_STORAGE_FILE_QCOW2,
+    chain = virStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2,
                                       -1, -1, false);
     if (!chain) {
         ret = -1;
@@ -932,15 +930,15 @@ mymain(void)
     }
 
     TEST_LOOKUP(16, "bogus", NULL, NULL, NULL);
-    TEST_LOOKUP(17, "sub/link2", start, chain, NULL);
-    TEST_LOOKUP(18, "wrap", start, chain, NULL);
-    TEST_LOOKUP(19, abswrap, start, chain, NULL);
-    TEST_LOOKUP(20, "../qcow2", chain->backingStore, chain->backingMeta, start);
-    /* FIXME: 21 is questionable, since there is no 'sub/qcow2' and
-     * since relative backing files should be looked up in the context
-     * of the directory containing the parent.  */
-    TEST_LOOKUP(21, "qcow2", chain->backingStore, chain->backingMeta, start);
-    TEST_LOOKUP(22, absqcow2, chain->backingStore, chain->backingMeta, start);
+    TEST_LOOKUP(17, "sub/link2", chain->canonPath, chain, NULL);
+    TEST_LOOKUP(18, "wrap", chain->canonPath, chain, NULL);
+    TEST_LOOKUP(19, abswrap, chain->canonPath, chain, NULL);
+    TEST_LOOKUP(20, "../qcow2", chain->backingStore, chain->backingMeta,
+                chain->canonPath);
+    TEST_LOOKUP(21, "qcow2", chain->backingStore, chain->backingMeta,
+                chain->canonPath);
+    TEST_LOOKUP(22, absqcow2, chain->backingStore, chain->backingMeta,
+                chain->canonPath);
     TEST_LOOKUP(23, "raw", chain->backingMeta->backingStore,
                 chain->backingMeta->backingMeta, chain->backingStore);
     TEST_LOOKUP(24, absraw, chain->backingMeta->backingStore,