thr3ads.net - Libguestfs - [Libguestfs] [PATCH nbdkit 3/4] common/protocol: Update nbd-protocol.h so it matches libnbd’s copy. [Sep 2019]

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

Richard W.M. Jones

2019-Sep-24 21:07 UTC

[Libguestfs] [PATCH nbdkit 0/4] common/protocol: Unify public <nbd-protocol.h>

We should have only one NBD protocol file.  Let's make nbdkit's
version the canonical one, and use it in libnbd.

Rich.

Richard W.M. Jones

2019-Sep-24 21:07 UTC

head link

[Libguestfs] [PATCH nbdkit 1/4] common/protocol: Rename protocol.h to nbd-protocol.h.

In preparation for installing this header as a common public header
for use by other projects, rename it.
---
 common/protocol/Makefile.am                    | 10 +++++-----
 common/protocol/{protocol.h => nbd-protocol.h} |  6 +++---
 common/protocol/protostrings.sed               |  6 +++---
 plugins/nbd/nbd-standalone.c                   |  2 +-
 server/protocol-handshake-newstyle.c           |  2 +-
 server/protocol-handshake-oldstyle.c           |  2 +-
 server/protocol-handshake.c                    |  2 +-
 server/protocol.c                              |  2 +-
 tests/test-layers.c                            |  2 +-
 9 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/common/protocol/Makefile.am b/common/protocol/Makefile.am
index 99df43b..996be26 100644
--- a/common/protocol/Makefile.am
+++ b/common/protocol/Makefile.am
@@ -32,7 +32,7 @@
 include $(top_srcdir)/common-rules.mk
 
 EXTRA_DIST = \
-	protocol.h \
+	nbd-protocol.h \
 	protostrings.c \
 	protostrings.sed \
 	$(NULL)
@@ -41,16 +41,16 @@ noinst_LTLIBRARIES = libprotocol.la
 
 libprotocol_la_SOURCES = \
         protostrings.c \
-	protocol.h \
+	nbd-protocol.h \
 	$(NULL)
 libprotocol_la_CFLAGS = $(WARNINGS_CFLAGS)
 
-# protostrings.c is generated from the protocol.h header file where it
-# is used to map NBD protocol flags to strings.
+# protostrings.c is generated from the nbd-protocol.h header file
+# where it is used to map NBD protocol flags to strings.
 
 BUILT_SOURCES = protostrings.c
 CLEANFILES += protostrings.c
-protostrings.c: protocol.h protostrings.sed Makefile
+protostrings.c: nbd-protocol.h protostrings.sed Makefile
 	rm -f $@ $@-t
 	$(SED) -n -f $(srcdir)/protostrings.sed < $< > $@-t
 	mv $@-t $@
diff --git a/common/protocol/protocol.h b/common/protocol/nbd-protocol.h
similarity index 99%
rename from common/protocol/protocol.h
rename to common/protocol/nbd-protocol.h
index bf54839..da2e0d0 100644
--- a/common/protocol/protocol.h
+++ b/common/protocol/nbd-protocol.h
@@ -30,8 +30,8 @@
  * SUCH DAMAGE.
  */
 
-#ifndef NBDKIT_PROTOCOL_H
-#define NBDKIT_PROTOCOL_H
+#ifndef NBD_PROTOCOL_H
+#define NBD_PROTOCOL_H
 
 #include <stdint.h>
 
@@ -244,4 +244,4 @@ extern const char *name_of_nbd_error (int);
 #define NBD_ENOTSUP    95
 #define NBD_ESHUTDOWN 108
 
-#endif /* NBDKIT_PROTOCOL_H */
+#endif /* NBD_PROTOCOL_H */
diff --git a/common/protocol/protostrings.sed b/common/protocol/protostrings.sed
index 1731c6b..cb1a76e 100644
--- a/common/protocol/protostrings.sed
+++ b/common/protocol/protostrings.sed
@@ -29,12 +29,12 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
-# Generate the protostrings.c file from protocol.h.
+# Generate the protostrings.c file from nbd-protocol.h.
 
 # Prologue.
 1i\
-/* Generated from protocol.h by protostrings.sed. */\
-\#include "protocol.h"\
+/* Generated from nbd-protocol.h by protostrings.sed. */\
+\#include "nbd-protocol.h"\
 
 
 # Match the precise sections of the source file.
diff --git a/plugins/nbd/nbd-standalone.c b/plugins/nbd/nbd-standalone.c
index 0fabbfe..a6779a4 100644
--- a/plugins/nbd/nbd-standalone.c
+++ b/plugins/nbd/nbd-standalone.c
@@ -53,7 +53,7 @@
 #define NBDKIT_API_VERSION 2
 
 #include <nbdkit-plugin.h>
-#include "protocol.h"
+#include "nbd-protocol.h"
 #include "byte-swapping.h"
 #include "cleanup.h"
 
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 38978c6..866360b 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -41,7 +41,7 @@
 
 #include "internal.h"
 #include "byte-swapping.h"
-#include "protocol.h"
+#include "nbd-protocol.h"
 
 /* Maximum number of client options we allow before giving up. */
 #define MAX_NR_OPTIONS 32
diff --git a/server/protocol-handshake-oldstyle.c
b/server/protocol-handshake-oldstyle.c
index f5425d3..87cb265 100644
--- a/server/protocol-handshake-oldstyle.c
+++ b/server/protocol-handshake-oldstyle.c
@@ -41,7 +41,7 @@
 
 #include "internal.h"
 #include "byte-swapping.h"
-#include "protocol.h"
+#include "nbd-protocol.h"
 
 int
 protocol_handshake_oldstyle (struct connection *conn)
diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
index a0d7dc2..2c2f35e 100644
--- a/server/protocol-handshake.c
+++ b/server/protocol-handshake.c
@@ -41,7 +41,7 @@
 
 #include "internal.h"
 #include "byte-swapping.h"
-#include "protocol.h"
+#include "nbd-protocol.h"
 
 int
 protocol_handshake (struct connection *conn)
diff --git a/server/protocol.c b/server/protocol.c
index e65a078..498db1d 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -45,7 +45,7 @@
 #include "internal.h"
 #include "byte-swapping.h"
 #include "minmax.h"
-#include "protocol.h"
+#include "nbd-protocol.h"
 
 static bool
 validate_request (struct connection *conn,
diff --git a/tests/test-layers.c b/tests/test-layers.c
index 9b595c4..1097f15 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -61,7 +61,7 @@
 #include "byte-swapping.h"
 #include "cleanup.h"
 #include "exit-with-parent.h"
-#include "protocol.h"
+#include "nbd-protocol.h"
 
 /* Declare program_name. */
 #if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME == 1
-- 
2.23.0

Richard W.M. Jones

2019-Sep-24 21:07 UTC

head link

[Libguestfs] [PATCH nbdkit 2/4] common/protocol: Remove protostrings.sed, use bash+sed instead.

Use a simple bash script to generate the protostrings.c functions.

Remove the extern decls from the nbd-protocol.h file which were used
previously in the generation of this file.  They have been moved to a
new internal header called "protostrings.h".
---
 common/protocol/Makefile.am                   |  8 ++-
 ...tostrings.sed => generate-protostrings.sh} | 56 +++++++++++--------
 common/protocol/nbd-protocol.h                | 10 ----
 common/protocol/protostrings.h                | 51 +++++++++++++++++
 plugins/nbd/nbd-standalone.c                  |  1 +
 server/protocol-handshake-newstyle.c          |  1 +
 server/protocol.c                             |  1 +
 7 files changed, 91 insertions(+), 37 deletions(-)

