[Libguestfs] [nbdkit PATCH] connections: Implement NBD_OPT_INFO

[Richard W.M. Jones]

On Thu, Nov 29, 2018 at 07:21:01AM -0600, Eric Blake wrote:
> 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 point that
> all we really have to do is tweak our debug messages :)
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> This turned out to be so trivial that I'm pushing it right away.
> 
>  docs/nbdkit-protocol.pod |  7 +++++++
>  src/protocol.h           |  1 +
>  src/connections.c        | 19 +++++++++++--------
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
> index e3e06a6..ddabb53 100644
> --- a/docs/nbdkit-protocol.pod
> +++ b/docs/nbdkit-protocol.pod
> @@ -109,6 +109,13 @@ Supported in nbdkit E<ge> 1.5.3.
>  This protocol enhancement allows the server to return errors when
>  negotiating the export name.
> 
> +=item C<NBD_OPT_INFO>
> +
> +Supported in nbdkit E<ge> 1.9.3.
> +
> +This protocol enhancement allows a client to inspect details about
> +the export without actually connecting.
> +
>  =item Structured Replies
> 
>  I<Not supported>.
> diff --git a/src/protocol.h b/src/protocol.h
> index 792a905..088dcab 100644
> --- a/src/protocol.h
> +++ b/src/protocol.h
> @@ -98,6 +98,7 @@ struct fixed_new_option_reply {
>  #define NBD_OPT_ABORT        2
>  #define NBD_OPT_LIST         3
>  #define NBD_OPT_STARTTLS     5
> +#define NBD_OPT_INFO         6
>  #define NBD_OPT_GO           7
> 
>  #define NBD_REP_ACK          1
> diff --git a/src/connections.c b/src/connections.c
> index 1b40e46..410a893 100644
> --- a/src/connections.c
> +++ b/src/connections.c
> @@ -636,6 +636,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
>    uint32_t optlen;
>    char data[MAX_OPTION_LENGTH+1];
>    struct new_handshake_finish handshake_finish;
> +  const char *optname;
> 
>    for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) {
>      if (conn->recv (conn, &new_option, sizeof new_option) == -1) {
> @@ -774,14 +775,16 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
>        }
>        break;
> 
> +    case NBD_OPT_INFO:
>      case NBD_OPT_GO:
> +      optname = option == NBD_OPT_INFO ? "NBD_OPT_INFO" : "NBD_OPT_GO";
>        if (conn->recv (conn, data, optlen) == -1) {
>          nbdkit_error ("read: %m");
>          return -1;
>        }
> 
>        if (optlen < 6) { /* 32 bit export length + 16 bit nr info */
> -        debug ("newstyle negotiation: NBD_OPT_GO option length < 6");
> +        debug ("newstyle negotiation: %s option length < 6", optname);
> 
>          if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID)
>              == -1)
> @@ -800,7 +803,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
>          memcpy (&exportnamelen, &data[0], 4);
>          exportnamelen = be32toh (exportnamelen);
>          if (exportnamelen > optlen-6 /* NB optlen >= 6, see above */) {
> -          debug ("newstyle negotiation: NBD_OPT_GO: export name too long");
> +          debug ("newstyle negotiation: %s: export name too long", optname);
>            if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID)
>                == -1)
>              return -1;
> @@ -809,8 +812,8 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
>          memcpy (&nrinfos, &data[exportnamelen+4], 2);
>          nrinfos = be16toh (nrinfos);
>          if (optlen != 4 + exportnamelen + 2 + 2*nrinfos) {
> -          debug ("newstyle negotiation: NBD_OPT_GO: "
> -                 "number of information requests incorrect");
> +          debug ("newstyle negotiation: %s: "
> +                 "number of information requests incorrect", optname);
>            if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID)
>                == -1)
>              return -1;
> @@ -827,9 +830,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
>          }
>          memcpy (requested_exportname, &data[4], exportnamelen);
>          requested_exportname[exportnamelen] = '\0';
> -        debug ("newstyle negotiation: NBD_OPT_GO: "
> +        debug ("newstyle negotiation: %s: "
>                 "client requested export '%s' (ignored)",
> -               requested_exportname);
> +               optname, requested_exportname);
> 
>          /* The spec is confusing, but it is required that we send back
>           * NBD_INFO_EXPORT, even if the client did not request it!
> @@ -855,8 +858,8 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
>            switch (info) {
>            case NBD_INFO_EXPORT: /* ignore - reply sent above */ break;
>            default:
> -            debug ("newstyle negotiation: NBD_OPT_GO: "
> -                   "ignoring NBD_INFO_* request %u", (unsigned) info);
> +            debug ("newstyle negotiation: %s: "
> +                   "ignoring NBD_INFO_* request %u", optname, (unsigned) info);
>              break;
>            }
>          }

Wow, surprisingly simple!

ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW