[libvirt] [PATCH 05/13] qemu: support NBD with Unix sockets

[Osier Yang]

On 2013年02月26日 01:44, Paolo Bonzini wrote:
> This reuses the XML format that was introduced for Gluster.
>
> Signed-off-by: Paolo Bonzini<pbonzini at redhat.com>
> ---
>   docs/formatdomain.html.in                          |  8 ++--
>   src/qemu/qemu_command.c                            | 49 +++++++++++++++-------
>   tests/qemuargv2xmltest.c                           |  1 +
>   .../qemuxml2argv-disk-drive-network-nbd-unix.args  |  5 +++
>   .../qemuxml2argv-disk-drive-network-nbd-unix.xml   | 33 +++++++++++++++
>   tests/qemuxml2argvtest.c                           |  2 +
>   tests/qemuxml2xmltest.c                            |  1 +
>   7 files changed, 80 insertions(+), 19 deletions(-)
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.args
>   create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-nbd-unix.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 90cfc03..094b509 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1713,9 +1713,11 @@
>               <td>  only one</td>
>             </tr>
>           </table>
> -        In case of gluster, valid values for transport attribute are tcp, rdma
> -        or unix. If nothing is specified, tcp is assumed. If transport is unix,
> -        socket attribute specifies path to unix socket.
> +        gluster supports "tcp", "rdma", "unix" as valid values for the
> +        transport attribute.  nbd supports "tcp" and "unix".  Others only

Generally we want "attribute <code>transport</code>", to make it more
clear in generated html pages.

> +        support "tcp".  If nothing is specified, "tcp" is assumed. If the
> +        transport is "unix", the socket attribute specifies the path to an
> +        AF_UNIX socket.

Likewise, "attribute <code>socket</code>".

>         </dd>
>         <dt><code>address</code></dt>
>         <dd>If present, the<code>address</code>  element ties the disk
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4762f0a..89cd065 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2138,25 +2138,34 @@ qemuParseNBDString(virDomainDiskDefPtr disk)
>           goto no_memory;
>
>       host = disk->src + strlen("nbd:");
> -    port = strchr(host, ':');
> -    if (!port) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("cannot parse nbd filename '%s'"), disk->src);
> -        goto error;
> -    }
> +    if (STRPREFIX(host, "unix:/")) {
> +        src = strchr(host + strlen("unix:"), ':');
> +        if (src)
> +            *src++ = '\0';

Same comments as previous patches.

>
> -    *port++ = '\0';
> -    h->name = strdup(host);
> -    if (!h->name)
> -        goto no_memory;
> +        h->transport = VIR_DOMAIN_DISK_PROTO_TRANS_UNIX;
> +        h->socket = strdup(host + strlen("unix:"));
> +    } else {
> +        port = strchr(host, ':');
> +        if (!port) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("cannot parse nbd filename '%s'"), disk->src);
> +            goto error;
> +        }
>
> -    src = strchr(port, ':');
> -    if (src)
> -        *src++ = '\0';
> +        *port++ = '\0';
> +        h->name = strdup(host);
> +        if (!h->name)
> +            goto no_memory;
>
> -    h->port = strdup(port);
> -    if (!h->port)
> -        goto no_memory;
> +        src = strchr(port, ':');
> +        if (src)
> +            *src++ = '\0';
> +
> +        h->port = strdup(port);
> +        if (!h->port)
> +            goto no_memory;
> +    }
>
>       if (src&&  STRPREFIX(src, "exportname=")) {
>           src = strdup(strchr(src, '=') + 1);
> @@ -2261,6 +2270,14 @@ qemuBuildNBDString(virDomainDiskDefPtr disk, virBufferPtr opt)
>           virBufferEscape(opt, ',', ",", ":%s",
>                           disk->hosts->port ? disk->hosts->port : "10809");
>           break;
> +    case VIR_DOMAIN_DISK_PROTO_TRANS_UNIX:
> +        if (!disk->hosts->socket) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("socket attribute required for unix transport"));
> +            return -1;
> +        }

Not sure if we should do this when parsing, as I think the "socket" is
required as long as the transport protocol is "unix", unless it's
generated somewhere.

Others look good.

Osier