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>

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) &&

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)

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);
 

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();