Message ID | 20190112175812.27068-18-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Series | nbd: add qemu-nbd --list | expand |
12.01.2019 20:58, 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, it is time to 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 > > Note that the output only lists sizes if the server sent > NBD_FLAG_HAS_FLAGS, because a newstyle server does not give > the size otherwise. It has the side effect that for really > old servers that did not send any flags, the size is not > output even though it was available. However, I'm not too > concerned about that - oldstyle servers are (rightfully) > getting less common to encounter (qemu 3.0 was the last > version where we even serve it), and most existing servers > that still even offer oldstyle negotiation (such as nbdkit) > still send flags (since that was added to the NBD protocol > in 2007 to permit read-only connections). > > Not done here, but maybe worth future experiments: capture > the meat of NBDExportInfo into a QAPI struct, and use the > generated QAPI pretty-printers instead of hand-rolling our > output loop. It would also permit us to add a JSON output > mode for machine parsing. > > Signed-off-by: Eric Blake <eblake@redhat.com> > Message-Id: <20181215135324.152629-21-eblake@redhat.com> > Reviewed-by: Richard W.M. Jones <rjones@redhat.com> > > --- > v3: comment tweak [Rich], rebase to earlier changes > --- > qemu-nbd.texi | 27 +++++++-- > qemu-nbd.c | 155 +++++++++++++++++++++++++++++++++++++++++++++----- > 2 files changed, 165 insertions(+), 17 deletions(-) > > diff --git a/qemu-nbd.texi b/qemu-nbd.texi > index 3f22559beb4..65caeb7874a 100644 > --- a/qemu-nbd.texi > +++ b/qemu-nbd.texi > @@ -2,6 +2,8 @@ > @c man begin SYNOPSIS > @command{qemu-nbd} [OPTION]... @var{filename} > > +@command{qemu-nbd} @option{-L} [OPTION]... > + > @command{qemu-nbd} @option{-d} @var{dev} > @c man end > @end example > @@ -14,6 +16,8 @@ Other uses: > @itemize > @item > Bind a /dev/nbdX block device to a QEMU server (on Linux). > +@item > +As a client to query exports of a remote NBD server. > @end itemize > > @c man end > @@ -31,13 +35,15 @@ See the @code{qemu(1)} manual page for full details of the properties > supported. The common object types that it makes sense to define are the > @code{secret} object, which is used to supply passwords and/or encryption > keys, and the @code{tls-creds} object, which is used to supply TLS > -credentials for the qemu-nbd server. > +credentials for the qemu-nbd server or client. > @item -p, --port=@var{port} > -The TCP port to listen on (default @samp{10809}). > +The TCP port to listen on as a server, or connect to as a client > +(default @samp{10809}). > @item -o, --offset=@var{offset} > The offset into the image. > @item -b, --bind=@var{iface} > -The interface to bind to (default @samp{0.0.0.0}). > +The interface to bind to as a server, or connect to as a client > +(default @samp{0.0.0.0}). > @item -k, --socket=@var{path} > Use a unix socket with path @var{path}. > @item --image-opts > @@ -97,10 +103,14 @@ Set the NBD volume export name (default of a zero-length string). > @item -D, --description=@var{description} > Set the NBD volume export description, as a human-readable > string. > +@item -L, --list > +Connect as a client and list all details about the exports exposed by > +a remote NBD server. > @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 > -option. > +option; or provide the credentials needed for connecting as a client > +in list mode. may be "list mode (--list)", as "list mode" is not directly defined. On the other hand, list option is extremely close to tls-creds, so it is obvious anyway. > @item --fork > Fork off the server process and exit the parent once the server is running. > @item -v, --verbose > @@ -159,6 +169,15 @@ qemu-nbd -c /dev/nbd0 -f qcow2 file.qcow2 > qemu-nbd -d /dev/nbd0 > @end example > > +Query a remote server to see details about what export(s) it is > +serving on port 10809, and authenticating via PSK: > + > +@example > +qemu-nbd \ > + --object tls-creds-psk,id=tls0,dir=/tmp/keys,username=eblake,endpoint=client \ > + --tls-creds tls0 -L -b remote.example.com > +@end example > + > @c man end > > @ignore > diff --git a/qemu-nbd.c b/qemu-nbd.c > index f1c24683129..daccb86d0d7 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -76,7 +76,8 @@ static void usage(const char *name) > { > (printf) ( > "Usage: %s [OPTIONS] FILE\n" > -"QEMU Disk Network Block Device Server\n" > +" or: %s -L [OPTIONS]\n" > +"QEMU Disk Network Block Device Utility\n" > "\n" > " -h, --help display this help and exit\n" > " -V, --version output version information and exit\n" > @@ -98,6 +99,7 @@ static void usage(const char *name) > " -B, --bitmap=NAME expose a persistent dirty bitmap\n" > "\n" > "General purpose options:\n" > +" -L, --list list exports available from another NBD server\n" > " --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" > @@ -131,7 +133,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) > @@ -243,6 +245,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; May be just return EXIT_FAUILURE here; remove out label; s/out_socket/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) { actually this is if (server not have a bug of not setting NBD_FLAG_HAS_FLAGS) { ... } Why not to print @size for example, if @flags field has a bug? Or, then, why to print flags, if @size has a bug? > + printf(" size: %" PRIu64 "\n", list[i].size); > + 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; > +} > + > + > #if HAVE_NBD_DEVICE > static void *show_parts(void *arg) > { > @@ -425,7 +513,8 @@ static QemuOptsList qemu_object_opts = { > > > > -static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp) > +static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, bool list, > + Error **errp) > { > Object *obj; > QCryptoTLSCreds *creds; > @@ -445,10 +534,18 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp) > return NULL; > } > > - if (creds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) { > - error_setg(errp, > - "Expecting TLS credentials with a server endpoint"); > - return NULL; > + if (list) { > + if (creds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) { > + error_setg(errp, > + "Expecting TLS credentials with a client endpoint"); > + return NULL; > + } > + } else { > + if (creds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) { > + error_setg(errp, > + "Expecting TLS credentials with a server endpoint"); > + return NULL; > + } > } > object_ref(obj); > return creds; > @@ -471,7 +568,8 @@ static void setup_address_and_port(const char **address, const char **port) > static const char *socket_activation_validate_opts(const char *device, > const char *sockpath, > const char *address, > - const char *port) > + const char *port, > + bool list) > { > if (device != NULL) { > return "NBD device can't be set when using socket activation"; > @@ -489,6 +587,10 @@ static const char *socket_activation_validate_opts(const char *device, > return "TCP port number can't be set when using socket activation"; > } > > + if (list) { > + return "List mode is incompatible with socket activation"; > + } > + > return NULL; > } > > @@ -512,7 +614,7 @@ int main(int argc, char **argv) > int64_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:T:D:B:"; > + const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:B:L"; > struct option lopt[] = { > { "help", no_argument, NULL, 'h' }, > { "version", no_argument, NULL, 'V' }, > @@ -525,6 +627,7 @@ int main(int argc, char **argv) > { "bitmap", required_argument, NULL, 'B' }, > { "connect", required_argument, NULL, 'c' }, > { "disconnect", no_argument, NULL, 'd' }, > + { "list", no_argument, NULL, 'L' }, > { "snapshot", no_argument, NULL, 's' }, > { "load-snapshot", required_argument, NULL, 'l' }, > { "nocache", no_argument, NULL, 'n' }, > @@ -559,7 +662,7 @@ int main(int argc, char **argv) > Error *local_err = NULL; > BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; > QDict *options = NULL; > - const char *export_name = ""; /* Default export name */ > + const char *export_name = NULL; /* defaults to "" later for server mode */ > const char *export_description = NULL; > const char *bitmap = NULL; > const char *tlscredsid = NULL; > @@ -567,6 +670,7 @@ int main(int argc, char **argv) > bool writethrough = true; > char *trace_file = NULL; > bool fork_process = false; > + bool list = false; > int old_stderr = -1; > unsigned socket_activation; > > @@ -760,13 +864,32 @@ int main(int argc, char **argv) > case QEMU_NBD_OPT_FORK: > fork_process = true; > break; > + case 'L': > + list = true; > + break; > } > } > > - if ((argc - optind) != 1) { > + if (list) { > + if (argc != optind) { > + error_report("List mode is incompatible with a file name"); > + exit(EXIT_FAILURE); > + } > + if (export_name || export_description || dev_offset || partition || > + device || disconnect || fmt || sn_id_or_name || bitmap) { > + error_report("List mode is incompatible with per-device settings"); > + exit(EXIT_FAILURE); and what about aio, discard, etc? Also, I think, it would be good to specify in Usage (or in man), which options are available for list mode. Hm, note, --help has grouping of options and man don't. > + } > + if (fork_process) { > + error_report("List mode is incompatible with forking"); > + exit(EXIT_FAILURE); > + } > + } else if ((argc - optind) != 1) { > error_report("Invalid number of arguments"); > error_printf("Try `%s --help' for more information.\n", argv[0]); > exit(EXIT_FAILURE); > + } else if (!export_name) { > + export_name = ""; > } > > qemu_opts_foreach(&qemu_object_opts, > @@ -785,7 +908,8 @@ int main(int argc, char **argv) > } else { > /* Using socket activation - check user didn't use -p etc. */ > const char *err_msg = socket_activation_validate_opts(device, sockpath, > - bindto, port); > + bindto, port, > + list); > if (err_msg != NULL) { > error_report("%s", err_msg); > exit(EXIT_FAILURE); > @@ -808,7 +932,7 @@ int main(int argc, char **argv) > error_report("TLS is not supported with a host device"); > exit(EXIT_FAILURE); > } > - tlscreds = nbd_get_tls_creds(tlscredsid, &local_err); > + tlscreds = nbd_get_tls_creds(tlscredsid, list, &local_err); > if (local_err) { > error_report("Failed to get TLS creds %s", > error_get_pretty(local_err)); > @@ -816,6 +940,11 @@ int main(int argc, char **argv) > } > } > > + if (list) { > + saddr = nbd_build_socket_address(sockpath, bindto, port); > + return qemu_nbd_client_list(saddr, tlscreds, bindto); > + } > + > #if !HAVE_NBD_DEVICE > if (disconnect || device) { > error_report("Kernel /dev/nbdN support not available"); > I don't have good understanding of tls related things, the rest looks OK, my suggestions are optional, so, if you don't want to improve docs and option conflict checking now: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
On 1/17/19 4:05 AM, Vladimir Sementsov-Ogievskiy wrote: > 12.01.2019 20:58, 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, it is time to 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). >> >> >> Not done here, but maybe worth future experiments: capture >> the meat of NBDExportInfo into a QAPI struct, and use the >> generated QAPI pretty-printers instead of hand-rolling our >> output loop. It would also permit us to add a JSON output >> mode for machine parsing. A start of that experiment has now been posted: https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg04196.html >> @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 >> -option. >> +option; or provide the credentials needed for connecting as a client >> +in list mode. > > may be "list mode (--list)", as "list mode" is not directly defined. On the other hand, > list option is extremely close to tls-creds, so it is obvious anyway. I'm thinking of adding this (and see conversation below that mentions [1]): diff --git i/qemu-nbd.texi w/qemu-nbd.texi index 65caeb7874a..0d297eed6db 100644 --- i/qemu-nbd.texi +++ w/qemu-nbd.texi @@ -105,7 +105,9 @@ Set the NBD volume export description, as a human-readable string. @item -L, --list Connect as a client and list all details about the exports exposed by -a remote NBD server. +a remote NBD server. This enables list mode, and is incompatible +with options that change behavior related to a specific export (such as +@option{--export-name}, @option{--offset}, ...). @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 >> +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; > > May be just return EXIT_FAUILURE here; > remove out label; > s/out_socket/out Will do. Probably leftovers from earlier attempts as I changed my approach over time. >> + 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) { > > actually this is > > if (server not have a bug of not setting NBD_FLAG_HAS_FLAGS) { Which, as the commit message mentions, is for servers so old and rare that it really doesn't matter. > ... > } > > Why not to print @size for example, if @flags field has a bug? > > Or, then, why to print flags, if @size has a bug? Because we don't have to worry about those servers being mainstream. >> >> - if ((argc - optind) != 1) { >> + if (list) { >> + if (argc != optind) { >> + error_report("List mode is incompatible with a file name"); >> + exit(EXIT_FAILURE); >> + } >> + if (export_name || export_description || dev_offset || partition || >> + device || disconnect || fmt || sn_id_or_name || bitmap) { >> + error_report("List mode is incompatible with per-device settings"); >> + exit(EXIT_FAILURE); > > and what about aio, discard, etc? I don't mind adding in any more options that you think are useful to flag the user on. Looks like I missed seen_aio, seen_cache, seen_discard. Catching '-s' is harder, as it merely sets a bit within flags rather than a witness variable. > Also, I think, it would be good to specify in Usage > (or in man), which options are available for list mode. I worry that keeping an exact list may be a maintenance nightmare (the two are likely to get out of sync); does my proposed wording above at [1] satisfy the problem by at least making the user aware that not all combinations will work? Another alternative would be to just silently ignore all per-export options, instead of warning the user that they are incompatible. I don't know if that's any friendlier, but it is less code. > > Hm, note, --help has grouping of options and man don't. That could be a separate patch, if it is desired (or squashed into 2/19) > > > I don't have good understanding of tls related things, the rest looks OK, > my suggestions are optional, so, if you don't want to improve docs and > option conflict checking now: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > >
17.01.2019 19:58, Eric Blake wrote: > On 1/17/19 4:05 AM, Vladimir Sementsov-Ogievskiy wrote: >> 12.01.2019 20:58, 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, it is time to 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). >>> > >>> >>> Not done here, but maybe worth future experiments: capture >>> the meat of NBDExportInfo into a QAPI struct, and use the >>> generated QAPI pretty-printers instead of hand-rolling our >>> output loop. It would also permit us to add a JSON output >>> mode for machine parsing. > > A start of that experiment has now been posted: > https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg04196.html > > >>> @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 >>> -option. >>> +option; or provide the credentials needed for connecting as a client >>> +in list mode. >> >> may be "list mode (--list)", as "list mode" is not directly defined. On the other hand, >> list option is extremely close to tls-creds, so it is obvious anyway. > > I'm thinking of adding this (and see conversation below that mentions [1]): > > diff --git i/qemu-nbd.texi w/qemu-nbd.texi > index 65caeb7874a..0d297eed6db 100644 > --- i/qemu-nbd.texi > +++ w/qemu-nbd.texi > @@ -105,7 +105,9 @@ Set the NBD volume export description, as a > human-readable > string. > @item -L, --list > Connect as a client and list all details about the exports exposed by > -a remote NBD server. > +a remote NBD server. This enables list mode, and is incompatible > +with options that change behavior related to a specific export (such as > +@option{--export-name}, @option{--offset}, ...). > @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 > > >>> +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; >> >> May be just return EXIT_FAUILURE here; >> remove out label; >> s/out_socket/out > > Will do. Probably leftovers from earlier attempts as I changed my > approach over time. > > >>> + 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) { >> >> actually this is >> >> if (server not have a bug of not setting NBD_FLAG_HAS_FLAGS) { > > Which, as the commit message mentions, is for servers so old and rare > that it really doesn't matter. > >> ... >> } >> >> Why not to print @size for example, if @flags field has a bug? >> >> Or, then, why to print flags, if @size has a bug? > > Because we don't have to worry about those servers being mainstream. > >>> >>> - if ((argc - optind) != 1) { >>> + if (list) { >>> + if (argc != optind) { >>> + error_report("List mode is incompatible with a file name"); >>> + exit(EXIT_FAILURE); >>> + } >>> + if (export_name || export_description || dev_offset || partition || >>> + device || disconnect || fmt || sn_id_or_name || bitmap) { >>> + error_report("List mode is incompatible with per-device settings"); >>> + exit(EXIT_FAILURE); >> >> and what about aio, discard, etc? > > I don't mind adding in any more options that you think are useful to > flag the user on. Looks like I missed seen_aio, seen_cache, > seen_discard. Catching '-s' is harder, as it merely sets a bit within > flags rather than a witness variable. > >> Also, I think, it would be good to specify in Usage >> (or in man), which options are available for list mode. > > I worry that keeping an exact list may be a maintenance nightmare (the > two are likely to get out of sync); does my proposed wording above at > [1] satisfy the problem by at least making the user aware that not all > combinations will work? I don't really care of it, current version is OK too. Of course, an addition sounds better than nothing) > > Another alternative would be to just silently ignore all per-export > options, instead of warning the user that they are incompatible. I > don't know if that's any friendlier, but it is less code. > >> >> Hm, note, --help has grouping of options and man don't. > > That could be a separate patch, if it is desired (or squashed into 2/19) > >> >> >> I don't have good understanding of tls related things, the rest looks OK, >> my suggestions are optional, so, if you don't want to improve docs and >> option conflict checking now: >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> >> >
diff --git a/qemu-nbd.texi b/qemu-nbd.texi index 3f22559beb4..65caeb7874a 100644 --- a/qemu-nbd.texi +++ b/qemu-nbd.texi @@ -2,6 +2,8 @@ @c man begin SYNOPSIS @command{qemu-nbd} [OPTION]... @var{filename} +@command{qemu-nbd} @option{-L} [OPTION]... + @command{qemu-nbd} @option{-d} @var{dev} @c man end @end example @@ -14,6 +16,8 @@ Other uses: @itemize @item Bind a /dev/nbdX block device to a QEMU server (on Linux). +@item +As a client to query exports of a remote NBD server. @end itemize @c man end @@ -31,13 +35,15 @@ See the @code{qemu(1)} manual page for full details of the properties supported. The common object types that it makes sense to define are the @code{secret} object, which is used to supply passwords and/or encryption keys, and the @code{tls-creds} object, which is used to supply TLS -credentials for the qemu-nbd server. +credentials for the qemu-nbd server or client. @item -p, --port=@var{port} -The TCP port to listen on (default @samp{10809}). +The TCP port to listen on as a server, or connect to as a client +(default @samp{10809}). @item -o, --offset=@var{offset} The offset into the image. @item -b, --bind=@var{iface} -The interface to bind to (default @samp{0.0.0.0}). +The interface to bind to as a server, or connect to as a client +(default @samp{0.0.0.0}). @item -k, --socket=@var{path} Use a unix socket with path @var{path}. @item --image-opts @@ -97,10 +103,14 @@ Set the NBD volume export name (default of a zero-length string). @item -D, --description=@var{description} Set the NBD volume export description, as a human-readable string. +@item -L, --list +Connect as a client and list all details about the exports exposed by +a remote NBD server. @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 -option. +option; or provide the credentials needed for connecting as a client +in list mode. @item --fork Fork off the server process and exit the parent once the server is running. @item -v, --verbose @@ -159,6 +169,15 @@ qemu-nbd -c /dev/nbd0 -f qcow2 file.qcow2 qemu-nbd -d /dev/nbd0 @end example +Query a remote server to see details about what export(s) it is +serving on port 10809, and authenticating via PSK: + +@example +qemu-nbd \ + --object tls-creds-psk,id=tls0,dir=/tmp/keys,username=eblake,endpoint=client \ + --tls-creds tls0 -L -b remote.example.com +@end example + @c man end @ignore diff --git a/qemu-nbd.c b/qemu-nbd.c index f1c24683129..daccb86d0d7 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -76,7 +76,8 @@ static void usage(const char *name) { (printf) ( "Usage: %s [OPTIONS] FILE\n" -"QEMU Disk Network Block Device Server\n" +" or: %s -L [OPTIONS]\n" +"QEMU Disk Network Block Device Utility\n" "\n" " -h, --help display this help and exit\n" " -V, --version output version information and exit\n" @@ -98,6 +99,7 @@ static void usage(const char *name) " -B, --bitmap=NAME expose a persistent dirty bitmap\n" "\n" "General purpose options:\n" +" -L, --list list exports available from another NBD server\n" " --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" @@ -131,7 +133,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) @@ -243,6 +245,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); + 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; +} + + #if HAVE_NBD_DEVICE static void *show_parts(void *arg) { @@ -425,7 +513,8 @@ static QemuOptsList qemu_object_opts = { -static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp) +static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, bool list, + Error **errp) { Object *obj; QCryptoTLSCreds *creds; @@ -445,10 +534,18 @@ static QCryptoTLSCreds *nbd_get_tls_creds(const char *id, Error **errp) return NULL; } - if (creds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) { - error_setg(errp, - "Expecting TLS credentials with a server endpoint"); - return NULL; + if (list) { + if (creds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) { + error_setg(errp, + "Expecting TLS credentials with a client endpoint"); + return NULL; + } + } else { + if (creds->endpoint != QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) { + error_setg(errp, + "Expecting TLS credentials with a server endpoint"); + return NULL; + } } object_ref(obj); return creds; @@ -471,7 +568,8 @@ static void setup_address_and_port(const char **address, const char **port) static const char *socket_activation_validate_opts(const char *device, const char *sockpath, const char *address, - const char *port) + const char *port, + bool list) { if (device != NULL) { return "NBD device can't be set when using socket activation"; @@ -489,6 +587,10 @@ static const char *socket_activation_validate_opts(const char *device, return "TCP port number can't be set when using socket activation"; } + if (list) { + return "List mode is incompatible with socket activation"; + } + return NULL; } @@ -512,7 +614,7 @@ int main(int argc, char **argv) int64_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:T:D:B:"; + const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:x:T:D:B:L"; struct option lopt[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, 'V' }, @@ -525,6 +627,7 @@ int main(int argc, char **argv) { "bitmap", required_argument, NULL, 'B' }, { "connect", required_argument, NULL, 'c' }, { "disconnect", no_argument, NULL, 'd' }, + { "list", no_argument, NULL, 'L' }, { "snapshot", no_argument, NULL, 's' }, { "load-snapshot", required_argument, NULL, 'l' }, { "nocache", no_argument, NULL, 'n' }, @@ -559,7 +662,7 @@ int main(int argc, char **argv) Error *local_err = NULL; BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF; QDict *options = NULL; - const char *export_name = ""; /* Default export name */ + const char *export_name = NULL; /* defaults to "" later for server mode */ const char *export_description = NULL; const char *bitmap = NULL; const char *tlscredsid = NULL; @@ -567,6 +670,7 @@ int main(int argc, char **argv) bool writethrough = true; char *trace_file = NULL; bool fork_process = false; + bool list = false; int old_stderr = -1; unsigned socket_activation; @@ -760,13 +864,32 @@ int main(int argc, char **argv) case QEMU_NBD_OPT_FORK: fork_process = true; break; + case 'L': + list = true; + break; } } - if ((argc - optind) != 1) { + if (list) { + if (argc != optind) { + error_report("List mode is incompatible with a file name"); + exit(EXIT_FAILURE); + } + if (export_name || export_description || dev_offset || partition || + device || disconnect || fmt || sn_id_or_name || bitmap) { + error_report("List mode is incompatible with per-device settings"); + exit(EXIT_FAILURE); + } + if (fork_process) { + error_report("List mode is incompatible with forking"); + exit(EXIT_FAILURE); + } + } else if ((argc - optind) != 1) { error_report("Invalid number of arguments"); error_printf("Try `%s --help' for more information.\n", argv[0]); exit(EXIT_FAILURE); + } else if (!export_name) { + export_name = ""; } qemu_opts_foreach(&qemu_object_opts, @@ -785,7 +908,8 @@ int main(int argc, char **argv) } else { /* Using socket activation - check user didn't use -p etc. */ const char *err_msg = socket_activation_validate_opts(device, sockpath, - bindto, port); + bindto, port, + list); if (err_msg != NULL) { error_report("%s", err_msg); exit(EXIT_FAILURE); @@ -808,7 +932,7 @@ int main(int argc, char **argv) error_report("TLS is not supported with a host device"); exit(EXIT_FAILURE); } - tlscreds = nbd_get_tls_creds(tlscredsid, &local_err); + tlscreds = nbd_get_tls_creds(tlscredsid, list, &local_err); if (local_err) { error_report("Failed to get TLS creds %s", error_get_pretty(local_err)); @@ -816,6 +940,11 @@ int main(int argc, char **argv) } } + if (list) { + saddr = nbd_build_socket_address(sockpath, bindto, port); + return qemu_nbd_client_list(saddr, tlscreds, bindto); + } + #if !HAVE_NBD_DEVICE if (disconnect || device) { error_report("Kernel /dev/nbdN support not available");