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