Re: [Qemu-block] [PATCH v2 18/22] nbd/client: Add nbd_receive_export_list()

[Eric Blake]

On 12/15/18 9:42 AM, Richard W.M. Jones wrote:
> On Sat, Dec 15, 2018 at 07:53:20AM -0600, 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
> > 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 adds the low-level client code for grabbing the list
> > of exports.  It benefits from the recent refactoring patches, as
> > well as a minor tweak of changing nbd_opt_go() to nbd_opt_info_or_go(),
> > in order to share as much code as possible when it comes to doing
> > validation of server replies.  The resulting information is stored
> > in an array of NBDExportInfo which has been expanded to any
> > description string, along with a convenience function for freeing
> > the list.
> >
> > Signed-off-by: Eric Blake <address@hidden>
> >
> > ---

> > +        while (1) {
> > +            char *name;
> > +            char *desc;
> > +
> > +            rc = nbd_receive_list(ioc, &name, &desc, errp);
> > +            if (rc < 0) {
> > +                goto out;
> > +            } else if (rc == 0) {
> > +                break;
> > +            }
> > +            array = g_renew(NBDExportInfo, array, ++count);
> > +            memset(&array[count - 1], 0, sizeof(*array));
> > +            array[count - 1].name = name;
> > +            array[count - 1].description = desc;
> > +            array[count - 1].structured_reply = result == 3;
> > +        }
>
> Do we care about limiting ‘count’ to some reasonable value here?

Maybe.

> I tried to look at the protocol document to see if there's a limit on
> the number of exports that a server can have, but if there is I cannot
> find it.  I don't know how much we care about malicious NBD servers --
> mostly I'm interested in malicious NBD clients :-)

You brought up the same question earlier in the series, and we already 
have a pre-existing repeat of the situation with NBD_OPT_LIST for 
servers that lack NBD_OPT_GO.

A client can always hang up on a server that it doesn't like; so maybe 
we could set our own limit - but what is a good limit?  An NBD server 
set up to export every file in a given directory, where that directory 
has over 1k files, doesn't seem too far-fetched.  Having 1000 contexts 
available in NBD_OPT_LIST_META_CONTEXT, on the other hand, is less likely.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org