[Libguestfs] [PATCH nbdkit] server: Add nbdkit_export_name() to allow export name to be read.

[Richard W.M. Jones]

On Tue, Sep 10, 2019 at 08:11:10AM -0500, Eric Blake wrote:
> > +=head1 EXPORT NAME
> > +
> > +If the client negotiated an NBD export name with nbdkit then plugins
> > +may read this from any connected callbacks.  Nbdkit's normal behaviour
> > +is to accept any export name passed by the client, log it in debug
> > +output, but otherwise ignore it.  By using C<nbdkit_export_name>
> > +plugins may choose to filter by export name or serve different
> > +content.
> 
> We may want to add text here and/or in .can_multi_conn to clarify that
> advertising multi-conn only matters to clients connecting to the SAME
> export name; it is safe to advertise multi-conn even when serving
> different content to different export names.

Good point ...

> > +
> > +=head2 C<nbdkit_export_name>
> > +
> > + const char *nbdkit_export_name (void);
> > +
> > +Return the optional NBD export name if one was negotiated with the
> > +current client (this uses thread-local magic so no parameter is
> > +required).  The returned string is only valid while the client is
> > +connected, so if you need to store it in the plugin you must copy it.
> > +
> > +The export name is a free-form text string, it is not necessarily a
> > +path or filename and it does not need to begin with a C<'/'>
> > +character.  The NBD protocol describes the empty string (C<"">) as a
> > +representing a "default export" or to be used in cases where the
> > +export name does not make sense.
> > +
> > +This may return C<NULL> which does I<not> indicate an error:
> > +
> > +=over 4
> > +
> > +=item *
> > +
> > +It only returns the export name when there is a connected client.
> > +
> > +=item *
> > +
> > +If the server is using the oldstyle protocol the client does not send
> > +an export name.
> 
> Or, we could just blindly state that in oldstyle mode, the client always
> behaves as if it connected to the export named "".  After all, qemu 3.1
> was where we changed things to allow a client to request an explicit
> export name of "" yet still connect to an oldstyle server in that case.

Yes, we can do this, and then reserve NULL for an actual error
(calling in a non-connected state).

> > +++ b/server/protocol-handshake-newstyle.c
> > @@ -274,11 +274,17 @@ negotiate_handshake_newstyle_options (struct connection *conn)
> >        if (conn_recv_full (conn, data, optlen,
> >                            "read: %s: %m", name_of_nbd_opt (option)) == -1)
> >          return -1;
> > -      /* Apart from printing it, ignore the export name. */
> > +      /* Print the export name and save it in the connection. */
> >        data[optlen] = '\0';
> > -      debug ("newstyle negotiation: %s: "
> > -             "client requested export '%s' (ignored)",
> > +      debug ("newstyle negotiation: %s: client requested export '%s'",
> >               name_of_nbd_opt (option), data);
> > +      free (conn->exportname);
> > +      conn->exportname = malloc (optlen+1);
> > +      if (conn->exportname == NULL) {
> > +        nbdkit_error ("malloc: %m");
> > +        return -1;
> > +      }
> > +      memcpy (conn->exportname, data, optlen+1);
> 
> Why malloc/memcpy instead of strdup() (two instances)?

When I wrote that I believed that [because NBD is using Pascal-style
len + data] we could handle arbitrary 8 bit sequences, eg. UTF-16LE
would be valid.  However I've just checked the NBD protocol document
and I see that we specify strings must be UTF-8, so strcpy would be
sensible here.

> Otherwise the idea looks reasonable. I replied in the other thread about
> how we could expose this to the sh plugin's open, even if we don't bump
> to v3 API yet.

I'm trying to do the minimum to solve Xiubo's problem (AIUI).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html