virStorageSourceChainLookup: Handle names like 'vda[4]' internally

All callers of this function called virStorageFileParseChainIndex
before. Internalize the logic of that function to prevent multiple calls
and passing around unnecessary temporary variables.

This is achieved by calling virStorageFileParseBackingStoreStr and using
it to fill the values internally.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ed966cf7e3a3264a3dbce08f1e601574722407a7..b3e663c9d57efcceb7deb0365329ba55b3c0913e 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14412,7 +14412,6 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
     const char *jobname = NULL;
     virDomainDiskDefPtr disk;
     virStorageSourcePtr baseSource = NULL;
-    unsigned int baseIndex = 0;
     g_autofree char *basePath = NULL;
     g_autofree char *backingPath = NULL;
     unsigned long long speed = bandwidth;
@@ -14448,9 +14447,8 @@ qemuDomainBlockPullCommon(virDomainObjPtr vm,
         goto endjob;
 
     if (base &&
-        (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 ||
-         !(baseSource = virStorageSourceChainLookup(disk->src, disk->src,
-                                                    base, baseIndex, NULL))))
+        !(baseSource = virStorageSourceChainLookup(disk->src, disk->src,
+                                                   base, disk->dst, NULL)))
         goto endjob;
 
     if (baseSource) {
@@ -15465,9 +15463,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
     int ret = -1;
     virDomainDiskDefPtr disk = NULL;
     virStorageSourcePtr topSource;
-    unsigned int topIndex = 0;
     virStorageSourcePtr baseSource = NULL;
-    unsigned int baseIndex = 0;
     virStorageSourcePtr top_parent = NULL;
     bool clean_access = false;
     g_autofree char *topPath = NULL;
@@ -15540,10 +15536,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
 
     if (!top || STREQ(top, disk->dst))
         topSource = disk->src;
-    else if (virStorageFileParseChainIndex(disk->dst, top, &topIndex) < 0 ||
-             !(topSource = virStorageSourceChainLookup(disk->src, NULL,
-                                                       top, topIndex,
-                                                       &top_parent)))
+    else if (!(topSource = virStorageSourceChainLookup(disk->src, NULL, top,
+                                                       disk->dst, &top_parent)))
         goto endjob;
 
     if (topSource == disk->src) {
@@ -15575,9 +15569,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
 
     if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
         baseSource = topSource->backingStore;
-    else if (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 ||
-             !(baseSource = virStorageSourceChainLookup(disk->src, topSource,
-                                                        base, baseIndex, NULL)))
+    else if (!(baseSource = virStorageSourceChainLookup(disk->src, topSource,
+                                                        base, disk->dst, NULL)))
         goto endjob;
 
     if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&

diff --git a/src/storage_file/storage_source.c b/src/storage_file/storage_source.c
index 52edb91112c9cad867fd6d283a2d70240997b925..4b46cc4e84c5deae32f5cb1dc9b1dab6dcdf12a8 100644 (file)
--- a/src/storage_file/storage_source.c
+++ b/src/storage_file/storage_source.c
@@ -198,32 +198,59 @@ virStorageSourceGetMetadataFromFD(const char *path,
 }
 
 
-/* Given a @chain, look for the backing store @name that is a backing file
- * of @startFrom (or any member of @chain if @startFrom is NULL) and return
- * that location within the chain.  @chain must always point to the top of
- * the chain.  Pass NULL for @name and 0 for @idx to find the base of the
- * chain.  Pass nonzero @idx to find the backing source according to its
- * position in the backing chain.  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.
+/**
+ * virStorageSourceChainLookup:
+ * @chain: chain top to look in
+ * @startFrom: move the starting point of @chain if non-NULL
+ * @name: name of the file to look for in @chain
+ * @diskTarget: optional disk target to validate against
+ * @parent: Filled with parent virStorageSource of the returned value if non-NULL.
+ *
+ * Looks up a storage source definition corresponding to @name in @chain and
+ * returns the corresponding virStorageSource. If @startFrom is non-NULL, the
+ * lookup starts from that virStorageSource.
+ *
+ * @name can be:
+ *  - NULL: the end of the source chain is returned
+ *  - "vda[4]": Storage source with 'id' == 4 is returned. If @diskTarget is
+ *              non-NULL it's also validated that the part before the square
+ *              bracket matches the requested target
+ *  - "/path/to/file": Literal path is matched. Symlink resolution is attempted
+ *                     if the filename doesn't string-match with the path.
  */
 virStorageSourcePtr
 virStorageSourceChainLookup(virStorageSourcePtr chain,
                             virStorageSourcePtr startFrom,
                             const char *name,
-                            unsigned int idx,
+                            const char *diskTarget,
                             virStorageSourcePtr *parent)
 {
     virStorageSourcePtr prev;
     const char *start = chain->path;
     bool nameIsFile = virStorageSourceBackinStoreStringIsFile(name);
+    g_autofree char *target = NULL;
+    unsigned int idx = 0;
+
+    if (diskTarget)
+        start = diskTarget;
 
     if (!parent)
         parent = &prev;
     *parent = NULL;
 
+    /* parse the "vda[4]" type string */
+    if (name &&
+        virStorageFileParseBackingStoreStr(name, &target, &idx) == 0) {
+        if (diskTarget &&
+            idx != 0 &&
+            STRNEQ(diskTarget, target)) {
+            virReportError(VIR_ERR_INVALID_ARG,
+                           _("requested target '%s' does not match target '%s'"),
+                           target, diskTarget);
+            return NULL;
+        }
+    }
+
     if (startFrom) {
         while (virStorageSourceIsBacking(chain) &&
                chain != startFrom->backingStore)

diff --git a/src/storage_file/storage_source.h b/src/storage_file/storage_source.h
index 58f88a3c01ec563a64e15ba18ff413d3698d46a5..6eb795b301f94284ca110b454eba567fa939c236 100644 (file)
--- a/src/storage_file/storage_source.h
+++ b/src/storage_file/storage_source.h
@@ -43,7 +43,7 @@ virStorageSourcePtr
 virStorageSourceChainLookup(virStorageSourcePtr chain,
                             virStorageSourcePtr startFrom,
                             const char *name,
-                            unsigned int idx,
+                            const char *diskTarget,
                             virStorageSourcePtr *parent)
     ATTRIBUTE_NONNULL(1);
 

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 99007dd662d5e41cdae629266d1106f61c03679f..c976a1c0d03b11476b0fdf68387ad481af807395 100644 (file)
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -366,7 +366,7 @@ testStorageLookup(const void *args)
     }
 
     result = virStorageSourceChainLookup(data->chain, data->from,
-                                         data->name, idx, &actualParent);
+                                         data->name, data->target, &actualParent);
     if (!data->expResult)
         virResetLastError();