On 11/12/2014 08:47 AM, Peter Krempa wrote:
> To be able to express some use cases of the RBD backing with libvirt, we
> need to be able to specify a config file for the RBD client to qemu as
> that is one of the commonly used options.
> ---
> docs/formatdomain.html.in | 8 ++++++++
> docs/schemas/domaincommon.rng | 8 ++++++++
> src/conf/domain_conf.c | 18 ++++++++++++++++--
> src/qemu/qemu_command.c | 3 +++
> src/util/virstoragefile.c | 5 +++++
> src/util/virstoragefile.h | 2 ++
> .../qemuxml2argv-disk-drive-network-rbd.args | 2 ++
> .../qemuxml2argv-disk-drive-network-rbd.xml | 8 ++++++++
> 8 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index fc35c5a..62bca45 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1677,6 +1677,7 @@
> <source protocol="rbd" name="image_name2">
> <host name="hostname" port="7000"/>
> <snapshot name="snapname"/>
> + <config file="/path/to/file"/>
> </source>
> <target dev="hdc" bus="ide"/>
> <auth username='myuser'>
> @@ -2015,6 +2016,13 @@
> source for storage systems such as rbd.
> (<span class="since">Since 1.2.11 for the qemu driver.</span>)
> </dd>
> + <dt><code>config</code></dt>
> + <dd>
> + The <code>file</code> attribute of <code>config</code> element
> + allows to specify a configuration file for storage backends.
> +
> + (<span class="since">Since 1.2.11</span> for 'rbd' disks in qemu)
So this reads funny/strange... In particular, "...attribute of config
element allows to specify..."
Perhaps,
"The <code>file</code> attribute for the <code>config</code> element
provides a fully qualified path to a configuration file to be provided
as a parameter to the client of a networked storage protocol. Supported
for 'rbd' <span class="since">since 1.2.11 (QEMU only).</span>"
NOTE: Going for consistent usage w/ 11/12... previously snapshot had
"storage systems" and this has "storage backends". Since both seem to
be keyed of which storage protocol is used, I went with storage protocol.
> + </dd>
> </dl>
>
> <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 154d222..6b2845a 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1460,6 +1460,14 @@
> <empty/>
> </element>
> </optional>
> + <optional>
> + <element name="config">
> + <attribute name="file">
> + <ref name="absFilePath"/>
> + </attribute>
> + <empty/>
> + </element>
> + </optional>
> <empty/>
> </element>
> </interleave>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 37a8042..a9a26a4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3166,7 +3166,8 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
> if (dev->type == VIR_DOMAIN_DEVICE_DISK) {
> virDomainDiskDefPtr disk = dev->data.disk;
>
> - /* internal snapshots are currently supported only with rbd: */
> + /* internal snapshots and config files 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) {
> @@ -3175,6 +3176,13 @@ virDomainDeviceDefPostParseInternal(virDomainDeviceDefPtr dev,
> "only with 'rbd' disks"));
> return -1;
> }
> +
> + if (disk->src->configFile) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("<config> element is currently supported "
> + "only with 'rbd' disks"));
> + return -1;
> + }
> }
> }
>
> @@ -5395,6 +5403,9 @@ virDomainDiskSourceParse(xmlNodePtr node,
> /* snapshot currently works only for remote disks */
> src->snapshot = virXPathString("string(./snapshot/@name)", ctxt);
>
> + /* config file currently only works with remote disks */
> + src->configFile = virXPathString("string(./config/@file)", ctxt);
> +
> if (virDomainStorageHostParse(node, &src->hosts, &src->nhosts) < 0)
> goto cleanup;
> break;
> @@ -16179,7 +16190,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
>
> VIR_FREE(path);
>
> - if (src->nhosts == 0 && !src->snapshot) {
> + if (src->nhosts == 0 && !src->snapshot && !src->configFile) {
> virBufferAddLit(buf, "/>\n");
> } else {
> virBufferAddLit(buf, ">\n");
> @@ -16205,6 +16216,9 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
> virBufferEscapeString(buf, "<snapshot name='%s'/>\n",
> src->snapshot);
>
if (src->configfile)
Rest is fine, ACK
John
> + virBufferEscapeString(buf, "<config file='%s'/>\n",
> + src->configFile);
> +
> virBufferAdjustIndent(buf, -2);
> virBufferAddLit(buf, "</source>\n");
> }
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 7923842..a19ad7e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3018,6 +3018,9 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src,
> }
> }
>
> + if (src->configFile)
> + virBufferEscape(&buf, '\\', ":", ":conf=%s", src->configFile);
> +
> if (virBufferCheckError(&buf) < 0)
> goto cleanup;
>
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index efd51d2..c5a309d 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1850,6 +1850,7 @@ virStorageSourceCopy(const virStorageSource *src,
> VIR_STRDUP(ret->relPath, src->relPath) < 0 ||
> VIR_STRDUP(ret->backingStoreRaw, src->backingStoreRaw) < 0 ||
> VIR_STRDUP(ret->snapshot, src->snapshot) < 0 ||
> + VIR_STRDUP(ret->configFile, src->configFile) < 0 ||
> VIR_STRDUP(ret->compat, src->compat) < 0)
> goto error;
>
> @@ -2348,6 +2349,10 @@ virStorageSourceParseRBDColonString(const char *rbdstr,
> }
> }
>
> + if (STRPREFIX(p, "conf=") &&
> + VIR_STRDUP(src->configFile, p + strlen("conf=")) < 0)
> + goto error;
> +
> p = next;
> }
> VIR_FREE(options);
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index caab0b4..578fbf2 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -239,6 +239,8 @@ struct _virStorageSource {
> int protocol; /* virStorageNetProtocol */
> char *volume; /* volume name for remote storage */
> char *snapshot; /* for storage systems supporting internal snapshots */
> + char *configFile; /* some storage systems use config file as part of
> + the source definition */
> 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 e4f1389..f634e8d 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args
> @@ -9,4 +9,6 @@ mon3.example.org\:6322,if=virtio,format=raw' \
> -drive 'file=rbd:pool/image@foo: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@foo:auth_supported=none:\
> +conf=/blah/test.conf,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 f6accd8..9ffe633 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.xml
> @@ -46,6 +46,14 @@
> </source>
> <target dev='vdc' bus='virtio'/>
> </disk>
> + <disk type='network' device='disk'>
> + <driver name='qemu' type='raw'/>
> + <source protocol='rbd' name='pool/image'>
> + <snapshot name='foo'/>
> + <config file='/blah/test.conf'/>
> + </source>
> + <target dev='vdd' bus='virtio'/>
> + </disk>
> <controller type='usb' index='0'/>
> <controller type='ide' index='0'/>
> <controller type='pci' index='0' model='pci-root'/>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list