[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 12/12] storage: rbd: Implement support for passing config file option




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'/>
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]