Re: [Qemu-block] [Qemu-devel] [PATCH v3 08/44] nbd: Add qemu-nbd -D for human-readable description

[Alex Bligh]

On 23 Apr 2016, at 00:40, Eric Blake <address@hidden> wrote:

> The NBD protocol allows servers to advertise a human-readable
> description alongside an export name during NBD_OPT_LIST.  Add
> an option to pass through the user's string to the NBD client.
> 
> Doing this also makes it easier to test commit 200650d4, which
> is the client counterpart of receiving the description.
> 
> Signed-off-by: Eric Blake <address@hidden>

Reviewed-by: Alex Bligh <address@hidden>

> ---
> include/block/nbd.h |  1 +
> nbd/nbd-internal.h  |  5 +++--
> nbd/server.c        | 34 ++++++++++++++++++++++++++--------
> qemu-nbd.c          | 12 +++++++++++-
> qemu-nbd.texi       |  5 ++++-
> 5 files changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 134f117..3e2d76b 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -107,6 +107,7 @@ BlockBackend *nbd_export_get_blockdev(NBDExport *exp);
> 
> NBDExport *nbd_export_find(const char *name);
> void nbd_export_set_name(NBDExport *exp, const char *name);
> +void nbd_export_set_description(NBDExport *exp, const char *description);
> void nbd_export_close_all(void);
> 
> void nbd_client_new(NBDExport *exp,
> diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
> index 3791535..035ead4 100644
> --- a/nbd/nbd-internal.h
> +++ b/nbd/nbd-internal.h
> @@ -103,9 +103,10 @@ static inline ssize_t read_sync(QIOChannel *ioc, void 
> *buffer, size_t size)
>     return nbd_wr_syncv(ioc, &iov, 1, 0, size, true);
> }
> 
> -static inline ssize_t write_sync(QIOChannel *ioc, void *buffer, size_t size)
> +static inline ssize_t write_sync(QIOChannel *ioc, const void *buffer,
> +                                 size_t size)
> {
> -    struct iovec iov = { .iov_base = buffer, .iov_len = size };
> +    struct iovec iov = { .iov_base = (void *) buffer, .iov_len = size };
> 
>     return nbd_wr_syncv(ioc, &iov, 1, 0, size, false);
> }
> diff --git a/nbd/server.c b/nbd/server.c
> index 31fc9cf..aa252a4 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -61,6 +61,7 @@ struct NBDExport {
> 
>     BlockBackend *blk;
>     char *name;
> +    char *description;
>     off_t dev_offset;
>     off_t size;
>     uint16_t nbdflags;
> @@ -128,7 +129,8 @@ static ssize_t nbd_negotiate_read(QIOChannel *ioc, void 
> *buffer, size_t size)
> 
> }
> 
> -static ssize_t nbd_negotiate_write(QIOChannel *ioc, void *buffer, size_t 
> size)
> +static ssize_t nbd_negotiate_write(QIOChannel *ioc, const void *buffer,
> +                                   size_t size)
> {
>     ssize_t ret;
>     guint watch;
> @@ -224,11 +226,15 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, 
> uint32_t type, uint32_t opt)
> 
> static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
> {
> -    uint64_t magic, name_len;
> +    uint64_t magic;
> +    size_t name_len, desc_len;
>     uint32_t opt, type, len;
> +    const char *name = exp->name ? exp->name : "";
> +    const char *desc = exp->description ? exp->description : "";
> 
> -    TRACE("Advertising export name '%s'", exp->name ? exp->name : "");
> -    name_len = strlen(exp->name);
> +    TRACE("Advertising export name '%s' description '%s'", name, desc);
> +    name_len = strlen(name);
> +    desc_len = strlen(desc);
>     magic = cpu_to_be64(NBD_REP_MAGIC);
>     if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
>         LOG("write failed (magic)");
> @@ -244,18 +250,22 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, 
> NBDExport *exp)
>         LOG("write failed (reply type)");
>         return -EINVAL;
>     }
> -    len = cpu_to_be32(name_len + sizeof(len));
> +    len = cpu_to_be32(name_len + desc_len + sizeof(len));
>     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
>         LOG("write failed (length)");
>         return -EINVAL;
>     }
>     len = cpu_to_be32(name_len);
>     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
> -        LOG("write failed (length)");
> +        LOG("write failed (name length)");
>         return -EINVAL;
>     }
> -    if (nbd_negotiate_write(ioc, exp->name, name_len) != name_len) {
> -        LOG("write failed (buffer)");
> +    if (nbd_negotiate_write(ioc, name, name_len) != name_len) {
> +        LOG("write failed (name buffer)");
> +        return -EINVAL;
> +    }
> +    if (nbd_negotiate_write(ioc, desc, desc_len) != desc_len) {
> +        LOG("write failed (description buffer)");
>         return -EINVAL;
>     }
>     return 0;
> @@ -877,6 +887,12 @@ void nbd_export_set_name(NBDExport *exp, const char 
> *name)
>     nbd_export_put(exp);
> }
> 
> +void nbd_export_set_description(NBDExport *exp, const char *description)
> +{
> +    g_free(exp->description);
> +    exp->description = g_strdup(description);
> +}
> +
> void nbd_export_close(NBDExport *exp)
> {
>     NBDClient *client, *next;
> @@ -886,6 +902,7 @@ void nbd_export_close(NBDExport *exp)
>         client_close(client);
>     }
>     nbd_export_set_name(exp, NULL);
> +    nbd_export_set_description(exp, NULL);
>     nbd_export_put(exp);
> }
> 
> @@ -904,6 +921,7 @@ void nbd_export_put(NBDExport *exp)
> 
>     if (--exp->refcount == 0) {
>         assert(exp->name == NULL);
> +        assert(exp->description == NULL);
> 
>         if (exp->close) {
>             exp->close(exp);
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 71bfdeb..a85e98f 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -77,6 +77,7 @@ static void usage(const char *name)
> "  -t, --persistent          don't exit on the last connection\n"
> "  -v, --verbose             display extra debugging information\n"
> "  -x, --export-name=NAME    expose export by name\n"
> +"  -D, --description=TEXT    with -x, also export a human-readable 
> description\n"
> "\n"
> "Exposing part of the image:\n"
> "  -o, --offset=OFFSET       offset into the image\n"
> @@ -464,7 +465,7 @@ int main(int argc, char **argv)
>     off_t fd_size;
>     QemuOpts *sn_opts = NULL;
>     const char *sn_id_or_name = NULL;
> -    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:";
> +    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:D:";
>     struct option lopt[] = {
>         { "help", no_argument, NULL, 'h' },
>         { "version", no_argument, NULL, 'V' },
> @@ -490,6 +491,7 @@ int main(int argc, char **argv)
>         { "verbose", no_argument, NULL, 'v' },
>         { "object", required_argument, NULL, QEMU_NBD_OPT_OBJECT },
>         { "export-name", required_argument, NULL, 'x' },
> +        { "description", required_argument, NULL, 'D' },
>         { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
>         { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
>         { NULL, 0, NULL, 0 }
> @@ -509,6 +511,7 @@ int main(int argc, char **argv)
>     BlockdevDetectZeroesOptions detect_zeroes = 
> BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
>     QDict *options = NULL;
>     const char *export_name = NULL;
> +    const char *export_description = NULL;
>     const char *tlscredsid = NULL;
>     bool imageOpts = false;
>     bool writethrough = true;
> @@ -672,6 +675,9 @@ int main(int argc, char **argv)
>         case 'x':
>             export_name = optarg;
>             break;
> +        case 'D':
> +            export_description = optarg;
> +            break;
>         case 'v':
>             verbose = 1;
>             break;
> @@ -899,7 +905,11 @@ int main(int argc, char **argv)
>     }
>     if (export_name) {
>         nbd_export_set_name(exp, export_name);
> +        nbd_export_set_description(exp, export_description);
>         newproto = true;
> +    } else if (export_description) {
> +        error_report("Export description requires an export name");
> +        exit(EXIT_FAILURE);
>     }
> 
>     server_ioc = qio_channel_socket_new();
> diff --git a/qemu-nbd.texi b/qemu-nbd.texi
> index 9f23343..923de74 100644
> --- a/qemu-nbd.texi
> +++ b/qemu-nbd.texi
> @@ -79,9 +79,12 @@ Disconnect the device @var{dev}
> Allow up to @var{num} clients to share the device (default @samp{1})
> @item -t, --persistent
> Don't exit on the last connection
> address@hidden -x NAME, --export-name=NAME
> address@hidden -x, address@hidden
> Set the NBD volume export name. This switches the server to use
> the new style NBD protocol negotiation
> address@hidden -D, address@hidden
> +Set the NBD volume export description, as a human-readable
> +string. Requires the use of @option{-x}
> @item --tls-creds=ID
> Enable mandatory TLS encryption for the server by setting the ID
> of the TLS credentials object previously created with the --object
> -- 
> 2.5.5
> 
> 

-- 
Alex Bligh