search for: nbd_opt_info

qemu is about to add 'qemu-nbd --list', which exercises NBD_OPT_LIST and NBD_OPT_INFO to give the user as much detail as possible about an export without actually connecting to it. For that to display more than the export name when nbdkit is the server, we need to implement NBD_OPT_INFO. Thankfully, the NBD spec intentionally made the command very similar to NBD_OPT_GO, to the poi...

Most known NBD clients do not bother with NBD_OPT_INFO (except for clients like 'qemu-nbd --list' that don't ever intend to connect), but go straight to NBD_OPT_GO. However, it's not too hard to hack up qemu to add in an extra client step (whether info on the same name, or more interestingly, info on a different name), as a patch again...

...ic Blake + =head1 COPYRIGHT Copyright (C) 2020 Red Hat Inc. diff --git a/TODO b/TODO index 239e0b0..4a0cd22 100644 --- a/TODO +++ b/TODO @@ -9,12 +9,8 @@ Example code integrating with ppoll, pollfd, APR pollset (and others?). Example command line utils to copy in/out (like qemu-img convert). -NBD_OPT_INFO mode (like qemu-nbd -L). - NBD resize extension. -NBD_INFO_BLOCK_SIZE. - TLS should properly shut down the session (calling gnutls_bye). Performance: Chart it over various buffer sizes and threads, as that @@ -71,12 +67,3 @@ Suggested API improvements: maybe nbd_shutdown should wait for t...

...o nbdinfo, I'm going ahead and pushing this now. We may still want to add further nbd_opt_* commands for other fine-grained tuning of negotiation, but for now, I think things have stabilized on this end, and I can return to polishing .list_exports on the nbdkit end. Eric Blake (2): api: add nbd_opt_info, nbd_aio_opt_info info: Use nbd_opt_info for fewer handles during --list docs/libnbd.pod | 4 + info/nbdinfo.pod | 2 + generator/API.ml | 116 ++++++--- generator/states-newstyle-opt-go.c |...

...ith: >> >> nbdkit -U - info --run 'nbdsh -u nbd+unix:///foo\?socket=$unixsocket \ >> -c "print(h.pread(3, 0))"' >> >> but that's a lot more painful to write. >> >> We _don't_ need to advertise 'foo' or 'bar' over NBD_OPT_INFO (at >> least, not for plugins that don't implement the forthcoming >> .export_list callback), but _do_ need a way for the captive >> application (nbdsh in this case) to know _which_ export the captive >> should connect to. >> >> And, if we reinstate _just_ th...

The first one is the nastiest - it is an assertion failure caused by a spec-compliant client and introduced by our security fix that was released in 1.14.1. Eric Blake (4): server: Fix regression for NBD_OPT_INFO before NBD_OPT_GO server: Fix back-to-back SET_META_CONTEXT server: Forbid NUL in export and context names server: Fix OPT_GO on different export than SET_META_CONTEXT server/internal.h | 5 ++- server/backend.c | 13 ++++++ server/connections.c...

...uf.option.option = htobe32 (NBD_OPT_LIST); > + h->sbuf.option.optlen = 0; > + h->wbuf = &h->sbuf; > + h->wlen = sizeof (h->sbuf.option); > + SET_NEXT_STATE (%SEND); > + return 0; Is it worth enhancing the code to add another list mode where we can also query NBD_OPT_INFO on each export thus listed? (Compare to 'qemu-nbd --list'). Hmm, if we do that, list mode may need to be an enum instead of a bool. But that would be a followup because it adds more complexity; this is already a good addition on its own. > + NEWSTYLE.OPT_LIST.CHECK_REPLY: > +...

...Red Hat security review for whether this deserves a CVE > (presumably either both issues, or neither, will have a CVE) Both CVEs have now been assigned: CVE-2019-14850 - denial of service due to premature .open, depending on plugin used CVE-2019-14851 - denial of service due to assertion after NBD_OPT_INFO, independent of plugin -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...?) Also changing the NBD protocol is not under consideration. I think we need to consider these aspects separately: (a) How filters and plugins get the client exportname. (b) How filters and plugins respond when the client issues NBD_OPT_LIST. (c) How the server, filters and plugins respond to NBD_OPT_INFO. (a) Client exportname --------------------- The client sends the export name of the export it wants to access with NBD_OPT_EXPORT_NAME or the newer NBD_OPT_GO. This is an opaque string (not a filename, pathname etc). Currently filters and plugins can fetch this using nbdkit_export_name(). Ho...

...(3, 0))"' Of course, you can accomplish the same with: nbdkit -U - info --run 'nbdsh -u nbd+unix:///foo\?socket=$unixsocket \ -c "print(h.pread(3, 0))"' but that's a lot more painful to write. We _don't_ need to advertise 'foo' or 'bar' over NBD_OPT_INFO (at least, not for plugins that don't implement the forthcoming .export_list callback), but _do_ need a way for the captive application (nbdsh in this case) to know _which_ export the captive should connect to. And, if we reinstate _just_ that usage of -e,... >> (4) I have temporaril...

