[libvirt] [PATCH V2 3/4] xenconfig: support xl<->xml conversion of rbd disk devices

[Ján Tomko]

On Wed, Feb 17, 2016 at 05:33:44PM -0700, Jim Fehlig wrote:
> The target= setting in xl disk configuration can be used to encode
> meta info that is meaningful to a backend. Leverage this fact to
> support qdisk network disk types such as rbd. E.g. <disk> config
> such as
> 
>    <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'/>
>      </source>
>      <target dev='hdb' bus='ide'/>
>      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
>    </disk>
> 
> can be converted to the following xl config (and vice versa)
> 
>   disk = [ "format=raw,vdev=hdb,access=rw,backendtype=qdisk,
>             target=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\\:6321\\;mon2.example.org\\:6322\\;mon3.example.org\\:6322"
>          ]
> 
> Note that in xl disk config, a literal backslash in target= must
> be escaped with a backslash. Conversion of <auth> config is not
> handled in this patch, but can be done in a follow-up patch.
> 
> Also add a test for the conversions.
> 
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
> 
> v2:
> Change commit msg, test, and code to escape literal backslash
> with a backslash.
> 
>  src/xenconfig/xen_xl.c                           | 153 +++++++++++++++++++++--
>  tests/xlconfigdata/test-rbd-multihost-noauth.cfg |  26 ++++
>  tests/xlconfigdata/test-rbd-multihost-noauth.xml |  51 ++++++++
>  tests/xlconfigtest.c                             |   1 +
>  4 files changed, 224 insertions(+), 7 deletions(-)

> +xenFormatXLDiskSrcNet(virStorageSourcePtr src)
> +{
> +    char *ret = NULL;
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    size_t i;
> +
> +    switch ((virStorageNetProtocol) src->protocol) {
> +    case VIR_STORAGE_NET_PROTOCOL_NBD:
> +    case VIR_STORAGE_NET_PROTOCOL_HTTP:
> +    case VIR_STORAGE_NET_PROTOCOL_HTTPS:
> +    case VIR_STORAGE_NET_PROTOCOL_FTP:
> +    case VIR_STORAGE_NET_PROTOCOL_FTPS:
> +    case VIR_STORAGE_NET_PROTOCOL_TFTP:
> +    case VIR_STORAGE_NET_PROTOCOL_ISCSI:
> +    case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
> +    case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
> +    case VIR_STORAGE_NET_PROTOCOL_LAST:
> +    case VIR_STORAGE_NET_PROTOCOL_NONE:
> +        virReportError(VIR_ERR_NO_SUPPORT,
> +                       _("Unsupported network block protocol '%s'"),
> +                       virStorageNetProtocolTypeToString(src->protocol));
> +        goto cleanup;
> +
> +    case VIR_STORAGE_NET_PROTOCOL_RBD:
> +        if (strchr(src->path, ':')) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("':' not allowed in RBD source volume name '%s'"),
> +                           src->path);
> +            goto cleanup;
> +        }
> +
> +        virBufferStrcat(&buf, "rbd:", src->path, NULL);
> +
> +        virBufferAddLit(&buf, ":auth_supported=none");
> +
> +        if (src->nhosts > 0) {
> +            virBufferAddLit(&buf, ":mon_host=");
> +            for (i = 0; i < src->nhosts; i++) {
> +                if (i)
> +                    virBufferAddLit(&buf, "\\\\;");
> +

You could add the separator unconditionally

> +                /* assume host containing : is ipv6 */
> +                if (strchr(src->hosts[i].name, ':'))
> +                    virBufferEscape(&buf, '\\', ":", "[%s]",
> +                                    src->hosts[i].name);
> +                else
> +                    virBufferAsprintf(&buf, "%s", src->hosts[i].name);
> +
> +                if (src->hosts[i].port)
> +                    virBufferAsprintf(&buf, "\\\\:%s", src->hosts[i].port);
> +            }

And use virBufferTrim after the cycle.

ACK either way.

Jan