thr3ads.net - Libguestfs - [Libguestfs] [nbdkit PATCH] connections: Implement NBD_OPT

If this information is useful, please help other people find it:
Share via:

Eric Blake

2018-Nov-29 13:21 UTC

[Libguestfs] [nbdkit PATCH] connections: Implement 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 point that
all we really have to do is tweak our debug messages :)

Signed-off-by: Eric Blake <eblake@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;
           }
         }
-- 
2.17.2

Richard W.M. Jones

2018-Nov-29 14:01 UTC

head link

Re: [Libguestfs] [nbdkit PATCH] connections: Implement NBD_OPT_INFO

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@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

Richard W.M. Jones

2018-Nov-29 14:02 UTC

head link

Re: [Libguestfs] [nbdkit PATCH] connections: Implement NBD_OPT_INFO

Oh, could you also update docs/nbdkit-protocol.pod too.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Possibly Parallel Threads

Search for more possibly parallel threads

Libguestfs - Nov 2018 - [nbdkit PATCH] connections: Implement NBD_OPT_INFO

[Libguestfs] [nbdkit PATCH] connections: Implement NBD_OPT_INFO

Re: [Libguestfs] [nbdkit PATCH] connections: Implement NBD_OPT_INFO

Re: [Libguestfs] [nbdkit PATCH] connections: Implement NBD_OPT_INFO

Possibly Parallel Threads