...t? > > (2) I found a bug in the new nbdinfo behaviour: > > $ nbdkit -fv file dir=/scratch > > (where /scratch is a directory with a lot of files in it) > > $ nbdinfo --version > libnbd 1.5.3 > $ nbdinfo --list nbd://localhost > [... lots of files shown ...] > nbd_opt_info: recv: Connection reset by peer > > nbdkit gives this error: > > nbdkit: file[1]: error: client exceeded maximum number of options (32) Yeah, we really ought to raise nbdkit's limits when .list_exports returns a large list. We haven't yet released nbdkit 1.24 where the fil...

...ink it has potential. > > I think we need to consider these aspects separately: > > (a) How filters and plugins get the client exportname. > > (b) How filters and plugins respond when the client issues NBD_OPT_LIST. > > (c) How the server, filters and plugins respond to NBD_OPT_INFO. I think (c) is already handled - they respond the same as they do to NBD_OPT_GO, but rather than starting to serve traffic, they go back to waiting for the next NBD_OPT_*. > > > (a) Client exportname > --------------------- > > The client sends the export name of the expor...

...t returned is the default export > > That could be expensive, reading the entire list and throwing away > all but the first. I'm now leaning towards two callbacks: > > .list_exports: compute all exports to advertise > .default_export: compute the name to be returned for > NBD_OPT_INFO("") and to use by default as $exportname in --run Maybe I don't understand this, but isn't NBD_OPT_INFO("") explicitly asking for info about the "" export name? In what situation would .default_export ever return anything other than ""? > Then,...

...nds before finally selecting an export or quitting, and we were justified in dropping indecisive chatty clients as being a waste of server resources. But now that we support .list_exports, it is reasonable for a client (such as 'qemu-nbd --list' or 'nbdinfo --list') to want to call NBD_OPT_INFO and several NBD_OPT_LIST_META_CONTEXT commands for every export returned in NBD_OPT_LIST. We still want to avoid clients that can tie us up in eternal handshaking, so let's reject a second call to NBD_OPT_LIST; but once the first call is made, the client now has a chance to get info on everyth...

...T_LIST); > >+ h->sbuf.option.optlen = 0; > >+ h->wbuf = &h->sbuf; > >+ h->wlen = sizeof (h->sbuf.option); > >+ SET_NEXT_STATE (%SEND); > >+ return 0; > > Is it worth enhancing the code to add another list mode where we can > also query NBD_OPT_INFO on each export thus listed? (Compare to > 'qemu-nbd --list'). Hmm, if we do that, list mode may need to be an > enum instead of a bool. But that would be a followup because it > adds more complexity; this is already a good addition on its own. I'm actually a little unclear...

...t; myplugin_open (int readonly) > { > const char *export = nbdkit_export_name (); > > ... Do something based on the export name ... > } I would also consider adding an optional callback: struct nbdkit_export { char *name; size_t size; // maybe other fields for supporting NBD_OPT_INFO... }; const nbdkit_export *(*list_exports) (void); A plugin that does not implement the callback advertises whatever export name was passed on the command line with -e (or defaults to advertising just ""), and accepts any name, or we could even state that NBD_OPT_LIST fails unless the c...

Of course at the moment nbdkit parses the NBD export name option but doesn't really do anything with it (except logging it). I wonder if we should make this available to plugins, in case they wish to serve different content to different clients based on the export name. Note I'm not suggesting that we use this feature in any existing plugins. If we wanted to do this there seem like two

...t;>> $ nbdkit -fv file dir=/scratch >>> >>> (where /scratch is a directory with a lot of files in it) >>> >>> $ nbdinfo --version >>> libnbd 1.5.3 >>> $ nbdinfo --list nbd://localhost >>> [... lots of files shown ...] >>> nbd_opt_info: recv: Connection reset by peer >>> >>> nbdkit gives this error: >>> >>> nbdkit: file[1]: error: client exceeded maximum number of options (32) >> >> Yeah, we really ought to raise nbdkit's limits when .list_exports >> returns a large list....

...-git a/generator/API.ml b/generator/API.ml index 36033817..d1849710 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -953,23 +953,24 @@ "set_request_meta_context", { (all C<nbd_connect_*> calls when L<nbd_set_opt_mode(3)> is false, or L<nbd_opt_go(3)> and L<nbd_opt_info(3)> when option mode is enabled) will also try to issue NBD_OPT_SET_META_CONTEXT when -the server supports structured replies and any contexts were -registered by L<nbd_add_meta_context(3)>. The default setting -is true; however the extra step of negotiating meta contexts is -not always...

With some crufty sed scripts we can generate functions that map from NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ"). This works on GNU sed and with FreeBSD, also with GNU sed's --posix option, so I guess the sed code is POSIX-compatible. Rich.