]> xenbits.xensource.com Git - libvirt.git/commitdiff
Add support for addressing backing stores by index
authorJiri Denemark <jdenemar@redhat.com>
Fri, 18 Apr 2014 12:35:33 +0000 (14:35 +0200)
committerJiri Denemark <jdenemar@redhat.com>
Fri, 25 Apr 2014 09:11:03 +0000 (11:11 +0200)
Each backing store of a given disk is associated with a unique index
(which is also formatted in domain XML) for easier addressing of any
particular backing store. With this patch, any backing store can be
addressed by its disk target and the index. For example, "vdc[4]"
addresses the backing store with index equal to 4 of the disk identified
by "vdc" target. Such shorthand can be used in any API in place for a
backing file path:

    virsh blockcommit domain vda --base vda[3] --top vda[2]

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
src/libvirt.c
src/libvirt_private.syms
src/qemu/qemu_driver.c
src/util/virstoragefile.c
src/util/virstoragefile.h
tests/virstoragetest.c

index 5d54cb595ba3bff5db8f56a4860aa3d467bea87f..b6c99c59c205e2e346126aadf3e632fa9adfba24 100644 (file)
@@ -19669,7 +19669,8 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
  * virDomainBlockRebase:
  * @dom: pointer to domain object
  * @disk: path to the block device, or device shorthand
- * @base: path to backing file to keep, or NULL for no backing file
+ * @base: path to backing file to keep, or device shorthand,
+ *        or NULL for no backing file
  * @bandwidth: (optional) specify copy bandwidth limit in MiB/s
  * @flags: bitwise-OR of virDomainBlockRebaseFlags
  *
@@ -19731,6 +19732,14 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
  * can be found by calling virDomainGetXMLDesc() and inspecting
  * elements within //domain/devices/disk.
  *
+ * The @base parameter can be either a path to a file within the backing
+ * chain, or the device target shorthand (the <target dev='...'/>
+ * sub-element, such as "vda") followed by an index to the backing chain
+ * enclosed in square brackets. Backing chain indexes can be found by
+ * inspecting //disk//backingStore/@index in the domain XML. Thus, for
+ * example, "vda[3]" refers to the backing store with index equal to "3"
+ * in the chain of disk "vda".
+ *
  * The maximum bandwidth (in MiB/s) that will be used to do the copy can be
  * specified with the bandwidth parameter.  If set to 0, libvirt will choose a
  * suitable default.  Some hypervisors do not support this feature and will
@@ -19793,9 +19802,10 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
  * virDomainBlockCommit:
  * @dom: pointer to domain object
  * @disk: path to the block device, or device shorthand
- * @base: path to backing file to merge into, or NULL for default
+ * @base: path to backing file to merge into, or device shorthand,
+ *        or NULL for default
  * @top: path to file within backing chain that contains data to be merged,
- *       or NULL to merge all possible data
+ *       or device shorthand, or NULL to merge all possible data
  * @bandwidth: (optional) specify commit bandwidth limit in MiB/s
  * @flags: bitwise-OR of virDomainBlockCommitFlags
  *
@@ -19845,6 +19855,14 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
  * can be found by calling virDomainGetXMLDesc() and inspecting
  * elements within //domain/devices/disk.
  *
+ * The @base and @top parameters can be either paths to files within the
+ * backing chain, or the device target shorthand (the <target dev='...'/>
+ * sub-element, such as "vda") followed by an index to the backing chain
+ * enclosed in square brackets. Backing chain indexes can be found by
+ * inspecting //disk//backingStore/@index in the domain XML. Thus, for
+ * example, "vda[3]" refers to the backing store with index equal to "3"
+ * in the chain of disk "vda".
+ *
  * The maximum bandwidth (in MiB/s) that will be used to do the commit can be
  * specified with the bandwidth parameter.  If set to 0, libvirt will choose a
  * suitable default.  Some hypervisors do not support this feature and will
