diff --git a/src/libvirt.c b/src/libvirt.c index 5d54cb595b..b6c99c59c2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -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 + * 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 + * 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 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 53dbdb4e8f..4cee2eb06d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1829,6 +1829,7 @@ virStorageFileGetMetadataFromBuf; virStorageFileGetMetadataFromFD; virStorageFileGetSCSIKey; virStorageFileIsClusterFS; +virStorageFileParseChainIndex; virStorageFileProbeFormat; virStorageFileProbeFormatFromBuf; virStorageFileResize; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 700310013c..67ba48734a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -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: diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 7ceb8ff695..1ce0fa1bc5 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -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); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 82198e5edb..148776ea8a 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -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); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index e730b8e4d7..018469a68a 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -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);