Libguestfs - Dec 2018 - [nbdkit PATCH 6/5] nbd: More debug details

With some crufty sed scripts we can generate functions that map from
NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ").

This works on GNU sed and with FreeBSD, also with GNU sed's --posix
option, so I guess the sed code is POSIX-compatible.

Rich.

---
 src/Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Makefile.am b/src/Makefile.am
index 3490c0f..65f9498 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -80,6 +80,7 @@ nbdkit_LDFLAGS = \
 # synopsis.c is generated from docs/synopsis.txt where it is also
 # used to generate the man page.  It is included in main.c.
 
+BUILT_SOURCES = synopsis.c
 EXTRA_DIST = synopsis.c
 nbdkit_DEPENDENCIES = synopsis.c
 CLEANFILES += synopsis.c
-- 
2.19.0.rc0

These are separate fields in the request.
---
 src/protocol.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/protocol.h b/src/protocol.h
index 088dcab..0444641 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -154,6 +154,7 @@ struct reply {
 #define NBD_CMD_FLUSH             3
 #define NBD_CMD_TRIM              4
 #define NBD_CMD_WRITE_ZEROES      6
+
 #define NBD_CMD_FLAG_FUA      (1<<0)
 #define NBD_CMD_FLAG_NO_HOLE  (1<<1)
 
-- 
2.19.0.rc0

This generates small functions which map from various integer NBD
protocol flags to the string equivalent.

eg:
    name_of_nbd_cmd (NBD_CMD_READ)
      ---> "NBD_CMD_READ"

This commit uses some hairy sed scripting to ensure that we don't add
any more dependencies to nbdkit.
---
 src/protocol.h       |  9 +++++++
 .gitignore           |  1 +
 src/Makefile.am      | 17 +++++++++++--
 src/protostrings.sed | 59 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/src/protocol.h b/src/protocol.h
index 0444641..6709ddc 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -81,10 +81,12 @@ struct fixed_new_option_reply {
 #define NBD_REP_MAGIC UINT64_C(0x3e889045565a9)
 
 /* Global flags. */
+extern const char *name_of_nbd_global_flag (int);
 #define NBD_FLAG_FIXED_NEWSTYLE 1
 #define NBD_FLAG_NO_ZEROES      2
 
 /* Per-export flags. */
+extern const char *name_of_nbd_flag (int);
 #define NBD_FLAG_HAS_FLAGS         (1 << 0)
 #define NBD_FLAG_READ_ONLY         (1 << 1)
 #define NBD_FLAG_SEND_FLUSH        (1 << 2)
@@ -94,6 +96,7 @@ struct fixed_new_option_reply {
 #define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6)
 
 /* NBD options (new style handshake only). */
+extern const char *name_of_nbd_opt (int);
 #define NBD_OPT_EXPORT_NAME  1
 #define NBD_OPT_ABORT        2
 #define NBD_OPT_LIST         3
@@ -101,6 +104,7 @@ struct fixed_new_option_reply {
 #define NBD_OPT_INFO         6
 #define NBD_OPT_GO           7
 
+extern const char *name_of_nbd_rep (int);
 #define NBD_REP_ACK          1
 #define NBD_REP_SERVER       2
 #define NBD_REP_INFO         3
@@ -110,6 +114,7 @@ struct fixed_new_option_reply {
 #define NBD_REP_ERR_PLATFORM 0x80000004
 #define NBD_REP_ERR_TLS_REQD 0x80000005
 
+extern const char *name_of_nbd_info (int);
 #define NBD_INFO_EXPORT      0
 
 /* NBD_INFO_EXPORT reply (follows fixed_new_option_reply). */
@@ -148,6 +153,8 @@ struct reply {
 #define NBD_REQUEST_MAGIC 0x25609513
 #define NBD_REPLY_MAGIC 0x67446698
 
+/* NBD commands. */
+extern const char *name_of_nbd_cmd (int);
 #define NBD_CMD_READ              0
 #define NBD_CMD_WRITE             1
 #define NBD_CMD_DISC              2 /* Disconnect. */
@@ -155,12 +162,14 @@ struct reply {
 #define NBD_CMD_TRIM              4
 #define NBD_CMD_WRITE_ZEROES      6
 
+extern const char *name_of_nbd_cmd_flag (int);
 #define NBD_CMD_FLAG_FUA      (1<<0)
 #define NBD_CMD_FLAG_NO_HOLE  (1<<1)
 
 /* Error codes (previously errno).
  * See
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ca4414804114fd0095b317785bc0b51862e62ebb
  */
+extern const char *name_of_nbd_error (int);
 #define NBD_SUCCESS     0
 #define NBD_EPERM       1
 #define NBD_EIO         5
diff --git a/.gitignore b/.gitignore
index 86ef6cb..8cfe734 100644
--- a/.gitignore
+++ b/.gitignore
@@ -47,6 +47,7 @@ Makefile.in
 /podwrapper.pl
 /src/nbdkit
 /src/nbdkit.pc
+/src/protostrings.c
 /src/synopsis.c
 /src/test-utils
 /stamp-h1
diff --git a/src/Makefile.am b/src/Makefile.am
index 65f9498..1563d74 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -49,6 +49,7 @@ nbdkit_SOURCES = \
 	options.h \
 	plugins.c \
 	protocol.h \
+	protostrings.c \
 	sockets.c \
 	threadlocal.c \
 	utils.c \
@@ -77,11 +78,23 @@ nbdkit_LDFLAGS = \
 	$(PTHREAD_LIBS) \
 	$(DL_LDFLAGS)
 
+# protostrings.c is generated from the protocol.h header file where it
+# is used to map NBD protocol flags to strings.
+
+BUILT_SOURCES = protostrings.c
+EXTRA_DIST = protostrings.c
+CLEANFILES += protostrings.c
+protostrings.c: protocol.h protostrings.sed Makefile
+	rm -f $@ $@-t
+	$(SED) -n -f protostrings.sed < $< > $@-t
+	mv $@-t $@
+	chmod 0444 $@
+
 # synopsis.c is generated from docs/synopsis.txt where it is also
 # used to generate the man page.  It is included in main.c.
 
-BUILT_SOURCES = synopsis.c
-EXTRA_DIST = synopsis.c
+BUILT_SOURCES += synopsis.c
+EXTRA_DIST += synopsis.c
 nbdkit_DEPENDENCIES = synopsis.c
 CLEANFILES += synopsis.c
 main.c: synopsis.c
diff --git a/src/protostrings.sed b/src/protostrings.sed
new file mode 100644
index 0000000..9a94f75
--- /dev/null
+++ b/src/protostrings.sed
@@ -0,0 +1,59 @@
+# nbdkit
+# Copyright (C) 2018 Red Hat Inc.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Generate the protostrings.c file from protocol.h.
+
+# Prologue.
+1i\
+/* Generated from protocol.h by protostrings.sed. */\
+\#include "protocol.h"\
+
+
+# Match the precise sections of the source file.
+/^extern const char \*name_of_/,/^$/ {
+
+  # Convert extern function prototype into a definition.
+  s/extern \(const char \*name_of_.*\) (int);/\1 (int fl) {\
+  switch (fl) {/;
+
+  # Convert #define lines into cases.
+  s/^#define \([_A-Z]*\).*/  case \1: return "\1\";/;
+
+  # Append closing brace.
+  s/^$/  default: return "unknown";\
+  }\
+}/;
+
+  # Print pattern buffer.
+  p;
+
+}
-- 
2.19.0.rc0

---
 src/connections.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/connections.c b/src/connections.c
index 410a893..0f99b67 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -777,7 +777,7 @@ _negotiate_handshake_newstyle_options (struct connection
*conn)
 
     case NBD_OPT_INFO:
     case NBD_OPT_GO:
-      optname = option == NBD_OPT_INFO ? "NBD_OPT_INFO" :
"NBD_OPT_GO";
+      optname = name_of_nbd_opt (option);
       if (conn->recv (conn, data, optlen) == -1) {
         nbdkit_error ("read: %m");
         return -1;
-- 
2.19.0.rc0

---
 src/connections.c | 53 +++++++++++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/src/connections.c b/src/connections.c
index 0f99b67..58ed6b0 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -676,14 +676,14 @@ _negotiate_handshake_newstyle_options (struct connection
*conn)
     switch (option) {
     case NBD_OPT_EXPORT_NAME:
       if (conn->recv (conn, data, optlen) == -1) {
-        nbdkit_error ("read: %m");
+        nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
         return -1;
       }
       /* Apart from printing it, ignore the export name. */
       data[optlen] = '\0';
-      debug ("newstyle negotiation: NBD_OPT_EXPORT_NAME: "
+      debug ("newstyle negotiation: %s: "
              "client requested export '%s' (ignored)",
-             data);
+             name_of_nbd_opt (option), data);
 
       /* We have to finish the handshake by sending handshake_finish. */
       if (finish_newstyle_options (conn) == -1)
@@ -706,7 +706,8 @@ _negotiate_handshake_newstyle_options (struct connection
*conn)
     case NBD_OPT_ABORT:
       if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1)
         return -1;
-      debug ("client sent NBD_OPT_ABORT to abort the connection");
+      debug ("client sent %s to abort the connection",
+             name_of_nbd_opt (option));
       return -1;
 
     case NBD_OPT_LIST:
@@ -715,14 +716,15 @@ _negotiate_handshake_newstyle_options (struct connection
*conn)
             == -1)
           return -1;
         if (conn->recv (conn, data, optlen) == -1) {
-          nbdkit_error ("read: %m");
+          nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
           return -1;
         }
         continue;
       }
 
       /* Send back the exportname. */
-      debug ("newstyle negotiation: advertising export '%s'",
exportname);
+      debug ("newstyle negotiation: %s: advertising export
'%s'",
+             name_of_nbd_opt (option), exportname);
       if (send_newstyle_option_reply_exportname (conn, option, NBD_REP_SERVER,
                                                  exportname) == -1)
         return -1;
@@ -737,7 +739,7 @@ _negotiate_handshake_newstyle_options (struct connection
*conn)
             == -1)
           return -1;
         if (conn->recv (conn, data, optlen) == -1) {
-          nbdkit_error ("read: %m");
+          nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
           return -1;
         }
         continue;
@@ -779,7 +781,7 @@ _negotiate_handshake_newstyle_options (struct connection
*conn)
     case NBD_OPT_GO:
       optname = name_of_nbd_opt (option);
       if (conn->recv (conn, data, optlen) == -1) {
-        nbdkit_error ("read: %m");
+        nbdkit_error ("read: %s: %m", optname);
         return -1;
       }
 
@@ -859,7 +861,8 @@ _negotiate_handshake_newstyle_options (struct connection
*conn)
           case NBD_INFO_EXPORT: /* ignore - reply sent above */ break;
           default:
             debug ("newstyle negotiation: %s: "
-                   "ignoring NBD_INFO_* request %u", optname,
(unsigned) info);
+                   "ignoring NBD_INFO_* request %u (%s)",
+                   optname, (unsigned) info, name_of_nbd_info (info));
             break;
           }
         }
@@ -979,7 +982,8 @@ validate_request (struct connection *conn,
   if (conn->readonly &&
       (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM ||
        cmd == NBD_CMD_WRITE_ZEROES)) {
-    nbdkit_error ("invalid request: write request on readonly
connection");
+    nbdkit_error ("invalid request: %s: write request on readonly
connection",
+                  name_of_nbd_cmd (cmd));
     *error = EROFS;
     return false;
   }
@@ -992,9 +996,9 @@ validate_request (struct connection *conn,
   case NBD_CMD_WRITE_ZEROES:
     if (!valid_range (conn, offset, count)) {
       /* XXX Allow writes to extend the disk? */
-      nbdkit_error ("invalid request: offset and count are out of range:
"
+      nbdkit_error ("invalid request: %s: offset and count are out of
range: "
                     "offset=%" PRIu64 " count=%" PRIu32,
-                    offset, count);
+                    name_of_nbd_cmd (cmd), offset, count);
       *error = (cmd == NBD_CMD_WRITE ||
                 cmd == NBD_CMD_WRITE_ZEROES) ? ENOSPC : EINVAL;
       return false;
@@ -1003,7 +1007,8 @@ validate_request (struct connection *conn,
 
   case NBD_CMD_FLUSH:
     if (offset != 0 || count != 0) {
-      nbdkit_error ("invalid flush request: expecting offset and count =
0");
+      nbdkit_error ("invalid request: %s: expecting offset and count =
0",
+                    name_of_nbd_cmd (cmd));
       *error = EINVAL;
       return false;
     }
@@ -1037,29 +1042,33 @@ validate_request (struct connection *conn,
   /* Refuse over-large read and write requests. */
   if ((cmd == NBD_CMD_WRITE || cmd == NBD_CMD_READ) &&
       count > MAX_REQUEST_SIZE) {
-    nbdkit_error ("invalid request: data request is too large (%"
PRIu32
-                  " > %d)", count, MAX_REQUEST_SIZE);
+    nbdkit_error ("invalid request: %s: data request is too large (%"
PRIu32
+                  " > %d)",
+                  name_of_nbd_cmd (cmd), count, MAX_REQUEST_SIZE);
     *error = ENOMEM;
     return false;
   }
 
   /* Flush allowed? */
   if (!conn->can_flush && cmd == NBD_CMD_FLUSH) {
-    nbdkit_error ("invalid request: flush operation not supported");
+    nbdkit_error ("invalid request: %s: flush operation not
supported",
+                  name_of_nbd_cmd (cmd));
     *error = EINVAL;
     return false;
   }
 
   /* Trim allowed? */
   if (!conn->can_trim && cmd == NBD_CMD_TRIM) {
-    nbdkit_error ("invalid request: trim operation not supported");
+    nbdkit_error ("invalid request: %s: trim operation not
supported",
+                  name_of_nbd_cmd (cmd));
     *error = EINVAL;
     return false;
   }
 
   /* Zero allowed? */
   if (!conn->can_zero && cmd == NBD_CMD_WRITE_ZEROES) {
-    nbdkit_error ("invalid request: write zeroes operation not
supported");
+    nbdkit_error ("invalid request: %s: write zeroes operation not
supported",
+                  name_of_nbd_cmd (cmd));
     *error = EINVAL;
     return false;
   }
@@ -1229,7 +1238,7 @@ recv_request_send_reply (struct connection *conn)
     count = be32toh (request.count);
 
     if (cmd == NBD_CMD_DISC) {
-      debug ("client sent disconnect command, closing connection");
+      debug ("client sent %s, closing connection", name_of_nbd_cmd
(cmd));
       return set_status (conn, 0);                   /* disconnect */
     }
 
@@ -1262,7 +1271,7 @@ recv_request_send_reply (struct connection *conn)
         r = -1;
       }
       if (r == -1) {
-        nbdkit_error ("read data: %m");
+        nbdkit_error ("read data: %s: %m", name_of_nbd_cmd (cmd));
         return set_status (conn, -1);
       }
     }
@@ -1300,7 +1309,7 @@ recv_request_send_reply (struct connection *conn)
 
     r = conn->send (conn, &reply, sizeof reply);
     if (r == -1) {
-      nbdkit_error ("write reply: %m");
+      nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd));
       return set_status (conn, -1);
     }
 
@@ -1308,7 +1317,7 @@ recv_request_send_reply (struct connection *conn)
     if (cmd == NBD_CMD_READ && !error) {
       r = conn->send (conn, buf, count);
       if (r == -1) {
-        nbdkit_error ("write data: %m");
+        nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd));
         return set_status (conn, -1);
       }
     }
-- 
2.19.0.rc0

On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
> ---
>   src/Makefile.am | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 3490c0f..65f9498 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -80,6 +80,7 @@ nbdkit_LDFLAGS = \
>   # synopsis.c is generated from docs/synopsis.txt where it is also
>   # used to generate the man page.  It is included in main.c.
>   
> +BUILT_SOURCES = synopsis.c
>   EXTRA_DIST = synopsis.c
>   nbdkit_DEPENDENCIES = synopsis.c
>   CLEANFILES += synopsis.c
Did this make an actual difference during parallel 'make', given that 
you already have nbdkit_DEPENDENCIES to trigger normal make 
prerequisites? Automake's implementation of BUILT_SOURCES states that 
they will be rebuilt even when Makefile itself is out of date (because 
configure.ac or Makefile.am was touched, for example), which is really 
early in the process (and annoying when trying to use tab completion to 
learn what targets make supports, since BUILT_SOURCES are built prior to 
computing what output to present).  But while it might be annoying, I'm 
not opposed to the patch, especially if you did hit a situation in this 
series where it mattered.

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

On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
> This generates small functions which map from various integer NBD
> protocol flags to the string equivalent.
> 
> eg:
>      name_of_nbd_cmd (NBD_CMD_READ)
>        ---> "NBD_CMD_READ"
> 
> This commit uses some hairy sed scripting to ensure that we don't add
> any more dependencies to nbdkit.
Hairy, but well-commented. It forces some rather strict formatting to 
keep things working, but we touch the file seldom enough that I don't 
think that's a problem.

Series looks good to me; I'll probably post a followup to 
plugins/nbd/nbd.c to also take advantage of the  new functions.

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

Use new 'name_of_*' functions to offer a bit more details about
messages being forwarded on to the remote server.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 plugins/nbd/nbd.c       | 8 +++++---
 plugins/nbd/Makefile.am | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index 672f35b..6bd8861 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -267,8 +267,9 @@ nbd_request_raw (struct handle *h, uint16_t flags, uint16_t
type,
   int r;

   pthread_mutex_lock (&h->write_lock);
-  nbdkit_debug ("sending request with type %d and cookie %#" PRIx64,
type,
-                cookie);
+  nbdkit_debug ("sending request type %d (%s), flags %#x, offset %#"
PRIx64
+                ", count %#x, cookie %#" PRIx64, type,
name_of_nbd_cmd(type),
+                flags, offset, count, cookie);
   r = write_full (h->fd, &req, sizeof req);
   if (buf && !r)
     r = write_full (h->fd, buf, count);
@@ -353,7 +354,8 @@ nbd_reply_raw (struct handle *h, int *fd)
     return nbd_mark_dead (h);
   if (be32toh (rep.magic) != NBD_REPLY_MAGIC)
     return nbd_mark_dead (h);
-  nbdkit_debug ("received reply for cookie %#" PRIx64, rep.handle);
+  nbdkit_debug ("received reply for cookie %#" PRIx64 ", status
%s",
+                rep.handle, name_of_nbd_error(be32toh (rep.error)));
   trans = find_trans_by_cookie (h, rep.handle);
   if (!trans) {
     nbdkit_error ("reply with unexpected cookie %#" PRIx64,
rep.handle);
diff --git a/plugins/nbd/Makefile.am b/plugins/nbd/Makefile.am
index e998a28..9f08057 100644
--- a/plugins/nbd/Makefile.am
+++ b/plugins/nbd/Makefile.am
@@ -38,7 +38,8 @@ plugin_LTLIBRARIES = nbdkit-nbd-plugin.la

 nbdkit_nbd_plugin_la_SOURCES = \
 	nbd.c \
-	$(top_srcdir)/include/nbdkit-plugin.h
+	$(top_srcdir)/include/nbdkit-plugin.h \
+	$(top_srcdir)/src/protostrings.c

 nbdkit_nbd_plugin_la_CPPFLAGS = \
 	-I$(top_srcdir)/include \
-- 
2.17.2

On Fri, Dec 07, 2018 at 10:00:42AM -0600, Eric Blake
wrote:
> Use new 'name_of_*' functions to offer a bit more details about
> messages being forwarded on to the remote server.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  plugins/nbd/nbd.c       | 8 +++++---
>  plugins/nbd/Makefile.am | 3 ++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
> index 672f35b..6bd8861 100644
> --- a/plugins/nbd/nbd.c
> +++ b/plugins/nbd/nbd.c
> @@ -267,8 +267,9 @@ nbd_request_raw (struct handle *h, uint16_t flags,
uint16_t type,
>    int r;
> 
>    pthread_mutex_lock (&h->write_lock);
> -  nbdkit_debug ("sending request with type %d and cookie %#"
PRIx64, type,
> -                cookie);
> +  nbdkit_debug ("sending request type %d (%s), flags %#x, offset
%#" PRIx64
> +                ", count %#x, cookie %#" PRIx64, type,
name_of_nbd_cmd(type),
> +                flags, offset, count, cookie);
>    r = write_full (h->fd, &req, sizeof req);
>    if (buf && !r)
>      r = write_full (h->fd, buf, count);
> @@ -353,7 +354,8 @@ nbd_reply_raw (struct handle *h, int *fd)
>      return nbd_mark_dead (h);
>    if (be32toh (rep.magic) != NBD_REPLY_MAGIC)
>      return nbd_mark_dead (h);
> -  nbdkit_debug ("received reply for cookie %#" PRIx64,
rep.handle);
> +  nbdkit_debug ("received reply for cookie %#" PRIx64 ",
status %s",
> +                rep.handle, name_of_nbd_error(be32toh (rep.error)));
>    trans = find_trans_by_cookie (h, rep.handle);
>    if (!trans) {
>      nbdkit_error ("reply with unexpected cookie %#" PRIx64,
rep.handle);
> diff --git a/plugins/nbd/Makefile.am b/plugins/nbd/Makefile.am
> index e998a28..9f08057 100644
> --- a/plugins/nbd/Makefile.am
> +++ b/plugins/nbd/Makefile.am
> @@ -38,7 +38,8 @@ plugin_LTLIBRARIES = nbdkit-nbd-plugin.la
> 
>  nbdkit_nbd_plugin_la_SOURCES = \
>  	nbd.c \
> -	$(top_srcdir)/include/nbdkit-plugin.h
> +	$(top_srcdir)/include/nbdkit-plugin.h \
> +	$(top_srcdir)/src/protostrings.c
> 
>  nbdkit_nbd_plugin_la_CPPFLAGS = \
>  	-I$(top_srcdir)/include \
Patch 6/5 had me confused for a while there :-)

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

On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
> This generates small functions which map from various integer NBD
> protocol flags to the string equivalent.
> 
> eg:
>      name_of_nbd_cmd (NBD_CMD_READ)
>        ---> "NBD_CMD_READ"
> 
> This commit uses some hairy sed scripting to ensure that we don't add
> any more dependencies to nbdkit.
> ---
> +++ b/src/Makefile.am
> @@ -49,6 +49,7 @@ nbdkit_SOURCES = \
>   	options.h \
>   	plugins.c \
>   	protocol.h \
> +	protostrings.c \
>   	sockets.c \
>   	threadlocal.c \
>   	utils.c \
> @@ -77,11 +78,23 @@ nbdkit_LDFLAGS = \
>   	$(PTHREAD_LIBS) \
>   	$(DL_LDFLAGS)
>   
> +# protostrings.c is generated from the protocol.h header file where it
> +# is used to map NBD protocol flags to strings.
> +
> +BUILT_SOURCES = protostrings.c
> +EXTRA_DIST = protostrings.c
> +CLEANFILES += protostrings.c
> +protostrings.c: protocol.h protostrings.sed Makefile
> +	rm -f $@ $@-t
> +	$(SED) -n -f protostrings.sed < $< > $@-t
'make distcheck' caught a couple issues with this (we want 
protostrings.sed in the tarball, and sed can't find the script in VPATH 
builds). Now fixed.

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