diff --git a/common/protocol/Makefile.am b/common/protocol/Makefile.am
index 996be26..74c288a 100644
--- a/common/protocol/Makefile.am
+++ b/common/protocol/Makefile.am
@@ -34,13 +34,14 @@ include $(top_srcdir)/common-rules.mk
 EXTRA_DIST = \
 	nbd-protocol.h \
 	protostrings.c \
-	protostrings.sed \
+	generate-protostrings.sh \
 	$(NULL)
 
 noinst_LTLIBRARIES = libprotocol.la
 
 libprotocol_la_SOURCES = \
         protostrings.c \
+	protostrings.h \
 	nbd-protocol.h \
 	$(NULL)
 libprotocol_la_CFLAGS = $(WARNINGS_CFLAGS)
@@ -50,8 +51,9 @@ libprotocol_la_CFLAGS = $(WARNINGS_CFLAGS)
 
 BUILT_SOURCES = protostrings.c
 CLEANFILES += protostrings.c
-protostrings.c: nbd-protocol.h protostrings.sed Makefile
+
+protostrings.c: nbd-protocol.h generate-protostrings.sh Makefile
 	rm -f $@ $@-t
-	$(SED) -n -f $(srcdir)/protostrings.sed < $< > $@-t
+	SED=$(SED) $(srcdir)/generate-protostrings.sh > $@-t
 	mv $@-t $@
 	chmod 0444 $@
diff --git a/common/protocol/protostrings.sed
b/common/protocol/generate-protostrings.sh
old mode 100644
new mode 100755
similarity index 61%
rename from common/protocol/protostrings.sed
rename to common/protocol/generate-protostrings.sh
index cb1a76e..d3f5d18
--- a/common/protocol/protostrings.sed
+++ b/common/protocol/generate-protostrings.sh
@@ -1,5 +1,6 @@
+#!/usr/bin/env bash
 # nbdkit
-# Copyright (C) 2018 Red Hat Inc.
+# Copyright (C) 2018-2019 Red Hat Inc.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
@@ -29,30 +30,37 @@
 # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 # SUCH DAMAGE.
 
-# Generate the protostrings.c file from nbd-protocol.h.
+# The header.
+cat <<'EOF'
+/* Generated from nbd-protocol.h by generate-protostrings.sh. */
 
-# Prologue.
-1i\
-/* Generated from nbd-protocol.h by protostrings.sed. */\
-\#include "nbd-protocol.h"\
+#include "nbd-protocol.h"
 
+EOF
 
-# Match the precise sections of the source file.
-/^extern const char \*name_of_/,/^$/ {
+declare -A functions=(
+    [global_flag]=NBD_FLAG_FIXED_NEWSTYLE
+    [flag]=NBD_FLAG_HAS_FLAGS
+    [opt]=NBD_OPT_EXPORT_NAME
+    [rep]=NBD_REP_ACK
+    [info]=NBD_INFO_EXPORT
+    [reply]=NBD_REPLY_FLAG_DONE
+    [reply_type]=NBD_REPLY_TYPE_NONE
+    [cmd]=NBD_CMD_READ
+    [cmd_flag]=NBD_CMD_FLAG_FUA
+    [error]=NBD_SUCCESS
+)
 
-  # 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;
-
-}
+# Generate each 'const char *name_of_nbd_<fn>'
+for fn in "${!functions[@]}"; do
+    echo 'extern const char *'
+    echo "name_of_nbd_$fn (int fl)"
+    echo '{'
+    echo '  switch (fl) {'
+    $SED -n "/^#define ${functions[$fn]}/,/^$/p" nbd-protocol.h |
+        $SED 's/^#define \([_A-Z]*\).*/  case \1:\n    return
"\1\";/'
+    echo '  default: return "unknown";'
+    echo '  }'
+    echo '}'
+    echo
+done
diff --git a/common/protocol/nbd-protocol.h b/common/protocol/nbd-protocol.h
index da2e0d0..60d35d0 100644
--- a/common/protocol/nbd-protocol.h
+++ b/common/protocol/nbd-protocol.h
@@ -80,12 +80,10 @@ 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)
@@ -99,7 +97,6 @@ extern const char *name_of_nbd_flag (int);
 #define NBD_FLAG_SEND_FAST_ZERO    (1 << 11)
 
 /* 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
@@ -113,7 +110,6 @@ extern const char *name_of_nbd_opt (int);
 #define NBD_REP_ERR(val) (0x80000000 | (val))
 #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000))
 
-extern const char *name_of_nbd_rep (int);
 #define NBD_REP_ACK          1
 #define NBD_REP_SERVER       2
 #define NBD_REP_INFO         3
@@ -124,7 +120,6 @@ extern const char *name_of_nbd_rep (int);
 #define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4)
 #define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5)
 
-extern const char *name_of_nbd_info (int);
 #define NBD_INFO_EXPORT      0
 
 /* NBD_INFO_EXPORT reply (follows fixed_new_option_reply). */
