[libvirt] [PATCH 4/4] Add support for addressing backing stores by index
Jiri Denemark
jdenemar at redhat.com
Tue Apr 22 12:49:50 UTC 2014
Each backing store of a given disk is associated with a unique index
(which is also formated 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 at redhat.com>
---
src/libvirt.c | 13 ++++++--
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 29 ++++++++++++-----
src/util/virstoragefile.c | 83 ++++++++++++++++++++++++++++++++++++++++-------
src/util/virstoragefile.h | 7 ++++
tests/virstoragetest.c | 51 ++++++++++++++++++++++++-----
6 files changed, 152 insertions(+), 32 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 4454829..a9f56c3 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19787,9 +19787,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
*
@@ -19839,6 +19840,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
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fe8a810..b77050d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1830,6 +1830,7 @@ virStorageFileGetMetadataFromBuf;
virStorageFileGetMetadataFromFD;
virStorageFileGetSCSIKey;
virStorageFileIsClusterFS;
+virStorageFileParseChainIndex;
virStorageFileProbeFormat;
virStorageFileProbeFormatFromBuf;
virStorageFileResize;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0283d99..4ed1123 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15285,8 +15285,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;
@@ -15297,7 +15300,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;
@@ -15338,12 +15343,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"),
@@ -15353,7 +15361,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) &&
@@ -15388,8 +15398,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 4fdbb8b..f4387e0 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1501,33 +1501,87 @@ 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 *index)
+{
+ char **strings = NULL;
+ unsigned int idx = 0;
+ char *suffix;
+ int ret = 0;
+
+ *index = 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;
+ }
+
+ *index = idx;
+
+ cleanup:
+ virStringFreeList(strings);
+ return ret;
+}
+
+/* Given a CHAIN, look for the backing file NAME within the chain starting
+ * from @startFrom or @chain if @startFrom is NULL and return its canonical
+ * name. @chain must always point to the top of the chain. Pass NULL for
+ * NAME and 0 for INDEX to find the base of the chain. Pass nonzero INDEX
+ * to find the backing source according to its position in the backing 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,
+ * independently freed. Reports an error and returns NULL if NAME is not
+ * found. */
virStorageSourcePtr
virStorageFileChainLookup(virStorageSourcePtr chain,
+ virStorageSourcePtr startFrom,
const char *name,
+ unsigned int index,
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 && !index) {
if (!chain->backingStore)
break;
+ } else if (index) {
+ VIR_DEBUG("%zu: %s", i, chain->path);
+ if (index == i)
+ break;
} else {
if (STREQ(name, chain->relPath))
break;
@@ -1544,6 +1598,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
*parent = chain->path;
parentDir = chain->relDir;
chain = chain->backingStore;
+ i++;
}
if (!chain)
@@ -1551,7 +1606,11 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
return chain;
error:
- if (name) {
+ if (index) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("could not find backing store %u in chain for '%s'"),
+ index, 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 82198e5..043896d 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 *index)
+ ATTRIBUTE_NONNULL(3);
+
virStorageSourcePtr virStorageFileChainLookup(virStorageSourcePtr chain,
+ virStorageSourcePtr startFrom,
const char *name,
+ unsigned int index,
const char **parent)
ATTRIBUTE_NONNULL(1);
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index edab03a..7802577 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 index;
+
+ if (virStorageFileParseChainIndex(data->target, data->name, &index) < 0 &&
+ data->expIndex) {
+ fprintf(stderr, "call should not have failed\n");
+ ret = -1;
+ }
+ if (index != data->expIndex) {
+ fprintf(stderr, "index: expected %u, got %u\n", data->expIndex, index);
+ 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,
+ index ? NULL : data->name,
+ index, 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, index, &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);
--
1.9.2
More information about the libvir-list
mailing list