[PATCH 10/12] virStorageSourceChainLookup: Handle names like 'vda[4]' internally

Peter Krempa pkrempa at redhat.com
Mon Jan 25 16:05:22 UTC 2021


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 at redhat.com>
---
 src/qemu/qemu_driver.c            | 19 ++++--------
 src/storage_file/storage_source.c | 49 ++++++++++++++++++++++++-------
 src/storage_file/storage_source.h |  2 +-
 tests/virstoragetest.c            |  2 +-
 4 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ed966cf7e3..b3e663c9d5 100644
--- 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 52edb91112..4b46cc4e84 100644
--- 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 58f88a3c01..6eb795b301 100644
--- 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 99007dd662..c976a1c0d0 100644
--- 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();

-- 
2.29.2




More information about the libvir-list mailing list