index 53dbdb4e8fc3fea4f1c04665d44f3bb9b9784d68..4cee2eb06dd6a05fc17276baa2705601c97642d1 100644 (file)
@@ -1829,6 +1829,7 @@ virStorageFileGetMetadataFromBuf;
 virStorageFileGetMetadataFromFD;
 virStorageFileGetSCSIKey;
 virStorageFileIsClusterFS;
+virStorageFileParseChainIndex;
 virStorageFileProbeFormat;
 virStorageFileProbeFormatFromBuf;
 virStorageFileResize;
index 700310013cc6762c65144d538865577e4ca1c817..67ba48734af488fe6742d3ee738fa0fd351a2bfb 100644 (file)
@@ -14854,6 +14854,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
     virObjectEventPtr event = NULL;
     int idx;
     virDomainDiskDefPtr disk;
+    virStorageSourcePtr baseSource = NULL;
+    unsigned int baseIndex = 0;
 
     if (!virDomainObjIsActive(vm)) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -14915,12 +14917,17 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
         goto endjob;
     }
 
+    if (base &&
+        (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 ||
+         !(baseSource = virStorageFileChainLookup(&disk->src,
+                                                  disk->src.backingStore,
+                                                  base, baseIndex, NULL))))
+        goto endjob;
+
     qemuDomainObjEnterMonitor(driver, vm);
-    /* XXX - libvirt should really be tracking the backing file chain
-     * itself, and validating that base is on the chain, rather than
-     * relying on qemu to do this.  */
-    ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode,
-                              async);
+    ret = qemuMonitorBlockJob(priv->mon, device,
+                              baseIndex ? baseSource->path : base,
+                              bandwidth, info, mode, async);
     qemuDomainObjExitMonitor(driver, vm);
     if (ret < 0)
         goto endjob;
@@ -15274,8 +15281,11 @@ qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth,
 
 
 static int
-qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
-                      const char *top, unsigned long bandwidth,
+qemuDomainBlockCommit(virDomainPtr dom,
+                      const char *path,
+                      const char *base,
+                      const char *top,
+                      unsigned long bandwidth,
                       unsigned int flags)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
@@ -15286,7 +15296,9 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
     int idx;
     virDomainDiskDefPtr disk = NULL;
     virStorageSourcePtr topSource;
+    unsigned int topIndex = 0;
     virStorageSourcePtr baseSource;
+    unsigned int baseIndex = 0;
     const char *top_parent = NULL;
     bool clean_access = false;
 
@@ -15327,12 +15339,15 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
     if (qemuDomainDetermineDiskChain(driver, vm, disk, false) < 0)
         goto endjob;
 
-    if (!top) {
+    if (!top)
         topSource = &disk->src;
-    } else if (!(topSource = virStorageFileChainLookup(disk->src.backingStore,
-                                                       top, &top_parent))) {
+    else if (virStorageFileParseChainIndex(disk->dst, top, &topIndex) < 0 ||
+             !(topSource = virStorageFileChainLookup(&disk->src,
+                                                     disk->src.backingStore,
+                                                     top, topIndex,
+                                                     &top_parent)))
         goto endjob;
-    }
+
     if (!topSource->backingStore) {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("top '%s' in chain for '%s' has no backing file"),
@@ -15342,7 +15357,9 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
 
     if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
         baseSource = topSource->backingStore;
-    else if (!(baseSource = virStorageFileChainLookup(topSource, base, NULL)))
+    else if (virStorageFileParseChainIndex(disk->dst, base, &baseIndex) < 0 ||
+             !(baseSource = virStorageFileChainLookup(&disk->src, topSource,
+                                                      base, baseIndex, NULL)))
         goto endjob;
 
     if ((flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) &&
@@ -15377,8 +15394,9 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
      * thing if the user specified a relative name). */
     qemuDomainObjEnterMonitor(driver, vm);
     ret = qemuMonitorBlockCommit(priv->mon, device,
-                                 top ? top : topSource->path,
-                                 base ? base : baseSource->path, bandwidth);
+                                 top && !topIndex ? top : topSource->path,
+                                 base && !baseIndex ? base : baseSource->path,
+                                 bandwidth);
     qemuDomainObjExitMonitor(driver, vm);
 
  endjob:
