[libvirt] [PATCH 11/12] storage: rbd: qemu: Add support for specifying internal RBD snapshots
John Ferlan
jferlan at redhat.com
Thu Nov 20 15:16:38 UTC 2014
On 11/12/2014 08:47 AM, Peter Krempa wrote:
> Some storage systems have internal support for snapshots. Libvirt should
> be able to select a correct snapshot when starting a VM.
>
> This patch adds a XML element to select a storage source snapshot for
> the RBD protocol which supports this feature.
> ---
> docs/formatdomain.html.in | 18 +++++++---
> docs/schemas/domaincommon.rng | 8 +++++
> src/conf/domain_conf.c | 38 +++++++++++++++++++---
> src/conf/domain_conf.h | 1 +
> src/conf/snapshot_conf.c | 6 ++--
> src/qemu/qemu_command.c | 3 ++
> src/util/virstoragefile.c | 8 +++++
> src/util/virstoragefile.h | 1 +
> .../qemuxml2argv-disk-drive-network-rbd.args | 4 +++
> .../qemuxml2argv-disk-drive-network-rbd.xml | 17 ++++++++++
> 10 files changed, 94 insertions(+), 10 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 4f44bc0..fc35c5a 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1676,6 +1676,7 @@
> <driver name="qemu" type="raw"/>
> <source protocol="rbd" name="image_name2">
> <host name="hostname" port="7000"/>
> + <snapshot name="snapname"/>
> </source>
> <target dev="hdc" bus="ide"/>
> <auth username='myuser'>
> @@ -1949,14 +1950,16 @@
> is only valid when the specified storage volume is of 'file' or
> 'block' type).
> <p>
> - When the disk <code>type</code> is "network", the <code>source</code>
> - may have zero or more <code>host</code> sub-elements used to
> - specify the hosts to connect.
> + The <code>source</code> element may contain the following sub elements:
> </p>
>
> <dl>
> <dt><code>host</code></dt>
> - <dd>The <code>host</code> element supports 4 attributes, viz. "name",
> + <dd>When the disk <code>type</code> is "network", the <code>source</code>
> + may have zero or more <code>host</code> sub-elements used to
> + specify the hosts to connect.
> +
> + The <code>host</code> element supports 4 attributes, viz. "name",
> "port", "transport" and "socket", which specify the hostname,
> the port number, transport type and path to socket, respectively.
> The meaning of this element and the number of the elements depend
> @@ -2005,6 +2008,13 @@
> transport is "unix", the socket attribute specifies the path to an
> AF_UNIX socket.
> </dd>
> + <dt><code>snapshot</code></dt>
> + <dd>
> + The <code>name</code> attribute of <code>snapshot</code> element can
> + optionally specify an internal snapshot name to be used as the
> + source for storage systems such as rbd.
> + (<span class="since">Since 1.2.11 for the qemu driver.</span>)
To follow other usages (adjust last 2 lines):
source for storage protocols.
Supported for 'rbd' <span class="since">since 1.2.11 (QEMU only).</span>
> + </dd>
> </dl>
>
> <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 6863ec6..154d222 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1452,6 +1452,14 @@
> </choice>
> </element>
> </zeroOrMore>
> + <optional>
> + <element name="snapshot">
> + <attribute name="name">
> + <ref name="genericName"/>
> + </attribute>
> + <empty/>
> + </element>
> + </optional>
> <empty/>
> </element>
> </interleave>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2c65276..37a8042 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3162,6 +3162,22 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
> return -1;
> }
>
> + /* verify disk source */
> + if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
> + virDomainDiskDefPtr disk = dev->data.disk;
> +
> + /* internal snapshots are currently supported only with rbd: */
> + if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_NETWORK &&
> + disk->src->protocol != VIR_STORAGE_NET_PROTOCOL_RBD) {
> + if (disk->src->snapshot) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("<snapshot> element is currently supported "
> + "only with 'rbd' disks"));
> + return -1;
> + }
> + }
> + }
> +
> return 0;
> }
>
> @@ -5316,10 +5332,14 @@ virDomainDiskSourcePoolDefParse(xmlNodePtr node,
>
> int
> virDomainDiskSourceParse(xmlNodePtr node,
> + xmlXPathContextPtr ctxt,
> virStorageSourcePtr src)
> {
> int ret = -1;
> char *protocol = NULL;
> + xmlNodePtr saveNode = ctxt->node;
> +
> + ctxt->node = node;
>
> switch ((virStorageType)src->type) {
> case VIR_STORAGE_TYPE_FILE:
> @@ -5372,6 +5392,9 @@ virDomainDiskSourceParse(xmlNodePtr node,
> tmp[0] = '\0';
> }
>
> + /* snapshot currently works only for remote disks */
> + src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
> +
> if (virDomainStorageHostParse(node, &src->hosts, &src->nhosts) < 0)
> goto cleanup;
> break;
> @@ -5397,6 +5420,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
>
> cleanup:
> VIR_FREE(protocol);
> + ctxt->node = saveNode;
> return ret;
> }
>
> @@ -5452,7 +5476,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
> goto cleanup;
> }
>
> - if (virDomainDiskSourceParse(source, backingStore) < 0 ||
> + if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 ||
> virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
> goto cleanup;
>
> @@ -5562,7 +5586,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
> xmlStrEqual(cur->name, BAD_CAST "source")) {
> sourceNode = cur;
>
> - if (virDomainDiskSourceParse(cur, def->src) < 0)
> + if (virDomainDiskSourceParse(cur, ctxt, def->src) < 0)
> goto error;
> source = def->src->path;
>
> @@ -5728,7 +5752,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
> _("mirror requires source element"));
> goto error;
> }
> - if (virDomainDiskSourceParse(mirrorNode, def->mirror) < 0)
> + if (virDomainDiskSourceParse(mirrorNode, ctxt,
> + def->mirror) < 0)
> goto error;
> }
> ready = virXMLPropString(cur, "ready");
> @@ -16154,11 +16179,12 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
>
> VIR_FREE(path);
>
> - if (src->nhosts == 0) {
> + if (src->nhosts == 0 && !src->snapshot) {
> virBufferAddLit(buf, "/>\n");
> } else {
> virBufferAddLit(buf, ">\n");
> virBufferAdjustIndent(buf, 2);
> +
> for (n = 0; n < src->nhosts; n++) {
> virBufferAddLit(buf, "<host");
> virBufferEscapeString(buf, " name='%s'",
> @@ -16175,6 +16201,10 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
>
> virBufferAddLit(buf, "/>\n");
> }
> +
if (src->snapshot)
Rest is fine, ACK
John
> + virBufferEscapeString(buf, "<snapshot name='%s'/>\n",
> + src->snapshot);
> +
> virBufferAdjustIndent(buf, -2);
> virBufferAddLit(buf, "</source>\n");
> }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 530a3ca..acbf13b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2477,6 +2477,7 @@ virDomainDiskRemove(virDomainDefPtr def, size_t i);
> virDomainDiskDefPtr
> virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);
> int virDomainDiskSourceParse(xmlNodePtr node,
> + xmlXPathContextPtr ctxt,
> virStorageSourcePtr src);
>
> bool virDomainHasDiskMirror(virDomainObjPtr vm);
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index 1f83b2c..66fc2e6 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -107,6 +107,7 @@ void virDomainSnapshotDefFree(virDomainSnapshotDefPtr def)
>
> static int
> virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
> + xmlXPathContextPtr ctxt,
> virDomainSnapshotDiskDefPtr def)
> {
> int ret = -1;
> @@ -154,7 +155,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
> if (!def->src->path &&
> xmlStrEqual(cur->name, BAD_CAST "source")) {
>
> - if (virDomainDiskSourceParse(cur, def->src) < 0)
> + if (virDomainDiskSourceParse(cur, ctxt, def->src) < 0)
> goto cleanup;
>
> } else if (!def->src->format &&
> @@ -352,7 +353,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
> goto cleanup;
> def->ndisks = n;
> for (i = 0; i < def->ndisks; i++) {
> - if (virDomainSnapshotDiskDefParseXML(nodes[i], &def->disks[i]) < 0)
> + if (virDomainSnapshotDiskDefParseXML(nodes[i], ctxt,
> + &def->disks[i]) < 0)
> goto cleanup;
> }
> VIR_FREE(nodes);
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 021ec07..7923842 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2988,6 +2988,9 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
>
> virBufferStrcat(&buf, "rbd:", src->path, NULL);
>
> + if (src->snapshot)
> + virBufferEscape(&buf, '\\', ":", "@%s", src->snapshot);
> +
> if (username) {
> virBufferEscape(&buf, '\\', ":", ":id=%s", username);
> virBufferEscape(&buf, '\\', ":",
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index bc7c51d..efd51d2 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1849,6 +1849,7 @@ virStorageSourceCopy(const virStorageSource *src,
> VIR_STRDUP(ret->driverName, src->driverName) < 0 ||
> VIR_STRDUP(ret->relPath, src->relPath) < 0 ||
> VIR_STRDUP(ret->backingStoreRaw, src->backingStoreRaw) < 0 ||
> + VIR_STRDUP(ret->snapshot, src->snapshot) < 0 ||
> VIR_STRDUP(ret->compat, src->compat) < 0)
> goto error;
>
> @@ -2280,6 +2281,13 @@ virStorageSourceParseRBDColonString(const char *rbdstr,
> *p = '\0';
> }
>
> + /* snapshot name */
> + if ((p = strchr(src->path, '@'))) {
> + if (VIR_STRDUP(src->snapshot, p + 1) < 0)
> + goto error;
> + *p = '\0';
> + }
> +
> /* options */
> if (!options)
> return 0; /* all done */
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index b1ba73a..caab0b4 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -238,6 +238,7 @@ struct _virStorageSource {
> char *path;
> int protocol; /* virStorageNetProtocol */
> char *volume; /* volume name for remote storage */
> + char *snapshot; /* for storage systems supporting internal snapshots */
> size_t nhosts;
> virStorageNetHostDefPtr hosts;
> virStorageSourcePoolDefPtr srcpool;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
> index 21d7b64..e4f1389 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
> @@ -5,4 +5,8 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \
> -drive 'file=rbd:pool/image:auth_supported=none:\
> mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
> mon3.example.org\:6322,if=virtio,format=raw' \
> +-drive file=rbd:pool/image at asdf:auth_supported=none,if=virtio,format=raw \
> +-drive 'file=rbd:pool/image at foo:auth_supported=none:\
> +mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
> +mon3.example.org\:6322,if=virtio,format=raw' \
> -net none -serial none -parallel none
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
> index 37e9db5..f6accd8 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
> @@ -29,6 +29,23 @@
> </source>
> <target dev='vda' bus='virtio'/>
> </disk>
> + <disk type='network' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source protocol='rbd' name='pool/image'>
> + <snapshot name='asdf'/>
> + </source>
> + <target dev='vdb' bus='virtio'/>
> + </disk>
> + <disk type='network' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source protocol='rbd' name='pool/image'>
> + <host name='mon1.example.org' port='6321'/>
> + <host name='mon2.example.org' port='6322'/>
> + <host name='mon3.example.org' port='6322'/>
> + <snapshot name='foo'/>
> + </source>
> + <target dev='vdc' bus='virtio'/>
> + </disk>
> <controller type='usb' index='0'/>
> <controller type='ide' index='0'/>
> <controller type='pci' index='0' model='pci-root'/>
>
More information about the libvir-list
mailing list