Re: [Qemu-block] [PATCH v4 13/25] block/nbd: Implement bdrv_dirname()

[Eric Blake]

On 01/16/2017 02:49 PM, Max Reitz wrote:
> The idea behind this implementation is that the export name might be
> interpreted as a path (which is the only apparent interpretation of
> relative filenames for NBD paths).
> 
> The default implementation of bdrv_dirname() would handle that just fine
> for nbd+tcp, but not for nbd+unix, because in that case, the last
> element of the path is the Unix socket path and not the export name.
> Therefore, we need to implement an own bdrv_dirname() which uses the
> legacy NBD URL which has the export name at the end.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/nbd.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 35f24be069..42f0cd638c 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -552,6 +552,34 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
> QDict *options)
>      bs->full_open_options = opts;
>  }
>  
> +static char *nbd_dirname(BlockDriverState *bs, Error **errp)
> +{
> +    BDRVNBDState *s = bs->opaque;
> +    const char *host = NULL, *port = NULL, *path = NULL;
> +
> +    if (s->saddr->type == SOCKET_ADDRESS_KIND_INET) {
> +        const InetSocketAddress *inet = s->saddr->u.inet.data;
> +        if (!inet->has_ipv4 && !inet->has_ipv6 && !inet->has_to) {
> +            host = inet->host;
> +            port = inet->port;
> +        }
> +    } else if (s->saddr->type == SOCKET_ADDRESS_KIND_UNIX) {
> +        path = s->saddr->u.q_unix.data->path;
> +    }
> +

Should we be catering to SOCKET_ADDRESS_KIND_VSOCK or
SOCKET_ADDRESS_KIND_FD (if only to assert that they aren't possible with
NBD)?

> +    if (path) {
> +        return g_strdup_printf("nbd:unix:%s:exportname=%s%s", path,
> +                               s->export ?: "", s->export ? "/" : "");
> +    } else if (host) {
> +        return g_strdup_printf("nbd://%s:%s/%s%s", host, port,
> +                               s->export ?: "", s->export ? "/" : "");
> +    }
> +
> +    error_setg(errp, "Cannot generate a base directory for NBD node '%s'",
> +               bs->filename);
> +    return NULL;
> +}
> +

The NBD protocol doesn't require the server to support directories, but
also doesn't place any requirements on export names, so this approach
assumes a particular server behavior, but is probably as good as any
other approach.  But I'm still not sold that we need this, vs. just
declaring that NBD has no directory structure and therefore we always
return NULL (even with nbd+tcp, and whether or not the older nbd: URI
was used).  So I'm not quite ready for R-b on this one yet.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature