Re: [Qemu-devel] [PATCH 13/14] qemu-nbd: Add --list option

[Vladimir Sementsov-Ogievskiy]

01.12.2018 1:03, Eric Blake wrote:
> We want to be able to detect whether a given qemu NBD server is
> exposing the right export(s) and dirty bitmaps, at least for
> regression testing.  We could use 'nbd-client -l' from the upstream
> NBD project to list exports, but it's annoying to rely on
> out-of-tree binaries; furthermore, nbd-client doesn't necessarily
> know about all of the qemu NBD extensions.  Thus, we plan on adding

ha, in this patch, not plan but add:)

> a new mode to qemu-nbd that merely sniffs all possible information
> from the server during handshake phase, then disconnects and dumps
> the information.
> 
> This patch actually implements --list/-L, while reusing other
> options such as --tls-creds for now designating how to connect
> as the client (rather than their non-list usage of how to operate
> as the server).
> 
> I debated about adding this functionality to something akin to
> 'qemu-img info' - but that tool does not readily lend itself
> to connecting to an arbitrary NBD server without also tying to
> a specific export (I may, however, still add ImageInfoSpecificNBD
> for reporting the bitmaps available when connecting to a single
> export).  And, while it may feel a bit odd that normally
> qemu-nbd is a server but 'qemu-nbd -L' is a client, we are not
> really making the qemu-nbd binary that much larger, because
> 'qemu-nbd -c' has to operate as both server and client
> simultaneously across two threads when feeding the kernel module
> for /dev/nbdN access.
> 
> Sample output:
> $ qemu-nbd -L
> exports available: 1
>   export: ''
>    size:  65536
>    flags: 0x4ed ( flush fua trim zeroes df cache )
>    min block: 512
>    opt block: 4096
>    max block: 33554432
>    available meta contexts: 1
>     base:allocation

don't you plan to bind this all to QAPI and expose in json?

> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>   qemu-nbd.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 141 insertions(+), 12 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c57053a0795..e19a841b869 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -76,6 +76,7 @@ static void usage(const char *name)
>   {
>       (printf) (
>   "Usage: %s [OPTIONS] FILE\n"
> +"  or:  %s -L [OPTIONS]\n"
>   "QEMU Disk Network Block Device Server\n"

Do anyone know, why thunderbird add additional space to the lines started from 
space when quoting,
which breaks indentation in quoted patches? How to fix it? I use plain text.

>   "\n"
>   "  -h, --help                display this help and exit\n"
> @@ -97,6 +98,7 @@ static void usage(const char *name)
>   "  -P, --partition=NUM       only expose partition NUM\n"
>   "\n"
>   "General purpose options:\n"
> +"  -L, --list                list NBD exports visible to client\n"

hm. I think, user who things that qemu-nbd is only a server, can understand 
this as
"dry run, don't actually start the server, but only list exports, which will be
available to clients, keeping in mind all other specified options".

so, may be, "list remote NBD server exports and options" or something like this?

>   "  --object type,id=ID,...   define an object such as 'secret' for 
> providing\n"
>   "                            passwords and/or encryption keys\n"
>   "  --tls-creds=ID            use id of an earlier --object to provide TLS\n"
> @@ -130,7 +132,7 @@ static void usage(const char *name)
>   "      --image-opts          treat FILE as a full set of image options\n"
>   "\n"
>   QEMU_HELP_BOTTOM "\n"
> -    , name, NBD_DEFAULT_PORT, "DEVICE");
> +    , name, name, NBD_DEFAULT_PORT, "DEVICE");
>   }
> 
>   static void version(const char *name)
> @@ -242,6 +244,92 @@ static void termsig_handler(int signum)
>   }
> 
> 
> +static int qemu_nbd_client_list(SocketAddress *saddr, QCryptoTLSCreds *tls,
> +                                const char *hostname)
> +{
> +    int ret = EXIT_FAILURE;
> +    int rc;
> +    Error *err = NULL;
> +    QIOChannelSocket *sioc;
> +    NBDExportInfo *list;
> +    int i, j;
> +
> +    sioc = qio_channel_socket_new();
> +    if (qio_channel_socket_connect_sync(sioc, saddr, &err) < 0) {
> +        error_report_err(err);
> +        goto out;
> +    }
> +    rc = nbd_receive_export_list(QIO_CHANNEL(sioc), tls, hostname, &list,
> +                                 &err);
> +    if (rc < 0) {
> +        if (err) {
> +            error_report_err(err);
> +        }
> +        goto out_socket;
> +    }
> +    printf("exports available: %d\n", rc);
> +    for (i = 0; i < rc; i++) {
> +        printf(" export: '%s'\n", list[i].name);
> +        if (list[i].description && *list[i].description) {
> +            printf("  description: %s\n", list[i].description);
> +        }
> +        if (list[i].flags & NBD_FLAG_HAS_FLAGS) {
> +            printf("  size:  %" PRIu64 "\n", list[i].size);

size is available only if NBD_FLAG_HAS_FLAGS? at least, accordingly to code,
in case 0: size is unrelated to flags.

> +            printf("  flags: 0x%x (", list[i].flags);
> +            if (list[i].flags & NBD_FLAG_READ_ONLY) {
> +                printf(" readonly");
> +            }
> +            if (list[i].flags & NBD_FLAG_SEND_FLUSH) {
> +                printf(" flush");
> +            }
> +            if (list[i].flags & NBD_FLAG_SEND_FUA) {
> +                printf(" fua");
> +            }
> +            if (list[i].flags & NBD_FLAG_ROTATIONAL) {
> +                printf(" rotational");
> +            }
> +            if (list[i].flags & NBD_FLAG_SEND_TRIM) {
> +                printf(" trim");
> +            }
> +            if (list[i].flags & NBD_FLAG_SEND_WRITE_ZEROES) {
> +                printf(" zeroes");
> +            }
> +            if (list[i].flags & NBD_FLAG_SEND_DF) {
> +                printf(" df");
> +            }
> +            if (list[i].flags & NBD_FLAG_CAN_MULTI_CONN) {
> +                printf(" multi");
> +            }
> +            if (list[i].flags & NBD_FLAG_SEND_RESIZE) {
> +                printf(" resize");
> +            }
> +            if (list[i].flags & NBD_FLAG_SEND_CACHE) {
> +                printf(" cache");
> +            }
> +            printf(" )\n");
> +        }
> +        if (list[i].min_block) {
> +            printf("  min block: %u\n", list[i].min_block);
> +            printf("  opt block: %u\n", list[i].opt_block);
> +            printf("  max block: %u\n", list[i].max_block);
> +        }
> +        if (list[i].n_contexts) {
> +            printf("  available meta contexts: %d\n", list[i].n_contexts);
> +            for (j = 0; j < list[i].n_contexts; j++) {
> +                printf("   %s\n", list[i].contexts[j]);
> +            }
> +        }
> +    }
> +    nbd_free_export_list(list, rc);
> +
> +    ret = EXIT_SUCCESS;
> + out_socket:
> +    object_unref(OBJECT(sioc));
> + out:
> +    return ret;
> +}
> +


-- 
Best regards,
Vladimir