index 7ceb8ff6950af43482faac2dd7270db5bb924101..1ce0fa1bc5362dba04e0d0e0fe8a1ece4123b4c6 100644 (file)
@@ -1507,33 +1507,86 @@ int virStorageFileGetSCSIKey(const char *path,
 }
 #endif
 
-/* 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.  */
+int
+virStorageFileParseChainIndex(const char *diskTarget,
+                              const char *name,
+                              unsigned int *chainIndex)
+{
+    char **strings = NULL;
+    unsigned int idx = 0;
+    char *suffix;
+    int ret = 0;
+
+    *chainIndex = 0;
+
+    if (name && diskTarget)
+        strings = virStringSplit(name, "[", 2);
+
+    if (virStringListLength(strings) != 2)
+        goto cleanup;
+
+    if (virStrToLong_ui(strings[1], &suffix, 10, &idx) < 0 ||
+        STRNEQ(suffix, "]"))
+        goto cleanup;
+
+    if (STRNEQ(diskTarget, strings[0])) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("requested target '%s' does not match target '%s'"),
+                       strings[0], diskTarget);
+        ret = -1;
+        goto cleanup;
+    }
+
+    *chainIndex = idx;
+
+ cleanup:
+    virStringFreeList(strings);
+    return ret;
+}
+
+/* Given a @chain, look for the backing store @name within the chain starting
+ * from @startFrom or @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.
+ */
 virStorageSourcePtr
 virStorageFileChainLookup(virStorageSourcePtr chain,
+                          virStorageSourcePtr startFrom,
                           const char *name,
+                          unsigned int idx,
                           const char **parent)
 {
     const char *start = chain->path;
     const char *tmp;
     const char *parentDir = ".";
     bool nameIsFile = virStorageIsFile(name);
+    size_t i;
 
     if (!parent)
         parent = &tmp;
-
     *parent = NULL;
+
+    i = 0;
+    if (startFrom) {
+        while (chain && chain != startFrom) {
+            chain = chain->backingStore;
+            i++;
+        }
+    }
+
     while (chain) {
-        if (!name) {
+        if (!name && !idx) {
             if (!chain->backingStore)
                 break;
+        } else if (idx) {
+            VIR_DEBUG("%zu: %s", i, chain->path);
+            if (idx == i)
+                break;
         } else {
             if (STREQ(name, chain->relPath))
                 break;
@@ -1550,6 +1603,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
         *parent = chain->path;
         parentDir = chain->relDir;
         chain = chain->backingStore;
+        i++;
     }
 
     if (!chain)
@@ -1557,7 +1611,11 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
     return chain;
 
  error:
-    if (name) {
+    if (idx) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("could not find backing store %u in chain for '%s'"),
+                       idx, start);
+    } else if (name) {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("could not find image '%s' in chain for '%s'"),
                        name, start);
index 82198e5edbdac5f10df5bf1917b912954adc80a7..148776ea8a4ce3877a5240b368566641fe5c90c9 100644 (file)
@@ -281,8 +281,15 @@ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path,
 int virStorageFileChainGetBroken(virStorageSourcePtr chain,
                                  char **broken_file);
 
+int virStorageFileParseChainIndex(const char *diskTarget,
+                                  const char *name,
+                                  unsigned int *chainIndex)
+    ATTRIBUTE_NONNULL(3);
+
 virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain,
+                                              virStorageSourcePtr startFrom,
                                               const char *name,
+                                              unsigned int idx,
                                               const char **parent)
     ATTRIBUTE_NONNULL(1);
 
index e730b8e4d7961efaf5b4f32a73c1905c2d97cda3..018469a68a9884961c0601c710a0bfa3fb9a602c 100644 (file)
@@ -413,7 +413,9 @@ testStorageChain(const void *args)
 struct testLookupData
 {
     virStorageSourcePtr chain;
+    const char *target;
     const char *name;
+    unsigned int expIndex;
     const char *expResult;
     virStorageSourcePtr expMeta;
     const char *expParent;
@@ -426,9 +428,22 @@ testStorageLookup(const void *args)
     int ret = 0;
     virStorageSourcePtr result;
     const char *actualParent;
+    unsigned int idx;
+
+    if (virStorageFileParseChainIndex(data->target, data->name, &idx) < 0 &&
+        data->expIndex) {
+        fprintf(stderr, "call should not have failed\n");
+        ret = -1;
+    }
+    if (idx != data->expIndex) {
+        fprintf(stderr, "index: expected %u, got %u\n", data->expIndex, idx);
+        ret = -1;
+    }
 
      /* Test twice to ensure optional parameter doesn't cause NULL deref. */
-    result = virStorageFileChainLookup(data->chain, data->name, NULL);
+    result = virStorageFileChainLookup(data->chain, NULL,
+                                       idx ? NULL : data->name,
+                                       idx, NULL);
 
     if (!data->expResult) {
         if (!virGetLastError()) {
@@ -455,7 +470,8 @@ testStorageLookup(const void *args)
         ret = -1;
     }
 
-    result = virStorageFileChainLookup(data->chain, data->name, &actualParent);
+    result = virStorageFileChainLookup(data->chain, data->chain,
+                                       data->name, idx, &actualParent);
     if (!data->expResult)
         virResetLastError();
 
@@ -878,14 +894,16 @@ mymain(void)
         goto cleanup;
     }
 
-#define TEST_LOOKUP(id, name, result, meta, parent)                  \
-    do {                                                             \
-        struct testLookupData data2 = { chain, name,                 \
-                                        result, meta, parent, };     \
-        if (virtTestRun("Chain lookup " #id,                         \
-                        testStorageLookup, &data2) < 0)              \
-            ret = -1;                                                \
+#define TEST_LOOKUP_TARGET(id, target, name, index, result, meta, parent)   \
+    do {                                                                    \
+        struct testLookupData data2 = { chain, target, name, index,         \
+                                        result, meta, parent, };            \
+        if (virtTestRun("Chain lookup " #id,                                \
+                        testStorageLookup, &data2) < 0)                     \
+            ret = -1;                                                       \
     } while (0)
+#define TEST_LOOKUP(id, name, result, meta, parent)                         \
+    TEST_LOOKUP_TARGET(id, NULL, name, 0, result, meta, parent)
 
     TEST_LOOKUP(0, "bogus", NULL, NULL, NULL);
     TEST_LOOKUP(1, "wrap", chain->path, chain, NULL);
@@ -969,6 +987,21 @@ mymain(void)
     TEST_LOOKUP(25, NULL, chain->backingStore->backingStore->path,
                 chain->backingStore->backingStore, chain->backingStore->path);
 
+    TEST_LOOKUP_TARGET(26, "vda", "bogus[1]", 0, NULL, NULL, NULL);
+    TEST_LOOKUP_TARGET(27, "vda", "vda[-1]", 0, NULL, NULL, NULL);
+    TEST_LOOKUP_TARGET(28, "vda", "vda[1][1]", 0, NULL, NULL, NULL);
+    TEST_LOOKUP_TARGET(29, "vda", "wrap", 0, chain->path, chain, NULL);
+    TEST_LOOKUP_TARGET(30, "vda", "vda[0]", 0, NULL, NULL, NULL);
+    TEST_LOOKUP_TARGET(31, "vda", "vda[1]", 1,
+                       chain->backingStore->path,
+                       chain->backingStore,
+                       chain->path);
+    TEST_LOOKUP_TARGET(32, "vda", "vda[2]", 2,
+                       chain->backingStore->backingStore->path,
+                       chain->backingStore->backingStore,
+                       chain->backingStore->path);
+    TEST_LOOKUP_TARGET(33, "vda", "vda[3]", 3, NULL, NULL, NULL);
+
  cleanup:
     /* Final cleanup */
     virStorageSourceFree(chain);