@@ -197,14 +192,12 @@ struct structured_reply_error {
 #define NBD_STRUCTURED_REPLY_MAGIC  0x668e33ef
 
 /* Structured reply flags. */
-extern const char *name_of_nbd_reply_flag (int);
 #define NBD_REPLY_FLAG_DONE         (1<<0)
 
 #define NBD_REPLY_TYPE_ERR(val) ((1<<15) | (val))
 #define NBD_REPLY_TYPE_IS_ERR(val) (!!((val) & (1<<15)))
 
 /* Structured reply types. */
-extern const char *name_of_nbd_reply_type (int);
 #define NBD_REPLY_TYPE_NONE         0
 #define NBD_REPLY_TYPE_OFFSET_DATA  1
 #define NBD_REPLY_TYPE_OFFSET_HOLE  2
@@ -213,7 +206,6 @@ extern const char *name_of_nbd_reply_type (int);
 #define NBD_REPLY_TYPE_ERROR_OFFSET NBD_REPLY_TYPE_ERR (2)
 
 /* 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. */
@@ -223,7 +215,6 @@ extern const char *name_of_nbd_cmd (int);
 #define NBD_CMD_WRITE_ZEROES      6
 #define NBD_CMD_BLOCK_STATUS      7
 
-extern const char *name_of_nbd_cmd_flag (int);
 #define NBD_CMD_FLAG_FUA       (1<<0)
 #define NBD_CMD_FLAG_NO_HOLE   (1<<1)
 #define NBD_CMD_FLAG_DF        (1<<2)
@@ -233,7 +224,6 @@ extern const char *name_of_nbd_cmd_flag (int);
 /* 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/common/protocol/protostrings.h b/common/protocol/protostrings.h
new file mode 100644
index 0000000..8029652
--- /dev/null
+++ b/common/protocol/protostrings.h
@@ -0,0 +1,51 @@
+/* nbdkit
+ * Copyright (C) 2018-2019 Red Hat Inc.
+ *
+ * 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.
+ */
+
+#ifndef NBDKIT_PROTOSTRINGS_H
+#define NBDKIT_PROTOSTRINGS_H
+
+/* The corresponding functions are generated by
+ * generate-protostrings.sh from the nbd-protocol.h header file.
+ */
+
+extern const char *name_of_nbd_global_flag (int);
+extern const char *name_of_nbd_flag (int);
+extern const char *name_of_nbd_opt (int);
+extern const char *name_of_nbd_rep (int);
+extern const char *name_of_nbd_info (int);
+extern const char *name_of_nbd_reply_flag (int);
+extern const char *name_of_nbd_reply_type (int);
+extern const char *name_of_nbd_cmd (int);
+extern const char *name_of_nbd_cmd_flag (int);
+extern const char *name_of_nbd_error (int);
+
+#endif /* NBDKIT_PROTOSTRINGS_H */
diff --git a/plugins/nbd/nbd-standalone.c b/plugins/nbd/nbd-standalone.c
index a6779a4..1789e39 100644
--- a/plugins/nbd/nbd-standalone.c
+++ b/plugins/nbd/nbd-standalone.c
@@ -54,6 +54,7 @@
 
 #include <nbdkit-plugin.h>
 #include "nbd-protocol.h"
+#include "protostrings.h"
 #include "byte-swapping.h"
 #include "cleanup.h"
 
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 866360b..104796a 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -42,6 +42,7 @@
 #include "internal.h"
 #include "byte-swapping.h"
 #include "nbd-protocol.h"
+#include "protostrings.h"
 
 /* Maximum number of client options we allow before giving up. */
 #define MAX_NR_OPTIONS 32
diff --git a/server/protocol.c b/server/protocol.c
index 498db1d..8df5ed5 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -46,6 +46,7 @@
 #include "byte-swapping.h"
 #include "minmax.h"
 #include "nbd-protocol.h"
+#include "protostrings.h"
 
 static bool
 validate_request (struct connection *conn,
-- 
2.23.0

Richard W.M. Jones

2019-Sep-24 21:07 UTC

head link

[Libguestfs] [PATCH nbdkit 3/4] common/protocol: Update nbd-protocol.h so it matches libnbd’s copy.

Diff against libnbd’s copy of this file, and change this one until it
matches.
---
 common/protocol/nbd-protocol.h       | 76 +++++++++++++++++-----------
 server/protocol-handshake-newstyle.c | 26 +++++-----
 server/protocol-handshake-oldstyle.c |  4 +-
 server/protocol.c                    | 25 ++++-----
 tests/test-layers.c                  | 14 ++---
 5 files changed, 81 insertions(+), 64 deletions(-)

diff --git a/common/protocol/nbd-protocol.h b/common/protocol/nbd-protocol.h
index 60d35d0..724ffb6 100644
--- a/common/protocol/nbd-protocol.h
+++ b/common/protocol/nbd-protocol.h
@@ -40,37 +40,46 @@
  * these structures.
  */
 
+#define NBD_MAX_STRING 4096 /* Maximum length of a string field */
+
 /* Old-style handshake. */
-struct old_handshake {
+struct nbd_old_handshake {
   char nbdmagic[8];           /* "NBDMAGIC" */
-  uint64_t version;           /* OLD_VERSION */
+  uint64_t version;           /* NBD_OLD_VERSION */
   uint64_t exportsize;
   uint16_t gflags;            /* global flags */
   uint16_t eflags;            /* per-export flags */
   char zeroes[124];           /* must be sent as zero bytes */
 } __attribute__((packed));
 
-#define OLD_VERSION UINT64_C(0x420281861253)
+#define NBD_OLD_VERSION UINT64_C(0x420281861253)
 
 /* New-style handshake. */
-struct new_handshake {
+struct nbd_new_handshake {
   char nbdmagic[8];           /* "NBDMAGIC" */
-  uint64_t version;           /* NEW_VERSION */
+  uint64_t version;           /* NBD_NEW_VERSION */
   uint16_t gflags;            /* global flags */
 } __attribute__((packed));
 
-#define NEW_VERSION UINT64_C(0x49484156454F5054)
+#define NBD_NEW_VERSION UINT64_C(0x49484156454F5054)
 
 /* New-style handshake option (sent by the client to us). */
-struct new_option {
+struct nbd_new_option {
   uint64_t version;           /* NEW_VERSION */
   uint32_t option;            /* NBD_OPT_* */
   uint32_t optlen;            /* option data length */
   /* option data follows */
 } __attribute__((packed));
 
+/* Newstyle handshake OPT_EXPORT_NAME reply message. */
+struct nbd_export_name_option_reply {
+  uint64_t exportsize;          /* size of export */
+  uint16_t eflags;              /* per-export flags */
+  char zeroes[124];             /* optional zeroes */
+} __attribute__((packed));;
+
 /* Fixed newstyle handshake reply message. */
-struct fixed_new_option_reply {
+struct nbd_fixed_new_option_reply {
   uint64_t magic;             /* NBD_REP_MAGIC */
   uint32_t option;            /* option we are replying to */
   uint32_t reply;             /* NBD_REP_* */
@@ -110,33 +119,37 @@ struct fixed_new_option_reply {
 #define NBD_REP_ERR(val) (0x80000000 | (val))
 #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000))
 
-#define NBD_REP_ACK          1
-#define NBD_REP_SERVER       2
-#define NBD_REP_INFO         3
-#define NBD_REP_META_CONTEXT 4
-#define NBD_REP_ERR_UNSUP    NBD_REP_ERR (1)
-#define NBD_REP_ERR_POLICY   NBD_REP_ERR (2)
-#define NBD_REP_ERR_INVALID  NBD_REP_ERR (3)
-#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4)
-#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5)
+#define NBD_REP_ACK                  1
+#define NBD_REP_SERVER               2
+#define NBD_REP_INFO                 3
+#define NBD_REP_META_CONTEXT         4
+#define NBD_REP_ERR_UNSUP            NBD_REP_ERR (1)
+#define NBD_REP_ERR_POLICY           NBD_REP_ERR (2)
+#define NBD_REP_ERR_INVALID          NBD_REP_ERR (3)
+#define NBD_REP_ERR_PLATFORM         NBD_REP_ERR (4)
+#define NBD_REP_ERR_TLS_REQD         NBD_REP_ERR (5)
+#define NBD_REP_ERR_UNKNOWN          NBD_REP_ERR (6)
+#define NBD_REP_ERR_SHUTDOWN         NBD_REP_ERR (7)
+#define NBD_REP_ERR_BLOCK_SIZE_REQD  NBD_REP_ERR (8)
+#define NBD_REP_ERR_TOO_BIG          NBD_REP_ERR (9)
 
 #define NBD_INFO_EXPORT      0
 
 /* NBD_INFO_EXPORT reply (follows fixed_new_option_reply). */
-struct fixed_new_option_reply_info_export {
+struct nbd_fixed_new_option_reply_info_export {
   uint16_t info;                /* NBD_INFO_EXPORT */
   uint64_t exportsize;          /* size of export */
   uint16_t eflags;              /* per-export flags */
 } __attribute__((packed));
 
 /* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */
-struct fixed_new_option_reply_meta_context {
+struct nbd_fixed_new_option_reply_meta_context {
   uint32_t context_id;          /* metadata context ID */
   /* followed by a string */
 } __attribute__((packed));
 
 /* NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */
-struct block_descriptor {
+struct nbd_block_descriptor {
   uint32_t length;              /* length of block */
   uint32_t status_flags;        /* block type (hole etc) */
 } __attribute__((packed));
@@ -144,14 +157,14 @@ struct block_descriptor {
 /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME.
  * Modern clients use NBD_OPT_GO instead of this.
  */
-struct new_handshake_finish {
+struct nbd_new_handshake_finish {
   uint64_t exportsize;
   uint16_t eflags;            /* per-export flags */
   char zeroes[124];           /* must be sent as zero bytes */
 } __attribute__((packed));
 
 /* Request (client -> server). */
-struct request {
+struct nbd_request {
   uint32_t magic;               /* NBD_REQUEST_MAGIC. */
   uint16_t flags;               /* Request flags. */
   uint16_t type;                /* Request type. */
@@ -161,14 +174,14 @@ struct request {
 } __attribute__((packed));
 
 /* Simple reply (server -> client). */
-struct simple_reply {
+struct nbd_simple_reply {
   uint32_t magic;               /* NBD_SIMPLE_REPLY_MAGIC. */
   uint32_t error;               /* NBD_SUCCESS or one of NBD_E*. */
   uint64_t handle;              /* Opaque handle. */
 } __attribute__((packed));
 
 /* Structured reply (server -> client). */
-struct structured_reply {
+struct nbd_structured_reply {
   uint32_t magic;               /* NBD_STRUCTURED_REPLY_MAGIC. */
   uint16_t flags;               /* NBD_REPLY_FLAG_* */
   uint16_t type;                /* NBD_REPLY_TYPE_* */
@@ -176,15 +189,20 @@ struct structured_reply {
   uint32_t length;              /* Length of payload which follows. */
 } __attribute__((packed));
 
-struct structured_reply_offset_data {
+struct nbd_structured_reply_offset_data {
   uint64_t offset;              /* offset */
   /* Followed by data. */
 } __attribute__((packed));
 
-struct structured_reply_error {
+struct nbd_structured_reply_offset_hole {
+  uint64_t offset;
+  uint32_t length;              /* Length of hole. */
+} __attribute__((packed));
+
+struct nbd_structured_reply_error {
   uint32_t error;               /* NBD_E* error number */
   uint16_t len;                 /* Length of human readable error. */
-  /* Followed by human readable error string. */
+  /* Followed by human readable error string, and possibly more structure. */
 } __attribute__((packed));
 
 #define NBD_REQUEST_MAGIC           0x25609513
@@ -221,9 +239,7 @@ struct structured_reply_error {
 #define NBD_CMD_FLAG_REQ_ONE   (1<<3)
 #define NBD_CMD_FLAG_FAST_ZERO (1<<4)
 
-/* Error codes (previously errno).
- * See
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ca4414804114fd0095b317785bc0b51862e62ebb
- */
+/* NBD error codes. */
 #define NBD_SUCCESS     0
 #define NBD_EPERM       1
 #define NBD_EIO         5
diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 104796a..413dcf0 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -55,7 +55,7 @@ static int
 send_newstyle_option_reply (struct connection *conn,
                             uint32_t option, uint32_t reply)
 {
-  struct fixed_new_option_reply fixed_new_option_reply;
+  struct nbd_fixed_new_option_reply fixed_new_option_reply;
 
   fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC);
   fixed_new_option_reply.option = htobe32 (option);
@@ -83,7 +83,7 @@ static int
 send_newstyle_option_reply_exportname (struct connection *conn,
                                        uint32_t option, uint32_t reply)
 {
-  struct fixed_new_option_reply fixed_new_option_reply;
+  struct nbd_fixed_new_option_reply fixed_new_option_reply;
   size_t name_len = strlen (exportname);
   uint32_t len;
 
@@ -119,8 +119,8 @@ send_newstyle_option_reply_info_export (struct connection
*conn,
                                         uint32_t option, uint32_t reply,
                                         uint16_t info, uint64_t exportsize)
 {
-  struct fixed_new_option_reply fixed_new_option_reply;
-  struct fixed_new_option_reply_info_export export;
+  struct nbd_fixed_new_option_reply fixed_new_option_reply;
+  struct nbd_fixed_new_option_reply_info_export export;
 
   fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC);
   fixed_new_option_reply.option = htobe32 (option);
@@ -147,8 +147,8 @@ send_newstyle_option_reply_meta_context (struct connection
*conn,
                                          uint32_t context_id,
                                          const char *name)
 {
-  struct fixed_new_option_reply fixed_new_option_reply;
-  struct fixed_new_option_reply_meta_context context;
+  struct nbd_fixed_new_option_reply fixed_new_option_reply;
+  struct nbd_fixed_new_option_reply_meta_context context;
   const size_t namelen = strlen (name);
 
   debug ("newstyle negotiation: %s: replying with %s id %d",
@@ -213,13 +213,13 @@ finish_newstyle_options (struct connection *conn, uint64_t
*exportsize)
 static int
 negotiate_handshake_newstyle_options (struct connection *conn)
 {
-  struct new_option new_option;
+  struct nbd_new_option new_option;
   size_t nr_options;
   uint64_t version;
   uint32_t option;
   uint32_t optlen;
   char data[MAX_OPTION_LENGTH+1];
-  struct new_handshake_finish handshake_finish;
+  struct nbd_new_handshake_finish handshake_finish;
   const char *optname;
   uint64_t exportsize;
 
@@ -229,10 +229,10 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
       return -1;
 
     version = be64toh (new_option.version);
-    if (version != NEW_VERSION) {
+    if (version != NBD_NEW_VERSION) {
       nbdkit_error ("unknown option version %" PRIx64
                     ", expecting %" PRIx64,
-                    version, NEW_VERSION);
+                    version, NBD_NEW_VERSION);
       return -1;
     }
 
@@ -296,7 +296,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
       if (conn->send (conn,
                       &handshake_finish,
                       (conn->cflags & NBD_FLAG_NO_ZEROES)
-                      ? offsetof (struct new_handshake_finish, zeroes)
+                      ? offsetof (struct nbd_new_handshake_finish, zeroes)
                       : sizeof handshake_finish, 0) == -1) {
         nbdkit_error ("write: %s: %m", optname);
         return -1;
@@ -670,7 +670,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
 int
 protocol_handshake_newstyle (struct connection *conn)
 {
-  struct new_handshake handshake;
+  struct nbd_new_handshake handshake;
   uint16_t gflags;
 
   gflags = (NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES) & mask_handshake;
@@ -678,7 +678,7 @@ protocol_handshake_newstyle (struct connection *conn)
   debug ("newstyle negotiation: flags: global 0x%x", gflags);
 
   memcpy (handshake.nbdmagic, "NBDMAGIC", 8);
-  handshake.version = htobe64 (NEW_VERSION);
+  handshake.version = htobe64 (NBD_NEW_VERSION);
   handshake.gflags = htobe16 (gflags);
 
   if (conn->send (conn, &handshake, sizeof handshake, 0) == -1) {
diff --git a/server/protocol-handshake-oldstyle.c
b/server/protocol-handshake-oldstyle.c
index 87cb265..8faa4f0 100644
--- a/server/protocol-handshake-oldstyle.c
+++ b/server/protocol-handshake-oldstyle.c
@@ -46,7 +46,7 @@
 int
 protocol_handshake_oldstyle (struct connection *conn)
 {
-  struct old_handshake handshake;
+  struct nbd_old_handshake handshake;
   uint64_t exportsize;
   uint16_t gflags, eflags;
 
@@ -61,7 +61,7 @@ protocol_handshake_oldstyle (struct connection *conn)
 
   memset (&handshake, 0, sizeof handshake);
   memcpy (handshake.nbdmagic, "NBDMAGIC", 8);
-  handshake.version = htobe64 (OLD_VERSION);
+  handshake.version = htobe64 (NBD_OLD_VERSION);
   handshake.exportsize = htobe64 (exportsize);
   handshake.gflags = htobe16 (gflags);
   handshake.eflags = htobe16 (eflags);
diff --git a/server/protocol.c b/server/protocol.c
index 8df5ed5..89fbdfa 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -367,7 +367,7 @@ send_simple_reply (struct connection *conn,
                    uint32_t error)
 {
   ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock);
-  struct simple_reply reply;
+  struct nbd_simple_reply reply;
   int r;
   int f = (cmd == NBD_CMD_READ && !error) ? SEND_MORE : 0;
 
@@ -404,8 +404,8 @@ send_structured_reply_read (struct connection *conn,
    * that yet we acquire the lock for the whole function.
    */
   ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock);
-  struct structured_reply reply;
-  struct structured_reply_offset_data offset_data;
+  struct nbd_structured_reply reply;
+  struct nbd_structured_reply_offset_data offset_data;
   int r;
 
   assert (cmd == NBD_CMD_READ);
@@ -442,7 +442,7 @@ send_structured_reply_read (struct connection *conn,
 /* Convert a list of extents into NBD_REPLY_TYPE_BLOCK_STATUS blocks.
  * The rules here are very complicated.  Read the spec carefully!
  */
-static struct block_descriptor *
+static struct nbd_block_descriptor *
 extents_to_block_descriptors (struct nbdkit_extents *extents,
                               uint16_t flags,
                               uint32_t count, uint64_t offset,
@@ -451,13 +451,14 @@ extents_to_block_descriptors (struct nbdkit_extents
*extents,
   const bool req_one = flags & NBD_CMD_FLAG_REQ_ONE;
   const size_t nr_extents = nbdkit_extents_count (extents);
   size_t i;
-  struct block_descriptor *blocks;
+  struct nbd_block_descriptor *blocks;
 
   /* This is checked in server/plugins.c. */
   assert (nr_extents >= 1);
 
   /* We may send fewer than nr_extents blocks, but never more. */
-  blocks = calloc (req_one ? 1 : nr_extents, sizeof (struct block_descriptor));
+  blocks = calloc (req_one ? 1 : nr_extents,
+                   sizeof (struct nbd_block_descriptor));
   if (blocks == NULL) {
     nbdkit_error ("calloc: %m");
     return NULL;
@@ -528,8 +529,8 @@ send_structured_reply_block_status (struct connection *conn,
                                     struct nbdkit_extents *extents)
 {
   ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock);
-  struct structured_reply reply;
-  CLEANUP_FREE struct block_descriptor *blocks = NULL;
+  struct nbd_structured_reply reply;
+  CLEANUP_FREE struct nbd_block_descriptor *blocks = NULL;
   size_t nr_blocks;
   uint32_t context_id;
   size_t i;
@@ -548,7 +549,7 @@ send_structured_reply_block_status (struct connection *conn,
   reply.flags = htobe16 (NBD_REPLY_FLAG_DONE);
   reply.type = htobe16 (NBD_REPLY_TYPE_BLOCK_STATUS);
   reply.length = htobe32 (sizeof context_id +
-                          nr_blocks * sizeof (struct block_descriptor));
+                          nr_blocks * sizeof (struct nbd_block_descriptor));
 
   r = conn->send (conn, &reply, sizeof reply, SEND_MORE);
   if (r == -1) {
@@ -583,8 +584,8 @@ send_structured_reply_error (struct connection *conn,
                              uint32_t error)
 {
   ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&conn->write_lock);
-  struct structured_reply reply;
-  struct structured_reply_error error_data;
+  struct nbd_structured_reply reply;
+  struct nbd_structured_reply_error error_data;
   int r;
 
   reply.magic = htobe32 (NBD_STRUCTURED_REPLY_MAGIC);
@@ -616,7 +617,7 @@ int
 protocol_recv_request_send_reply (struct connection *conn)
 {
   int r;
-  struct request request;
+  struct nbd_request request;
   uint16_t cmd, flags;
   uint32_t magic, count, error = 0;
   uint64_t offset;
diff --git a/tests/test-layers.c b/tests/test-layers.c
index 1097f15..e82cbea 100644
--- a/tests/test-layers.c
+++ b/tests/test-layers.c
@@ -86,13 +86,13 @@ main (int argc, char *argv[])
   int err;
   pthread_t thread;
   int sock;
-  struct new_handshake handshake;
+  struct nbd_new_handshake handshake;
   uint32_t cflags;
-  struct new_option option;
-  struct new_handshake_finish handshake_finish;
+  struct nbd_new_option option;
+  struct nbd_new_handshake_finish handshake_finish;
   uint16_t eflags;
-  struct request request;
-  struct simple_reply reply;
+  struct nbd_request request;
+  struct nbd_simple_reply reply;
   char data[512];
 
 #ifndef HAVE_EXIT_WITH_PARENT
@@ -175,7 +175,7 @@ main (int argc, char *argv[])
     exit (EXIT_FAILURE);
   }
   if (memcmp (handshake.nbdmagic, "NBDMAGIC", 8) != 0 ||
-      be64toh (handshake.version) != NEW_VERSION) {
+      be64toh (handshake.version) != NBD_NEW_VERSION) {
     fprintf (stderr, "%s: unexpected NBDMAGIC or version\n",
              program_name);
     exit (EXIT_FAILURE);
@@ -189,7 +189,7 @@ main (int argc, char *argv[])
   }
 
   /* Send NBD_OPT_EXPORT_NAME with no export name. */
-  option.version = htobe64 (NEW_VERSION);
+  option.version = htobe64 (NBD_NEW_VERSION);
   option.option = htobe32 (NBD_OPT_EXPORT_NAME);
   option.optlen = htobe32 (0);
   if (send (sock, &option, sizeof option, 0) != sizeof option) {
-- 
2.23.0

Richard W.M. Jones

2019-Sep-24 21:07 UTC

head link

[Libguestfs] [PATCH nbdkit 4/4] common/protocol: Install <nbd-protocol.h> as a public header.

Some further small changes are made to allow this header to be used
from arbitrary ISO C compilers.

It can now be used from other projects.
---
 common/protocol/Makefile.am    |  2 ++
 common/protocol/nbd-protocol.h | 38 ++++++++++++++++++++--------------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/common/protocol/Makefile.am b/common/protocol/Makefile.am
index 74c288a..b115c58 100644
--- a/common/protocol/Makefile.am
+++ b/common/protocol/Makefile.am
@@ -37,6 +37,8 @@ EXTRA_DIST = \
 	generate-protostrings.sh \
 	$(NULL)
 
+include_HEADERS = nbd-protocol.h
+
 noinst_LTLIBRARIES = libprotocol.la
 
 libprotocol_la_SOURCES = \
diff --git a/common/protocol/nbd-protocol.h b/common/protocol/nbd-protocol.h
index 724ffb6..7df411a 100644
--- a/common/protocol/nbd-protocol.h
+++ b/common/protocol/nbd-protocol.h
@@ -36,10 +36,16 @@
 #include <stdint.h>
 
 /* Note that all NBD fields are sent on the wire in network byte
- * order, so we must use beXXtoh or htobeXX when reading or writing
+ * order, so you must use beXXtoh or htobeXX when reading or writing
  * these structures.
  */
 
+#if defined(__GNUC__) || defined(__clang__)
+#define NBD_ATTRIBUTE_PACKED __attribute__((__packed__))
+#else
+#define NBD_ATTRIBUTE_PACKED
+#endif
+
 #define NBD_MAX_STRING 4096 /* Maximum length of a string field */
 
 /* Old-style handshake. */
@@ -50,7 +56,7 @@ struct nbd_old_handshake {
   uint16_t gflags;            /* global flags */
   uint16_t eflags;            /* per-export flags */
   char zeroes[124];           /* must be sent as zero bytes */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 #define NBD_OLD_VERSION UINT64_C(0x420281861253)
 
@@ -59,7 +65,7 @@ struct nbd_new_handshake {
   char nbdmagic[8];           /* "NBDMAGIC" */
   uint64_t version;           /* NBD_NEW_VERSION */
   uint16_t gflags;            /* global flags */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 #define NBD_NEW_VERSION UINT64_C(0x49484156454F5054)
 
@@ -69,14 +75,14 @@ struct nbd_new_option {
   uint32_t option;            /* NBD_OPT_* */
   uint32_t optlen;            /* option data length */
   /* option data follows */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 /* Newstyle handshake OPT_EXPORT_NAME reply message. */
 struct nbd_export_name_option_reply {
   uint64_t exportsize;          /* size of export */
   uint16_t eflags;              /* per-export flags */
   char zeroes[124];             /* optional zeroes */
-} __attribute__((packed));;
+} NBD_ATTRIBUTE_PACKED;;
 
 /* Fixed newstyle handshake reply message. */
 struct nbd_fixed_new_option_reply {
@@ -84,7 +90,7 @@ struct nbd_fixed_new_option_reply {
   uint32_t option;            /* option we are replying to */
   uint32_t reply;             /* NBD_REP_* */
   uint32_t replylen;
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 #define NBD_REP_MAGIC UINT64_C(0x3e889045565a9)
 
@@ -140,19 +146,19 @@ struct nbd_fixed_new_option_reply_info_export {
   uint16_t info;                /* NBD_INFO_EXPORT */
   uint64_t exportsize;          /* size of export */
   uint16_t eflags;              /* per-export flags */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 /* NBD_REP_META_CONTEXT reply (follows fixed_new_option_reply). */
 struct nbd_fixed_new_option_reply_meta_context {
   uint32_t context_id;          /* metadata context ID */
   /* followed by a string */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 /* NBD_REPLY_TYPE_BLOCK_STATUS block descriptor. */
 struct nbd_block_descriptor {
   uint32_t length;              /* length of block */
   uint32_t status_flags;        /* block type (hole etc) */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME.
  * Modern clients use NBD_OPT_GO instead of this.
@@ -161,7 +167,7 @@ struct nbd_new_handshake_finish {
   uint64_t exportsize;
   uint16_t eflags;            /* per-export flags */
   char zeroes[124];           /* must be sent as zero bytes */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 /* Request (client -> server). */
 struct nbd_request {
@@ -171,14 +177,14 @@ struct nbd_request {
   uint64_t handle;              /* Opaque handle. */
   uint64_t offset;              /* Request offset. */
   uint32_t count;               /* Request length. */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 /* Simple reply (server -> client). */
 struct nbd_simple_reply {
   uint32_t magic;               /* NBD_SIMPLE_REPLY_MAGIC. */
   uint32_t error;               /* NBD_SUCCESS or one of NBD_E*. */
   uint64_t handle;              /* Opaque handle. */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 /* Structured reply (server -> client). */
 struct nbd_structured_reply {
@@ -187,23 +193,23 @@ struct nbd_structured_reply {
   uint16_t type;                /* NBD_REPLY_TYPE_* */
   uint64_t handle;              /* Opaque handle. */
   uint32_t length;              /* Length of payload which follows. */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 struct nbd_structured_reply_offset_data {
   uint64_t offset;              /* offset */
   /* Followed by data. */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 struct nbd_structured_reply_offset_hole {
   uint64_t offset;
   uint32_t length;              /* Length of hole. */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 struct nbd_structured_reply_error {
   uint32_t error;               /* NBD_E* error number */
   uint16_t len;                 /* Length of human readable error. */
   /* Followed by human readable error string, and possibly more structure. */
-} __attribute__((packed));
+} NBD_ATTRIBUTE_PACKED;
 
 #define NBD_REQUEST_MAGIC           0x25609513
 #define NBD_SIMPLE_REPLY_MAGIC      0x67446698
-- 
2.23.0

Richard W.M. Jones

2019-Sep-24 21:23 UTC

head link

[Libguestfs] [PATCH nbdkit 0/4] common/protocol: Unify public <nbd-protocol.h>

The cover letter was rather brief, let me try to explain this change
some more ...

When we created libnbd, we needed the definitions of various NBD
protocol things (like protocol message layouts, constants etc).  We
copied a header file from nbdkit which provided these.  Over time this
header file has diverged from nbdkit's copy.

In fact the libnbd one has been generally enhanced over the nbdkit
version:

 - All symbols in libnbd's copy are prefixed by NBD_ or nbd_.

 - More coverage of the protocol.

It would be good, not just for nbdkit/libnbd but for other projects,
if we could unify these two files again and publish a liberally
licensed copy as /usr/include/nbd-protocol.h.

This commit series does this for nbdkit.

For libnbd there is an associated patch which just synchronizes the
two files.  I propose that we eventually change libnbd so that it uses
the public (ie. nbdkit) version, but could fall back to an internal
copy if the public version does not exist (so that libnbd doesn't need
to depend on nbdkit).

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

Eric Blake

2019-Sep-24 21:55 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 0/4] common/protocol: Unify public <nbd-protocol.h>

On 9/24/19 4:23 PM, Richard W.M. Jones wrote:> The cover letter was rather brief, let me try to explain this change
> some more ...
> 
> When we created libnbd, we needed the definitions of various NBD
> protocol things (like protocol message layouts, constants etc).  We
> copied a header file from nbdkit which provided these.  Over time this
> header file has diverged from nbdkit's copy.
> 
> In fact the libnbd one has been generally enhanced over the nbdkit
> version:
> 
>  - All symbols in libnbd's copy are prefixed by NBD_ or nbd_.
> 
>  - More coverage of the protocol.
> 
> It would be good, not just for nbdkit/libnbd but for other projects,
> if we could unify these two files again and publish a liberally
> licensed copy as /usr/include/nbd-protocol.h.
And nbdkit definitely has the more-permissive license, as the better
location to stick it.
> 
> This commit series does this for nbdkit.
> 
> For libnbd there is an associated patch which just synchronizes the
> two files.  I propose that we eventually change libnbd so that it uses
> the public (ie. nbdkit) version, but could fall back to an internal
> copy if the public version does not exist (so that libnbd doesn't need
> to depend on nbdkit).
It is a quasi-circular dependency: nbdkit depends on libnbd (if you
build libnbd-nbd-plugin), and libnbd depends on nbdkit (if you want the
latest header, rather than an in-tree fallback copied from an earlier
point in time); and both projects like to use the other in their
testsuites.  But I think we are still at a point where either project
can be bootstrapped first without the other being installed (test
coverage may be smaller, and nbdkit-nbd-plugin may be omitted, but that
doesn't stop the rest of either package from working).

So the idea sounds fine to me.

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

Eric Blake

2019-Sep-24 21:58 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 1/4] common/protocol: Rename protocol.h to nbd-protocol.h.

On 9/24/19 4:07 PM, Richard W.M. Jones wrote:> In preparation for installing this header as a common public header
> for use by other projects, rename it.
> ---
>  common/protocol/Makefile.am                    | 10 +++++-----
>  common/protocol/{protocol.h => nbd-protocol.h} |  6 +++---
>  common/protocol/protostrings.sed               |  6 +++---
>  plugins/nbd/nbd-standalone.c                   |  2 +-
>  server/protocol-handshake-newstyle.c           |  2 +-
>  server/protocol-handshake-oldstyle.c           |  2 +-
>  server/protocol-handshake.c                    |  2 +-
>  server/protocol.c                              |  2 +-
>  tests/test-layers.c                            |  2 +-
>  9 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/common/protocol/Makefile.am b/common/protocol/Makefile.am
> index 99df43b..996be26 100644
> --- a/common/protocol/Makefile.am
> +++ b/common/protocol/Makefile.am
> @@ -32,7 +32,7 @@
>  include $(top_srcdir)/common-rules.mk
>  
>  EXTRA_DIST = \
> -	protocol.h \
> +	nbd-protocol.h \
>  	protostrings.c \
>  	protostrings.sed \
>  	$(NULL)
> @@ -41,16 +41,16 @@ noinst_LTLIBRARIES = libprotocol.la
>  
>  libprotocol_la_SOURCES = \
>          protostrings.c \
> -	protocol.h \
> +	nbd-protocol.h \
Huh, we've got inconsistent space-v-tab, if you want to touch it while here.

ACK

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

Eric Blake

2019-Sep-24 22:12 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 2/4] common/protocol: Remove protostrings.sed, use bash+sed instead.

On 9/24/19 4:07 PM, Richard W.M. Jones wrote:> Use a simple bash script to generate the protostrings.c functions.
> 
> Remove the extern decls from the nbd-protocol.h file which were used
> previously in the generation of this file.  They have been moved to a
> new internal header called "protostrings.h".
> ---
>  common/protocol/Makefile.am                   |  8 ++-
>  ...tostrings.sed => generate-protostrings.sh} | 56 +++++++++++--------
Wow - git thinks .sed and .sh are similar!  Probably more a case of the
boilerplate copyright being identical, and the rest of the file being
short enough in spite of more-or-less wholesale rewrite.
>  common/protocol/nbd-protocol.h                | 10 ----
>  common/protocol/protostrings.h                | 51 +++++++++++++++++
>  plugins/nbd/nbd-standalone.c                  |  1 +
>  server/protocol-handshake-newstyle.c          |  1 +
>  server/protocol.c                             |  1 +
>  7 files changed, 91 insertions(+), 37 deletions(-)
> 
> +protostrings.c: nbd-protocol.h generate-protostrings.sh Makefile
>  	rm -f $@ $@-t
> -	$(SED) -n -f $(srcdir)/protostrings.sed < $< > $@-t
> +	SED=$(SED) $(srcdir)/generate-protostrings.sh > $@-t
Okay, not just a wholesale rewrite, but keeping a sed invocation while
wrapping in even more logic.
> --- a/common/protocol/protostrings.sed
> +++ b/common/protocol/generate-protostrings.sh
> @@ -1,5 +1,6 @@
> +#!/usr/bin/env bash
>  # nbdkit
> -# Copyright (C) 2018 Red Hat Inc.
> +# Copyright (C) 2018-2019 Red Hat Inc.
>  #
>  # Redistribution and use in source and binary forms, with or without
>  # modification, are permitted provided that the following conditions are
> @@ -29,30 +30,37 @@
>  # OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>  # SUCH DAMAGE.
>  
> -# Generate the protostrings.c file from nbd-protocol.h.
> +# The header.
> +cat <<'EOF'
> +/* Generated from nbd-protocol.h by generate-protostrings.sh. */
Worth using:

cat << EOF
/* Generated from nbd-protocol.h by $0. */

?  Or is that too prone to non-reproducible builds if $0 includes a full
path (would $(basename -- $0) fix that)?
>  
> -# Prologue.
> -1i\
> -/* Generated from nbd-protocol.h by protostrings.sed. */\
> -\#include "nbd-protocol.h"\
> +#include "nbd-protocol.h"
>  
> +EOF
We don't have a mention of license/copyright in the generated file
(other than implicitly assuming the same license as in the source files
that generated it).  Is now a good time to spit one out as part of our
prologue?
>  
> -# Match the precise sections of the source file.
> -/^extern const char \*name_of_/,/^$/ {
> +declare -A functions=(
> +    [global_flag]=NBD_FLAG_FIXED_NEWSTYLE
> +    [flag]=NBD_FLAG_HAS_FLAGS
> +    [opt]=NBD_OPT_EXPORT_NAME
> +    [rep]=NBD_REP_ACK
> +    [info]=NBD_INFO_EXPORT
> +    [reply]=NBD_REPLY_FLAG_DONE
> +    [reply_type]=NBD_REPLY_TYPE_NONE
> +    [cmd]=NBD_CMD_READ
> +    [cmd_flag]=NBD_CMD_FLAG_FUA
> +    [error]=NBD_SUCCESS
> +)
Definite bashism.  That's fine, given the #! line.
>  
> -  # 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;
> -
> -}
> +# Generate each 'const char *name_of_nbd_<fn>'
> +for fn in "${!functions[@]}"; do
Is this going to be deterministic, or are we at the mercy of bash's
hashing?  Should we sort the list to guarantee stable output order?
> +    echo 'extern const char *'
> +    echo "name_of_nbd_$fn (int fl)"
> +    echo '{'
> +    echo '  switch (fl) {'
> +    $SED -n "/^#define ${functions[$fn]}/,/^$/p" nbd-protocol.h
|
> +        $SED 's/^#define \([_A-Z]*\).*/  case \1:\n    return
"\1\";/'
> +    echo '  default: return "unknown";'
> +    echo '  }'
> +    echo '}'
> +    echo
> +done
No sanity checking that $# was 0, but for an internal-only build script,
that probably doesn't matter.  Looks like a sane conversion.

> diff --git a/plugins/nbd/nbd-standalone.c b/plugins/nbd/nbd-standalone.c
> index a6779a4..1789e39 100644
> --- a/plugins/nbd/nbd-standalone.c
> +++ b/plugins/nbd/nbd-standalone.c
> @@ -54,6 +54,7 @@
>  
>  #include <nbdkit-plugin.h>
>  #include "nbd-protocol.h"
> +#include "protostrings.h"
Should we switch from "nbd-protocol.h" to <nbd-protocol.h> at
some point
in the series, to reflect our intent of installing it? But doesn't
affect this patch.

ACK

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

Eric Blake

2019-Sep-24 22:20 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 3/4] common/protocol: Update nbd-protocol.h so it matches libnbd’s copy.

On 9/24/19 4:07 PM, Richard W.M. Jones wrote:> Diff against libnbd’s copy of this file, and change this one until it
> matches.
> ---
>  common/protocol/nbd-protocol.h       | 76 +++++++++++++++++-----------
>  server/protocol-handshake-newstyle.c | 26 +++++-----
>  server/protocol-handshake-oldstyle.c |  4 +-
>  server/protocol.c                    | 25 ++++-----
>  tests/test-layers.c                  | 14 ++---
>  5 files changed, 81 insertions(+), 64 deletions(-)
> 
> diff --git a/common/protocol/nbd-protocol.h
b/common/protocol/nbd-protocol.h
> index 60d35d0..724ffb6 100644
> --- a/common/protocol/nbd-protocol.h
> +++ b/common/protocol/nbd-protocol.h
> @@ -40,37 +40,46 @@
>   * these structures.
>   */
>  
> +#define NBD_MAX_STRING 4096 /* Maximum length of a string field */
> +
>  /* Old-style handshake. */
> -struct old_handshake {
> +struct nbd_old_handshake {
>    char nbdmagic[8];           /* "NBDMAGIC" */
> -  uint64_t version;           /* OLD_VERSION */
> +  uint64_t version;           /* NBD_OLD_VERSION */
>    uint64_t exportsize;
>    uint16_t gflags;            /* global flags */
>    uint16_t eflags;            /* per-export flags */
Different from the current NBD protocol spec, which calls out uint64_t
exportsize, uint32_t eflags (partly to emphasize that no oldstyle server
will ever set gflags).  The difference is not fatal.

>  
> +/* Newstyle handshake OPT_EXPORT_NAME reply message. */
> +struct nbd_export_name_option_reply {
> +  uint64_t exportsize;          /* size of export */
> +  uint16_t eflags;              /* per-export flags */
> +  char zeroes[124];             /* optional zeroes */
> +} __attribute__((packed));;
Double ;;

>  
>  #define NBD_INFO_EXPORT      0
We're missing other defined NBD_INFO_* constants (from both projects);
we'll get there when we add support for block size constraints, but
could add them now (even if they remain unused a bit longer) if desired.
 Probably as a separate patch.

> @@ -144,14 +157,14 @@ struct block_descriptor {
>  /* New-style handshake server reply when using NBD_OPT_EXPORT_NAME.
>   * Modern clients use NBD_OPT_GO instead of this.
>   */
> -struct new_handshake_finish {
> +struct nbd_new_handshake_finish {
>    uint64_t exportsize;
>    uint16_t eflags;            /* per-export flags */
>    char zeroes[124];           /* must be sent as zero bytes */
>  } __attribute__((packed));
Redundant with nbd_export_name_option_reply.

Otherwise looks good.

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

Eric Blake

2019-Sep-24 22:21 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 4/4] common/protocol: Install <nbd-protocol.h> as a public header.

On 9/24/19 4:07 PM, Richard W.M. Jones wrote:> Some further small changes are made to allow this header to be used
> from arbitrary ISO C compilers.
> 
> It can now be used from other projects.
> ---
>  common/protocol/Makefile.am    |  2 ++
>  common/protocol/nbd-protocol.h | 38 ++++++++++++++++++++--------------
>  2 files changed, 24 insertions(+), 16 deletions(-)
> 
>  /* Newstyle handshake OPT_EXPORT_NAME reply message. */
>  struct nbd_export_name_option_reply {
>    uint64_t exportsize;          /* size of export */
>    uint16_t eflags;              /* per-export flags */
>    char zeroes[124];             /* optional zeroes */
> -} __attribute__((packed));;
> +} NBD_ATTRIBUTE_PACKED;;
Still double ;;

ACK

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

Eric Blake

2019-Sep-25 12:28 UTC

head link

Re: [Libguestfs] [PATCH nbdkit 4/4] common/protocol: Install <nbd-protocol.h> as a public header.

On 9/24/19 4:07 PM, Richard W.M. Jones wrote:> Some further small changes are made to allow this header to be used
> from arbitrary ISO C compilers.
> 
> It can now be used from other projects.
> ---
>  
> +#if defined(__GNUC__) || defined(__clang__)
> +#define NBD_ATTRIBUTE_PACKED __attribute__((__packed__))
> +#else
> +#define NBD_ATTRIBUTE_PACKED
> +#endif
> +
Actually, I just realized that this fallback is wrong - it induces
padding bytes, and thus creates a struct incompatible with the wire
format.  We're better off making the #else use
#error Please port to your compiler's notion of a packed struct

so that the file serves as a reference, but does not get mistakenly used
with incorrect padding.
>  #define NBD_MAX_STRING 4096 /* Maximum length of a string field */
>  
>  /* Old-style handshake. */
> @@ -50,7 +56,7 @@ struct nbd_old_handshake {
>    uint16_t gflags;            /* global flags */
>    uint16_t eflags;            /* per-export flags */
>    char zeroes[124];           /* must be sent as zero bytes */
> -} __attribute__((packed));
> +} NBD_ATTRIBUTE_PACKED;
This one actually gets wire alignment without packing.
>  
>  #define NBD_OLD_VERSION UINT64_C(0x420281861253)
>  
> @@ -59,7 +65,7 @@ struct nbd_new_handshake {
>    char nbdmagic[8];           /* "NBDMAGIC" */
>    uint64_t version;           /* NBD_NEW_VERSION */
>    uint16_t gflags;            /* global flags */
> -} __attribute__((packed));
> +} NBD_ATTRIBUTE_PACKED;
This one would get tail padding, messing up anyone using sizeof(struct
nbd_new_handshake).
>  ...
> @@ -140,19 +146,19 @@ struct nbd_fixed_new_option_reply_info_export {
>    uint16_t info;                /* NBD_INFO_EXPORT */
>    uint64_t exportsize;          /* size of export */
>    uint16_t eflags;              /* per-export flags */
> -} __attribute__((packed));
> +} NBD_ATTRIBUTE_PACKED;
>  
and here's one that gets member offsets wrong, if packed is not used.


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

Reasonably Related Threads

Search for more reasonably related threads

Libguestfs - Sep 2019 - [PATCH nbdkit 3/4] common/protocol: Update nbd-protocol.h so it matches libnbd’s copy.

[Libguestfs] [PATCH nbdkit 0/4] common/protocol: Unify public <nbd-protocol.h>

[Libguestfs] [PATCH nbdkit 1/4] common/protocol: Rename protocol.h to nbd-protocol.h.

[Libguestfs] [PATCH nbdkit 2/4] common/protocol: Remove protostrings.sed, use bash+sed instead.

[Libguestfs] [PATCH nbdkit 3/4] common/protocol: Update nbd-protocol.h so it matches libnbd’s copy.

[Libguestfs] [PATCH nbdkit 4/4] common/protocol: Install <nbd-protocol.h> as a public header.

[Libguestfs] [PATCH nbdkit 0/4] common/protocol: Unify public <nbd-protocol.h>

Re: [Libguestfs] [PATCH nbdkit 0/4] common/protocol: Unify public <nbd-protocol.h>

Re: [Libguestfs] [PATCH nbdkit 1/4] common/protocol: Rename protocol.h to nbd-protocol.h.

Re: [Libguestfs] [PATCH nbdkit 2/4] common/protocol: Remove protostrings.sed, use bash+sed instead.

Re: [Libguestfs] [PATCH nbdkit 3/4] common/protocol: Update nbd-protocol.h so it matches libnbd’s copy.

Re: [Libguestfs] [PATCH nbdkit 4/4] common/protocol: Install <nbd-protocol.h> as a public header.

Re: [Libguestfs] [PATCH nbdkit 4/4] common/protocol: Install <nbd-protocol.h> as a public header.

Reasonably Related Threads