Libguestfs - Sep 2020 - Re: [libnbd PATCH v2 4/5] api: Add STRICT_FLAGS to set_strict_mode

In v2:
- now based on my proposal to add LIBNBD_SHUTDOWN_IMMEDIATE
- four flags instead of two: STRICT_FLAGS is new (patch 4),
and STRICT_BOUNDS is separate from STRICT_ZERO_SIZE (patch 5)
- various refactorings for more shared code and less duplication

Eric Blake (5):
  api: Add xxx_MASK constant for each Flags type
  generator: Refactor filtering of accepted OFlags
  api: Add nbd_set_strict_mode
  api: Add STRICT_FLAGS to set_strict_mode
  api: Add STRICT_BOUNDS/ZERO_SIZE to nbd_set_strict_mode

 docs/libnbd.pod                               |   7 +
 lib/internal.h                                |   6 +-
 generator/API.ml                              | 291 ++++++++++++++----
 generator/API.mli                             |   4 +-
 generator/C.ml                                |  38 ++-
 generator/GoLang.ml                           |  18 +-
 generator/OCaml.ml                            |  15 +-
 generator/Python.ml                           |  15 +-
 lib/disconnect.c                              |  13 +-
 lib/handle.c                                  |  21 +-
 lib/rw.c                                      | 232 ++++++--------
 python/t/110-defaults.py                      |   3 +-
 python/t/120-set-non-defaults.py              |   5 +-
 ocaml/tests/test_110_defaults.ml              |   3 +-
 ocaml/tests/test_120_set_non_defaults.ml      |   3 +-
 tests/errors.c                                |  72 ++++-
 .../libnbd/libnbd_110_defaults_test.go        |   2 +-
 .../libnbd_120_set_non_defaults_test.go       |   4 +-
 18 files changed, 488 insertions(+), 264 deletions(-)

-- 
2.28.0

Document and make it easier for applications to determine which
bitmask values were supported at compile time.  Also, generating
bitmask values in hex tends to be more legible than in decimal, as it
makes it obvious that the values are intended for bitwise operations.
And it doesn't hurt that this reduces churn in some of the tests if
new bits are added.
---
 docs/libnbd.pod                                     |  7 +++++++
 generator/API.ml                                    | 13 +++++++++++--
 generator/C.ml                                      | 13 ++++++++-----
 generator/GoLang.ml                                 |  8 ++++++--
 generator/OCaml.ml                                  |  9 +++++++++
 generator/Python.ml                                 |  5 ++++-
 lib/handle.c                                        |  5 ++---
 python/t/110-defaults.py                            |  3 +--
 python/t/120-set-non-defaults.py                    |  5 ++---
 ocaml/tests/test_110_defaults.ml                    |  3 +--
 ocaml/tests/test_120_set_non_defaults.ml            |  3 +--
 tests/errors.c                                      |  9 +++------
 .../libnbd/libnbd_110_defaults_test.go              |  2 +-
 .../libnbd/libnbd_120_set_non_defaults_test.go      |  4 ++--
 14 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/docs/libnbd.pod b/docs/libnbd.pod
index f2ba3bb..d8dffea 100644
--- a/docs/libnbd.pod
+++ b/docs/libnbd.pod
@@ -520,6 +520,13 @@ To get the size of the export in bytes, use
L<nbd_get_size(3)>:
 You can read and write data from the NBD server using L<nbd_pread(3)>
 and L<nbd_pwrite(3)> or their asynchronous equivalents.

+All data commands support a C<flags> argument (mandatory in C, but
+optional in languages where it can default to 0).  For convenience,
+the constant C<LIBNBD_CMD_FLAG_MASK> is defined with the set of flags
+currently recognized by libnbd, where future NBD protocol extensions
+may result in additional flags being supported; but in general,
+specific data commands only accept a subset of known flags.
+
 Some servers also support:

 =over 4
diff --git a/generator/API.ml b/generator/API.ml
index cc1a7ba..42eeac0 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -679,6 +679,8 @@ fewer all-zero padding bytes over the connection.

 =back

+For convenience, the constant C<LIBNBD_HANDSHAKE_FLAG_MASK> is
+available to describe all flags supported by this build of libnbd.
 Future NBD extensions may add further flags, which in turn may
 be enabled by default in newer libnbd.  As such, when attempting
 to disable only one specific bit, it is wiser to first call
@@ -895,7 +897,11 @@ ORed together:

 =item C<LIBNBD_ALLOW_TRANSPORT_VSOCK>

-=back";
+=back
+
+For convenience, the constant C<LIBNBD_ALLOW_TRANSPORT_MASK> is
+available to describe all transports recognized by this build of
+libnbd.  A future version of the library may add new flags.";
     see_also = [Link "connect_uri"; Link
"set_uri_allow_tls"];
   };

@@ -1629,7 +1635,10 @@ issuing those commands before informing the server of the
intent
 to disconnect.

 =back
-";
+
+For convenience, the constant C<LIBNBD_SHUTDOWN_MASK> is available
+to describe all shutdown flags recognized by this build of libnbd.
+A future version of the library may add new flags.";
     see_also = [Link "close"; Link "aio_disconnect"];
     example = Some "examples/reads-and-writes.c";
   };
diff --git a/generator/C.ml b/generator/C.ml
index c9f0ff4..4d4958d 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -349,11 +349,15 @@ let generate_include_libnbd_h ()    ) all_enums;
   List.iter (
     fun { flag_prefix; flags } ->
+      let mask = ref 0 in
       List.iter (
         fun (flag, i) ->
           let flag = sprintf "LIBNBD_%s_%s" flag_prefix flag in
-          pr "#define %-40s %d\n" flag i
+          pr "#define %-40s 0x%02x\n" flag i;
+          mask := !mask lor i
       ) flags;
+      let flag = sprintf "LIBNBD_%s_MASK" flag_prefix in
+      pr "#define %-40s 0x%02x\n" flag !mask;
       pr "\n"
   ) all_flags;
   List.iter (
@@ -490,13 +494,12 @@ let generate_lib_api_c ()      );

     (* Check parameters are valid. *)
-    let print_flags_check n { flag_prefix; flags } +    let print_flags_check n
{ flag_prefix }        let value = match errcode with
         | Some value -> value
         | None -> assert false in
-      let mask = List.fold_left (lor) 0 (List.map snd flags) in
-      pr "  if (unlikely ((%s & ~%d) != 0)) {\n" n mask;
-      pr "    set_error (EINVAL, \"%%s: invalid value for flag:
%%d\",\n";
+      pr "  if (unlikely ((%s & ~LIBNBD_%s_MASK) != 0)) {\n" n
flag_prefix;
+      pr "    set_error (EINVAL, \"%%s: invalid value for flag:
0x%%x\",\n";
       pr "               \"%s\", %s);\n" n n;
       pr "    ret = %s;\n" value;
       pr "    goto out;\n";
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 359c7d0..f07b074 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -447,12 +447,16 @@ import (
 ";
   List.iter (
     fun { flag_prefix; flags } ->
-      pr "type %s uint32\n" (camel_case flag_prefix);
+      let flag_type = camel_case flag_prefix in
+      let mask = ref 0 in
+      pr "type %s uint32\n" flag_type;
       pr "const (\n";
       List.iter (
         fun (flag, v) ->
-          pr "    %s_%s = %s(%d)\n" flag_prefix flag (camel_case
flag_prefix) v
+          pr "    %s_%s = %s(0x%02x)\n" flag_prefix flag flag_type v;
+          mask := !mask lor v
       ) flags;
+      pr "    %s_MASK = %s(0x%02x)\n" flag_prefix flag_type !mask;
       pr ")\n";
       pr "\n"
   ) all_flags;
diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index 4bcd450..43b3679 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -164,6 +164,8 @@ type cookie = int64
           pr "  | %s\n" flag
       ) flags;
       pr "  | UNKNOWN of int\n";
+      pr "\n";
+      pr "  val mask : t list\n";
       pr "end\n";
       pr "\n"
   ) all_flags;
@@ -269,6 +271,13 @@ let ()            pr "  | %s\n" flag
       ) flags;
       pr "  | UNKNOWN of int\n";
+      pr "\n";
+      pr "  let mask = [\n";
+      List.iter (
+        fun (flag, _) ->
+          pr "    %s;\n" flag
+      ) flags;
+      pr "  ]\n";
       pr "end\n";
       pr "\n"
   ) all_flags;
diff --git a/generator/Python.ml b/generator/Python.ml
index 71368c6..fd09eae 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -696,11 +696,14 @@ Error.__str__ = _str
   ) all_enums;
   List.iter (
     fun { flag_prefix; flags } ->
+      let mask = ref 0 in
       List.iter (
         fun (flag, i) ->
           let flag = sprintf "%s_%s" flag_prefix flag in
-          pr "%s = %d\n" flag i
+          pr "%s = 0x%02x\n" flag i;
+          mask := !mask lor i
       ) flags;
+      pr "%s_MASK = 0x%02x\n" flag_prefix !mask;
       pr "\n"
   ) all_flags;
   List.iter (fun (n, i) -> pr "%s = %d\n" n i) constants;
diff --git a/lib/handle.c b/lib/handle.c
index 7987d59..4d26842 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -65,12 +65,11 @@ nbd_create (void)
   h->tls_verify_peer = true;
   h->request_sr = true;

-  h->uri_allow_transports = (uint32_t) -1;
+  h->uri_allow_transports = LIBNBD_ALLOW_TRANSPORT_MASK;
   h->uri_allow_tls = LIBNBD_TLS_ALLOW;
   h->uri_allow_local_file = false;

-  h->gflags = (LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE |
-               LIBNBD_HANDSHAKE_FLAG_NO_ZEROES);
+  h->gflags = LIBNBD_HANDSHAKE_FLAG_MASK;

   s = getenv ("LIBNBD_DEBUG");
   h->debug = s && strcmp (s, "1") == 0;
diff --git a/python/t/110-defaults.py b/python/t/110-defaults.py
index 47c1d02..fb961cf 100644
--- a/python/t/110-defaults.py
+++ b/python/t/110-defaults.py
@@ -22,6 +22,5 @@ assert h.get_export_name() == ""
 assert h.get_full_info() is False
 assert h.get_tls() == nbd.TLS_DISABLE
 assert h.get_request_structured_replies() is True
-assert h.get_handshake_flags() == (nbd.HANDSHAKE_FLAG_FIXED_NEWSTYLE |
-                                   nbd.HANDSHAKE_FLAG_NO_ZEROES)
+assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK
 assert h.get_opt_mode() is False
diff --git a/python/t/120-set-non-defaults.py b/python/t/120-set-non-defaults.py
index 3963021..3da0c23 100644
--- a/python/t/120-set-non-defaults.py
+++ b/python/t/120-set-non-defaults.py
@@ -34,11 +34,10 @@ if h.supports_tls():
 h.set_request_structured_replies(False)
 assert h.get_request_structured_replies() is False
 try:
-    h.set_handshake_flags(nbd.HANDSHAKE_FLAG_NO_ZEROES << 1)
+    h.set_handshake_flags(nbd.HANDSHAKE_FLAG_MASK + 1)
     assert False
 except nbd.Error:
-    assert h.get_handshake_flags() == (nbd.HANDSHAKE_FLAG_NO_ZEROES |
-                                       nbd.HANDSHAKE_FLAG_FIXED_NEWSTYLE)
+    assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK
 h.set_handshake_flags(0)
 assert h.get_handshake_flags() == 0
 h.set_opt_mode(True)
diff --git a/ocaml/tests/test_110_defaults.ml b/ocaml/tests/test_110_defaults.ml
index 54f2cbc..f5886fc 100644
--- a/ocaml/tests/test_110_defaults.ml
+++ b/ocaml/tests/test_110_defaults.ml
@@ -28,8 +28,7 @@ let ()    let sr = NBD.get_request_structured_replies nbd in
   assert (sr = true);
   let flags = NBD.get_handshake_flags nbd in
-  assert (flags = [ NBD.HANDSHAKE_FLAG.FIXED_NEWSTYLE;
-                    NBD.HANDSHAKE_FLAG.NO_ZEROES ]);
+  assert (flags = NBD.HANDSHAKE_FLAG.mask);
   let opt = NBD.get_opt_mode nbd in
   assert (opt = false)

diff --git a/ocaml/tests/test_120_set_non_defaults.ml
b/ocaml/tests/test_120_set_non_defaults.ml
index 79fe184..b660e5d 100644
--- a/ocaml/tests/test_120_set_non_defaults.ml
+++ b/ocaml/tests/test_120_set_non_defaults.ml
@@ -46,8 +46,7 @@ let ()    with
     NBD.Error _ -> ();
   let flags = NBD.get_handshake_flags nbd in
-  assert (flags = [ NBD.HANDSHAKE_FLAG.FIXED_NEWSTYLE;
-                    NBD.HANDSHAKE_FLAG.NO_ZEROES ]);
+  assert (flags = NBD.HANDSHAKE_FLAG.mask);
   NBD.set_handshake_flags nbd [];
   let flags = NBD.get_handshake_flags nbd in
   assert (flags = []);
diff --git a/tests/errors.c b/tests/errors.c
index 906c7da..8166429 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -86,7 +86,6 @@ main (int argc, char *argv[])
    */
   const char *cmd[] = { "nbdkit", "-s",
"--exit-with-parent", "sh",
                         script, NULL };
-  int i;

   progname = argv[0];

@@ -147,15 +146,13 @@ main (int argc, char *argv[])
   }

   /* Attempt to set a bitmask with an unknown bit. */
-  i = LIBNBD_HANDSHAKE_FLAG_NO_ZEROES << 1;
-  if (nbd_set_handshake_flags (nbd, i) != -1) {
+  if (nbd_set_handshake_flags (nbd, LIBNBD_HANDSHAKE_FLAG_MASK + 1) != -1) {
     fprintf (stderr, "%s: test failed: "
              "nbd_set_handshake_flags did not reject invalid
bitmask\n",
              argv[0]);
     exit (EXIT_FAILURE);
   }
-  i = LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE | LIBNBD_HANDSHAKE_FLAG_NO_ZEROES;
-  if (nbd_get_handshake_flags (nbd) != i) {
+  if (nbd_get_handshake_flags (nbd) != LIBNBD_HANDSHAKE_FLAG_MASK) {
     fprintf (stderr, "%s: test failed: "
              "nbd_get_handshake_flags not left at default value\n",
              argv[0]);
@@ -247,7 +244,7 @@ main (int argc, char *argv[])
   check (EINVAL, "nbd_pread: ");

   /* Use unknown command flags */
-  if (nbd_pread (nbd, buf, 1, 0, -1) != -1) {
+  if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
     fprintf (stderr, "%s: test failed: "
              "nbd_pread did not fail with bogus flags\n",
              argv[0]);
diff --git a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
index 9af0ed8..9e782ac 100644
--- a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
+++ b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
@@ -63,7 +63,7 @@ func Test110Defaults(t *testing.T) {
 	if err != nil {
 		t.Fatalf("could not get handshake flags: %s", err)
 	}
-	if flags != HANDSHAKE_FLAG_FIXED_NEWSTYLE | HANDSHAKE_FLAG_NO_ZEROES {
+	if flags != HANDSHAKE_FLAG_MASK {
 		t.Fatalf("unexpected handshake flags")
 	}

diff --git
a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
index 5d8cce7..b0b06bd 100644
--- a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
+++ b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
@@ -93,7 +93,7 @@ func Test120SetNonDefaults(t *testing.T) {
 		t.Fatalf("unexpected structured replies state")
 	}

-	err = h.SetHandshakeFlags(HANDSHAKE_FLAG_NO_ZEROES << 1)
+	err = h.SetHandshakeFlags(HANDSHAKE_FLAG_MASK + 1)
 	if err == nil {
 		t.Fatalf("expect failure for out-of-range flags")
 	}
@@ -101,7 +101,7 @@ func Test120SetNonDefaults(t *testing.T) {
 	if err != nil {
 		t.Fatalf("could not get handshake flags: %s", err)
 	}
-	if flags != HANDSHAKE_FLAG_FIXED_NEWSTYLE | HANDSHAKE_FLAG_NO_ZEROES {
+	if flags != HANDSHAKE_FLAG_MASK {
 		t.Fatalf("unexpected handshake flags")
 	}

-- 
2.28.0

Rather than having to open-code the list of accepted command flags in
the unlocked version of each command, we can store that information in
the generator to produce the check directly in the public API.
---
 generator/API.ml    | 53 +++++++++++++++++++++++++++++----------------
 generator/API.mli   |  3 ++-
 generator/C.ml      | 26 +++++++++++++++-------
 generator/GoLang.ml | 10 ++++-----
 generator/OCaml.ml  |  6 ++---
 generator/Python.ml | 10 ++++-----
 lib/disconnect.c    | 10 ---------
 lib/rw.c            | 45 --------------------------------------
 8 files changed, 67 insertions(+), 96 deletions(-)

diff --git a/generator/API.ml b/generator/API.ml
index 42eeac0..b212e95 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -55,7 +55,7 @@ and arg  | UInt64 of string
 and optarg  | OClosure of closure
-| OFlags of string * flags
+| OFlags of string * flags * string list option
 and ret  | RBool
 | RStaticString
@@ -1485,7 +1485,11 @@ Future NBD extensions may result in additional
C<size_type> values.
   "pread", {
     default_call with
     args = [ BytesOut ("buf", "count"); UInt64
"offset" ];
-    optargs = [ OFlags ("flags", cmd_flags) ];
+    (* We could silently accept flag DF, but it really only makes sense
+     * with callbacks, because otherwise there is no observable change
+     * except that the server may fail where it would otherwise succeed.
+     *)
+    optargs = [ OFlags ("flags", cmd_flags, Some []) ];
     ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "read from the NBD server";
@@ -1509,7 +1513,7 @@ protocol extensions).";
     default_call with
     args = [ BytesOut ("buf", "count"); UInt64
"offset";
              Closure chunk_closure ];
-    optargs = [ OFlags ("flags", cmd_flags) ];
+    optargs = [ OFlags ("flags", cmd_flags, Some ["DF"]) ];
     ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "read from the NBD server";
@@ -1585,7 +1589,7 @@ actually obeys the flag.";
   "pwrite", {
     default_call with
     args = [ BytesIn ("buf", "count"); UInt64
"offset" ];
-    optargs = [ OFlags ("flags", cmd_flags) ];
+    optargs = [ OFlags ("flags", cmd_flags, Some ["FUA"])
];
     ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "write to the NBD server";
@@ -1608,7 +1612,8 @@ L<nbd_can_fua(3)>).";

   "shutdown", {
     default_call with
-    args = []; optargs = [ OFlags ("flags", shutdown_flags) ]; ret =
RErr;
+    args = []; optargs = [ OFlags ("flags", shutdown_flags, None) ];
+    ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "disconnect from the NBD server";
     longdesc = "\
@@ -1645,7 +1650,7 @@ A future version of the library may add new flags.";

   "flush", {
     default_call with
-    args = []; optargs = [ OFlags ("flags", cmd_flags) ]; ret = RErr;
+    args = []; optargs = [ OFlags ("flags", cmd_flags, Some []) ];
ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "send flush command to the NBD server";
     longdesc = "\
@@ -1662,7 +1667,7 @@ protocol extensions).";
   "trim", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset" ];
-    optargs = [ OFlags ("flags", cmd_flags) ];
+    optargs = [ OFlags ("flags", cmd_flags, Some ["FUA"])
];
     ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "send trim command to the NBD server";
@@ -1685,7 +1690,7 @@ L<nbd_can_fua(3)>).";
   "cache", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset" ];
-    optargs = [ OFlags ("flags", cmd_flags) ];
+    optargs = [ OFlags ("flags", cmd_flags, Some []) ];
     ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "send cache (prefetch) command to the NBD server";
@@ -1705,7 +1710,8 @@ protocol extensions).";
   "zero", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset" ];
-    optargs = [ OFlags ("flags", cmd_flags) ];
+    optargs = [ OFlags ("flags", cmd_flags,
+                        Some ["FUA"; "NO_HOLE";
"FAST_ZERO"]) ];
     ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "send write zeroes command to the NBD server";
@@ -1733,7 +1739,7 @@ cannot do this, see L<nbd_can_fast_zero(3)>).";
   "block_status", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset"; Closure
extent_closure ];
-    optargs = [ OFlags ("flags", cmd_flags) ];
+    optargs = [ OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
     ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "send block status command to the NBD server";
@@ -2026,7 +2032,8 @@ callback.";
   "aio_pread", {
     default_call with
     args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags, Some []) ];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "read from the NBD server";
@@ -2048,7 +2055,8 @@ completed.  Other parameters behave as documented in
L<nbd_pread(3)>.";
     default_call with
     args = [ BytesPersistOut ("buf", "count"); UInt64
"offset";
              Closure chunk_closure ];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags, Some ["DF"]) ];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "read from the NBD server";
@@ -2067,7 +2075,8 @@ Other parameters behave as documented in
L<nbd_pread_structured(3)>.";
   "aio_pwrite", {
     default_call with
     args = [ BytesPersistIn ("buf", "count"); UInt64
"offset" ];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags, Some ["FUA"])
];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "write to the NBD server";
@@ -2086,7 +2095,7 @@ completed.  Other parameters behave as documented in
L<nbd_pwrite(3)>.";

   "aio_disconnect", {
     default_call with
-    args = []; optargs = [ OFlags ("flags", cmd_flags) ]; ret = RErr;
+    args = []; optargs = [ OFlags ("flags", cmd_flags, Some []) ];
ret = RErr;
     permitted_states = [ Connected ];
     shortdesc = "disconnect from the NBD server";
     longdesc = "\
@@ -2111,7 +2120,8 @@ however, L<nbd_shutdown(3)> will call this function
if appropriate.";
   "aio_flush", {
     default_call with
     args = [];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags, Some []) ];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "send flush command to the NBD server";
@@ -2130,7 +2140,8 @@ Other parameters behave as documented in
L<nbd_flush(3)>.";
   "aio_trim", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset" ];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags, Some ["FUA"])
];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "send trim command to the NBD server";
@@ -2149,7 +2160,8 @@ Other parameters behave as documented in
L<nbd_trim(3)>.";
   "aio_cache", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset" ];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags, Some []) ];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "send cache (prefetch) command to the NBD server";
@@ -2168,7 +2180,9 @@ Other parameters behave as documented in
L<nbd_cache(3)>.";
   "aio_zero", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset" ];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags,
+                        Some ["FUA"; "NO_HOLE";
"FAST_ZERO"]) ];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "send write zeroes command to the NBD server";
@@ -2188,7 +2202,8 @@ Other parameters behave as documented in
L<nbd_zero(3)>.";
   "aio_block_status", {
     default_call with
     args = [ UInt64 "count"; UInt64 "offset"; Closure
extent_closure ];
-    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
+    optargs = [ OClosure completion_closure;
+                OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
     ret = RCookie;
     permitted_states = [ Connected ];
     shortdesc = "send block status command to the NBD server";
diff --git a/generator/API.mli b/generator/API.mli
index e45b5c0..db978ca 100644
--- a/generator/API.mli
+++ b/generator/API.mli
@@ -65,7 +65,8 @@ and arg  | UInt64 of string         (** 64 bit unsigned int *)
 and optarg  | OClosure of closure      (** optional closure *)
-| OFlags of string * flags (** optional flags, uint32_t in C *)
+| OFlags of string * flags * string list option (** optional flags, uint32_t
+                                                    in C, and valid subset *)
 and ret  | RBool                    (** return a boolean, or error *)
 | RStaticString            (** return a static string (must be located in
diff --git a/generator/C.ml b/generator/C.ml
index 4d4958d..86d9c5c 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -191,7 +191,7 @@ and print_arg_list' ?(handle = false) ?(types = true)
?(closure_style = Direct)
            | AddressOf -> "&"
            | Pointer -> "*" in
          pr "%s%s_callback" mark cbname
-      | OFlags (n, _) ->
+      | OFlags (n, _, _) ->
          if types then pr "uint32_t ";
          pr "%s" n
   ) optargs
@@ -494,11 +494,21 @@ let generate_lib_api_c ()      );

     (* Check parameters are valid. *)
-    let print_flags_check n { flag_prefix } +    let print_flags_check n {
flag_prefix; flags } subset        let value = match errcode with
         | Some value -> value
         | None -> assert false in
-      pr "  if (unlikely ((%s & ~LIBNBD_%s_MASK) != 0)) {\n" n
flag_prefix;
+      let mask = match subset with
+        | Some [] -> "0"
+        | Some subset ->
+           let v = ref 0 in
+           List.iter (
+             fun (flag, i) ->
+               if List.mem flag subset then v := !v lor i
+           ) flags;
+           sprintf "0x%x" !v
+        | None -> "LIBNBD_" ^ flag_prefix ^ "_MASK" in
+      pr "  if (unlikely ((%s & ~%s) != 0)) {\n" n mask;
       pr "    set_error (EINVAL, \"%%s: invalid value for flag:
0x%%x\",\n";
       pr "               \"%s\", %s);\n" n n;
       pr "    ret = %s;\n" value;
@@ -536,7 +546,7 @@ let generate_lib_api_c ()           pr "  }\n";
          need_out_label := true
       | Flags (n, flags) ->
-         print_flags_check n flags
+         print_flags_check n flags None
       | String n ->
          let value = match errcode with
            | Some value -> value
@@ -552,8 +562,8 @@ let generate_lib_api_c ()      List.iter (
       function
       | OClosure _ -> ()
-      | OFlags (n, flags) ->
-         print_flags_check n flags
+      | OFlags (n, flags, subset) ->
+         print_flags_check n flags subset
     ) optargs;

     (* Make the call. *)
@@ -635,7 +645,7 @@ let generate_lib_api_c ()      List.iter (
       function
       | OClosure { cbname } -> pr " %s=%%s" cbname
-      | OFlags (n, _) -> pr " %s=0x%%x" n
+      | OFlags (n, _, _) -> pr " %s=0x%%x" n
     ) optargs;
     pr "\"";
     List.iter (
@@ -660,7 +670,7 @@ let generate_lib_api_c ()        function
       | OClosure { cbname } ->
          pr ", CALLBACK_IS_NULL (%s_callback) ? \"<fun>\"
: \"NULL\"" cbname
-      | OFlags (n, _) -> pr ", %s" n
+      | OFlags (n, _, _) -> pr ", %s" n
     ) optargs;
     pr ");\n";
     List.iter (
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index f07b074..81446a6 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -83,7 +83,7 @@ let go_arg_type = function

 let go_name_of_optarg = function
   | OClosure { cbname } -> sprintf "%sCallback" (camel_case
cbname)
-  | OFlags (n, _) -> String.capitalize_ascii n
+  | OFlags (n, _, _) -> String.capitalize_ascii n

 let go_ret_type = function
   (* RErr returns only the error, with no return value. *)
@@ -202,7 +202,7 @@ let print_binding (name, { args; optargs; ret; shortdesc }) 
pr "  %s " fname;
         (match optarg with
          | OClosure { cbname } -> pr "%sCallback" (camel_case
cbname)
-         | OFlags (_, {flag_prefix}) -> pr "%s" (camel_case
flag_prefix)
+         | OFlags (_, {flag_prefix}, _) -> pr "%s" (camel_case
flag_prefix)
         );
         pr "\n"
     ) optargs;
@@ -298,7 +298,7 @@ let print_binding (name, { args; optargs; ret; shortdesc }) 
function
       | OClosure { cbname } -> pr "    var c_%s
C.nbd_%s_callback\n"
                                  cbname cbname
-      | OFlags (n, _) -> pr "    var c_%s C.uint32_t\n" n
+      | OFlags (n, _, _) -> pr "    var c_%s C.uint32_t\n" n
     ) optargs;
     pr "    if optargs != nil {\n";
     List.iter (
@@ -312,7 +312,7 @@ let print_binding (name, { args; optargs; ret; shortdesc }) 
cbname cbname;
              pr "            c_%s.user_data = unsafe.Pointer (C.long_to_vp
(C.long (registerCallbackId (optargs.%s))))\n"
                cbname (go_name_of_optarg optarg)
-          | OFlags (n, _) ->
+          | OFlags (n, _, _) ->
              pr "            c_%s = C.uint32_t (optargs.%s)\n"
                n (go_name_of_optarg optarg);
          );
@@ -346,7 +346,7 @@ let print_binding (name, { args; optargs; ret; shortdesc }) 
List.iter (
     function
     | OClosure { cbname} -> pr ", c_%s" cbname
-    | OFlags (n, _) -> pr ", c_%s" n
+    | OFlags (n, _, _) -> pr ", c_%s" n
   ) optargs;
   pr ")\n";

diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index 43b3679..28acb50 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -72,7 +72,7 @@ and ocaml_ret_to_string = function
 and ocaml_optarg_to_string = function
   | OClosure { cbname; cbargs } ->
      sprintf "?%s:(%s)" cbname (ocaml_closuredecl_to_string cbargs)
-  | OFlags (n, { flag_prefix }) -> sprintf "?%s:%s.t list" n
flag_prefix
+  | OFlags (n, { flag_prefix }, _) -> sprintf "?%s:%s.t list" n
flag_prefix

 and ocaml_closuredecl_to_string cbargs    let cbargs = List.map
ocaml_cbarg_to_string cbargs in
@@ -112,7 +112,7 @@ let ocaml_name_of_arg = function

 let ocaml_name_of_optarg = function
   | OClosure { cbname } -> cbname
-  | OFlags (n, _) -> n
+  | OFlags (n, _, _) -> n

 let num_params args optargs    List.length optargs + 1 (* handle *) +
List.length args
@@ -614,7 +614,7 @@ let print_ocaml_binding (name, { args; optargs; ret })      
pr "  }\n";
        pr "  %s_callback.user_data = %s_user_data;\n" cbname cbname;
        pr "  %s_callback.free = free_user_data;\n" cbname;
-    | OFlags (n, { flag_prefix }) ->
+    | OFlags (n, { flag_prefix }, _) ->
        pr "  uint32_t %s;\n" n;
        pr "  if (%sv != Val_int (0)) /* Some [ list of %s.t ] */\n"
          n flag_prefix;
diff --git a/generator/Python.ml b/generator/Python.ml
index fd09eae..1705ad9 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -320,7 +320,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error }         pr "  nbd_%s_callback %s = { .callback =
%s_wrapper,\n"
          cbname cbname cbname;
        pr "                         .free = free_user_data };\n"
-    | OFlags (n, _) ->
+    | OFlags (n, _, _) ->
        pr "  uint32_t %s_u32;\n" n;
        pr "  unsigned int %s; /* really uint32_t */\n" n
   ) optargs;
@@ -378,7 +378,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error }    List.iter (
     function
     | OClosure { cbname } -> pr ", &py_%s_fn" cbname
-    | OFlags (n, _) -> pr ", &%s" n
+    | OFlags (n, _, _) -> pr ", &%s" n
   ) optargs;
   pr "))\n";
   pr "    goto out;\n";
@@ -402,7 +402,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error }         pr "  }\n";
        pr "  else\n";
        pr "    %s.callback = NULL; /* we're not going to call it
*/\n" cbname
-    | OFlags (n, _) -> pr "  %s_u32 = %s;\n" n n
+    | OFlags (n, _, _) -> pr "  %s_u32 = %s;\n" n n
   ) optargs;
   List.iter (
     function
@@ -472,7 +472,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error }    List.iter (
     function
     | OClosure { cbname } -> pr ", %s" cbname
-    | OFlags (n, _) -> pr ", %s_u32" n
+    | OFlags (n, _, _) -> pr ", %s_u32" n
   ) optargs;
   pr ");\n";
   List.iter (
@@ -802,7 +802,7 @@ class NBD(object):
         List.map (
           function
           | OClosure { cbname } -> cbname, Some "None", None
-          | OFlags (n, _) -> n, Some "0", None
+          | OFlags (n, _, _) -> n, Some "0", None
         ) optargs in
       let args = args @ optargs in
       pr "    def %s(" name;
diff --git a/lib/disconnect.c b/lib/disconnect.c
index b8356b7..9de1e34 100644
--- a/lib/disconnect.c
+++ b/lib/disconnect.c
@@ -30,11 +30,6 @@
 int
 nbd_unlocked_shutdown (struct nbd_handle *h, uint32_t flags)
 {
-  if ((flags & ~LIBNBD_SHUTDOWN_IMMEDIATE) != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   /* If IMMEDIATE, abort any commands that have not yet had any bytes
    * sent to the server, so that NBD_CMD_DISC will be first in line.
    */
@@ -69,11 +64,6 @@ nbd_unlocked_aio_disconnect (struct nbd_handle *h, uint32_t
flags)
 {
   int64_t id;

-  if (flags != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   id = nbd_internal_command_common (h, 0, NBD_CMD_DISC, 0, 0, NULL, NULL);
   if (id == -1)
     return -1;
diff --git a/lib/rw.c b/lib/rw.c
index f3adb71..95c002c 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -280,15 +280,6 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf,
 {
   struct command_cb cb = { .completion = *completion };

-  /* We could silently accept flag DF, but it really only makes sense
-   * with callbacks, because otherwise there is no observable change
-   * except that the server may fail where it would otherwise succeed.
-   */
-  if (flags != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count,
                                       buf, &cb);
@@ -304,11 +295,6 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h,
void *buf,
   struct command_cb cb = { .fn.chunk = *chunk,
                            .completion = *completion };

-  if ((flags & ~LIBNBD_CMD_FLAG_DF) != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   if ((flags & LIBNBD_CMD_FLAG_DF) != 0 &&
       nbd_unlocked_can_df (h) != 1) {
     set_error (EINVAL, "server does not support the DF flag");
@@ -334,11 +320,6 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void
*buf,
     return -1;
   }

-  if ((flags & ~LIBNBD_CMD_FLAG_FUA) != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
       nbd_unlocked_can_fua (h) != 1) {
     set_error (EINVAL, "server does not support the FUA flag");
@@ -362,11 +343,6 @@ nbd_unlocked_aio_flush (struct nbd_handle *h,
     return -1;
   }

-  if (flags != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, 0, NBD_CMD_FLUSH, 0, 0,
                                       NULL, &cb);
@@ -389,11 +365,6 @@ nbd_unlocked_aio_trim (struct nbd_handle *h,
     return -1;
   }

-  if ((flags & ~LIBNBD_CMD_FLAG_FUA) != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
       nbd_unlocked_can_fua (h) != 1) {
     set_error (EINVAL, "server does not support the FUA flag");
@@ -427,11 +398,6 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,
     return -1;
   }

-  if (flags != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count,
                                       NULL, &cb);
@@ -454,12 +420,6 @@ nbd_unlocked_aio_zero (struct nbd_handle *h,
     return -1;
   }

-  if ((flags & ~(LIBNBD_CMD_FLAG_FUA | LIBNBD_CMD_FLAG_NO_HOLE |
-                 LIBNBD_CMD_FLAG_FAST_ZERO)) != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
       nbd_unlocked_can_fua (h) != 1) {
     set_error (EINVAL, "server does not support the FUA flag");
@@ -504,11 +464,6 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
     return -1;
   }

-  if ((flags & ~LIBNBD_CMD_FLAG_REQ_ONE) != 0) {
-    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
-    return -1;
-  }
-
   if (count == 0) {             /* NBD protocol forbids this. */
     set_error (EINVAL, "count cannot be 0");
     return -1;
-- 
2.28.0

Right now, libnbd has refused to issue a command not advertised as
supported by a server, mainly because the NBD protocol does not
guarantee what the server will do, and libnbd would rather stay in
sync with the server than drop the connection.  However, for
integration purposes, it can be handy to coerce libnbd into sending
something to see how the server will react (whether it be an extension
libnbd has not yet learned, or an intentional bad request to test
server error handling).  Time to make this something the user can
control, by adding a new strictness mode.  Later patches will add
more knobs to the mode.
---
 lib/internal.h   |   3 +
 generator/API.ml | 165 ++++++++++++++++++++++++++++++++++++-----------
 lib/handle.c     |  16 +++++
 lib/rw.c         | 134 +++++++++++++++++++++-----------------
 tests/errors.c   |  21 +++++-
 5 files changed, 240 insertions(+), 99 deletions(-)

diff --git a/lib/internal.h b/lib/internal.h
index 96699b5..2a5147f 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -148,6 +148,9 @@ struct nbd_handle {
   bool debug;
   nbd_debug_callback debug_callback;

+  /* How strict to be. */
+  uint32_t strict;
+
   /* State machine.
    *
    * The actual current state is ‘state’.  ‘public_state’ is updated
diff --git a/generator/API.ml b/generator/API.ml
index b212e95..aa970e6 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -102,6 +102,13 @@ and link  | ExternalLink of string * int
 | URLLink of string

+let strict_call_description = "\n
+By default, libnbd will reject attempts to use this function with
+parameters that are likely to result in server failure, such as
+requesting an unknown command flag.  The L<nbd_set_strict_mode(3)>
+function can be used to alter which scenarios should await a server
+reply rather than failing fast."
+
 let non_blocking_test_call_description = "\n
 This call does not block, because it returns data that is saved in
 the handle from the NBD protocol handshake."
@@ -171,7 +178,13 @@ let handshake_flags = {
   flags = [
     "FIXED_NEWSTYLE", 1 lsl 0;
     "NO_ZEROES",      1 lsl 1;
-    ]
+  ]
+}
+let strict_flags = {
+  flag_prefix = "STRICT";
+  flags = [
+    "COMMANDS",       1 lsl 0;
+  ]
 }
 let allow_transport_flags = {
   flag_prefix = "ALLOW_TRANSPORT";
@@ -187,8 +200,8 @@ let shutdown_flags = {
     "IMMEDIATE", 1 lsl 1;
   ]
 }
-let all_flags = [ cmd_flags; handshake_flags; allow_transport_flags;
-                  shutdown_flags ]
+let all_flags = [ cmd_flags; handshake_flags; strict_flags;
+                  allow_transport_flags; shutdown_flags ]

 let default_call = { args = []; optargs = []; ret = RErr;
                      shortdesc = ""; longdesc = ""; example
= None;
@@ -451,7 +464,7 @@ test whether this is the case with
L<nbd_supports_tls(3)>.";

   "get_tls", {
     default_call with
-    args = []; ret = REnum (tls_enum);
+    args = []; ret = REnum tls_enum;
     may_set_error = false;
     shortdesc = "get the TLS request setting";
     longdesc = "\
@@ -610,7 +623,7 @@ for integration testing, it can be useful to clear this flag
 rather than find a way to alter the server to fail the negotiation
 request.";
     see_also = [Link "get_request_structured_replies";
-                Link "set_handshake_flags";
+                Link "set_handshake_flags"; Link
"set_strict_mode";
                 Link "get_structured_replies_negotiated";
                 Link "can_meta_context"; Link "can_df"];
   };
@@ -692,7 +705,7 @@ blindly setting a constant value.";

   "get_handshake_flags", {
     default_call with
-    args = []; ret = RFlags (handshake_flags);
+    args = []; ret = RFlags handshake_flags;
     may_set_error = false;
     shortdesc = "see which handshake flags are supported";
     longdesc = "\
@@ -706,10 +719,62 @@ protocol defines new handshake flags, then the return
value from
 a newer library version may include bits that were undefined at
 the time of compilation.";
     see_also = [Link "set_handshake_flags";
-                Link "get_protocol";
+                Link "get_protocol"; Link
"set_strict_mode";
                 Link "aio_is_created"; Link
"aio_is_ready"];
   };

+  "set_strict_mode", {
+    default_call with
+    args = [ Flags ("flags", strict_flags) ]; ret = RErr;
+    shortdesc = "control how strictly to follow NBD protocol";
+    longdesc = "\
+By default, libnbd tries to detect requests that would trigger
+undefined behavior in the NBD protocol, and rejects them client
+side without causing any network traffic, rather than risking
+undefined server behavior.  However, for integration testing, it
+can be handy to relax the strictness of libnbd, to coerce it into
+sending such requests over the network for testing the robustness
+of the server in dealing with such traffic.
+
+The C<flags> argument is a bitmask, including zero or more of the
+following strictness flags:
+
+=over 4
+
+=item C<LIBNBD_STRICT_COMMANDS> = 1
+
+If set, this flag rejects client requests that do not comply with the
+set of advertised server flags (for example, attempting a write on
+a read-only server, or attempting to use C<LIBNBD_CMD_FLAG_FUA> when
+L<nbd_can_fua(3)> returned false).  If clear, this flag relies on the
+server to reject unexpected commands.
+
+=back
+
+For convenience, the constant C<LIBNBD_STRICT_MASK> is available to
+describe all strictness flags supported by this build of libnbd.
+Future versions of libnbd may add further flags, which are likely
+to be enabled by default for additional client-side filtering.  As
+such, when attempting to relax only one specific bit while keeping
+remaining checks at the client side, it is wiser to first call
+L<nbd_get_strict_mode(3)> and modify that value, rather than
+blindly setting a constant value.";
+    see_also = [Link "get_strict_mode"; Link
"set_handshake_flags"];
+  };
+
+  "get_strict_mode", {
+    default_call with
+    args = []; ret = RFlags strict_flags;
+    may_set_error = false;
+    shortdesc = "see which strictness flags are in effect";
+    longdesc = "\
+Return flags indicating which protocol strictness items are being
+enforced locally by libnbd rather than the server.  The return value
+from a newer library version may include bits that were undefined at
+the time of compilation.";
+    see_also = [Link "set_strict_mode"];
+  };
+
   "set_opt_mode", {
     default_call with
     args = [Bool "enable"]; ret = RErr;
@@ -1598,15 +1663,18 @@ Issue a write command to the NBD server, writing the
data in
 C<buf> to the range starting at C<offset> and ending at
 C<offset> + C<count> - 1.  NBD can only write all or nothing
 using this call.  The call returns when the command has been
-acknowledged by the server, or there is an error.
+acknowledged by the server, or there is an error.  Note this will
+generally return an error if L<nbd_is_read_only(3)> is true.

 The C<flags> parameter may be C<0> for no flags, or may contain
 C<LIBNBD_CMD_FLAG_FUA> meaning that the server should not
 return until the data has been committed to permanent storage
 (if that is supported - some servers cannot do this, see
-L<nbd_can_fua(3)>).";
+L<nbd_can_fua(3)>)."
+^ strict_call_description;
     see_also = [Link "can_fua"; Link "is_read_only";
-                Link "aio_pwrite"; Link "get_block_size"];
+                Link "aio_pwrite"; Link "get_block_size";
+                Link "set_strict_mode"];
     example = Some "examples/reads-and-writes.c";
   };

@@ -1657,11 +1725,13 @@ A future version of the library may add new
flags.";
 Issue the flush command to the NBD server.  The function should
 return when all write commands which have completed have been
 committed to permanent storage on the server.  Note this will
-return an error if L<nbd_can_flush(3)> is false.
+generally return an error if L<nbd_can_flush(3)> is false.

 The C<flags> parameter must be C<0> for now (it exists for future
NBD
-protocol extensions).";
-    see_also = [Link "can_flush"; Link "aio_flush"];
+protocol extensions)."
+^ strict_call_description;
+    see_also = [Link "can_flush"; Link "aio_flush";
+                Link "set_strict_mode"];
   };

   "trim", {
@@ -1676,15 +1746,17 @@ Issue a trim command to the NBD server, which if
supported
 by the server causes a hole to be punched in the backing
 store starting at C<offset> and ending at C<offset> +
C<count> - 1.
 The call returns when the command has been acknowledged by the server,
-or there is an error.
+or there is an error.  Note this will generally return an error
+if L<nbd_can_trim(3)> is false or L<nbd_is_read_only(3)> is true.

 The C<flags> parameter may be C<0> for no flags, or may contain
 C<LIBNBD_CMD_FLAG_FUA> meaning that the server should not
 return until the data has been committed to permanent storage
 (if that is supported - some servers cannot do this, see
-L<nbd_can_fua(3)>).";
-    see_also = [Link "can_fua"; Link "can_trim";
-                Link "aio_trim"];
+L<nbd_can_fua(3)>)."
+^ strict_call_description;
+    see_also = [Link "can_fua"; Link "can_trim"; Link
"is_read_only";
+                Link "aio_trim"; Link "set_strict_mode"];
   };

   "cache", {
@@ -1699,12 +1771,14 @@ Issue the cache (prefetch) command to the NBD server,
which
 if supported by the server causes data to be prefetched
 into faster storage by the server, speeding up a subsequent
 L<nbd_pread(3)> call.  The server can also silently ignore
-this command.  Note this will return an error if
+this command.  Note this will generally return an error if
 L<nbd_can_cache(3)> is false.

 The C<flags> parameter must be C<0> for now (it exists for future
NBD
-protocol extensions).";
-    see_also = [Link "can_cache"; Link "aio_cache"];
+protocol extensions)."
+^ strict_call_description;
+    see_also = [Link "can_cache"; Link "aio_cache";
+                Link "set_strict_mode"];
   };

   "zero", {
@@ -1720,7 +1794,8 @@ Issue a write zeroes command to the NBD server, which if
supported
 by the server causes a zeroes to be written efficiently
 starting at C<offset> and ending at C<offset> + C<count> - 1.
 The call returns when the command has been acknowledged by the server,
-or there is an error.
+or there is an error.  Note this will generally return an error if
+L<nbd_can_zero(3)> is false or L<nbd_is_read_only(3)> is true.

 The C<flags> parameter may be C<0> for no flags, or may contain
 C<LIBNBD_CMD_FLAG_FUA> meaning that the server should not
@@ -1731,9 +1806,11 @@ the server should favor writing actual allocated zeroes
over
 punching a hole, and/or C<LIBNBD_CMD_FLAG_FAST_ZERO> meaning
 that the server must fail quickly if writing zeroes is no
 faster than a normal write (if that is supported - some servers
-cannot do this, see L<nbd_can_fast_zero(3)>).";
-    see_also = [Link "can_fua"; Link "can_zero";
-                Link "can_fast_zero"; Link "aio_zero"];
+cannot do this, see L<nbd_can_fast_zero(3)>)."
+^ strict_call_description;
+    see_also = [Link "can_fua"; Link "can_zero"; Link
"is_read_only";
+                Link "can_fast_zero"; Link "aio_zero";
+                Link "set_strict_mode"];
   };

   "block_status", {
@@ -1792,9 +1869,10 @@ The C<flags> parameter may be C<0> for no
flags, or may contain
 C<LIBNBD_CMD_FLAG_REQ_ONE> meaning that the server should
 return only one extent per metadata context where that extent
 does not exceed C<count> bytes; however, libnbd does not
-validate that the server obeyed the flag.";
+validate that the server obeyed the flag."
+^ strict_call_description;
     see_also = [Link "add_meta_context"; Link
"can_meta_context";
-                Link "aio_block_status"];
+                Link "aio_block_status"; Link
"set_strict_mode"];
   };

   "poll", {
@@ -2088,9 +2166,10 @@ Or supply the optional C<completion_callback> which
will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

 Note that you must ensure C<buf> is valid until the command has
-completed.  Other parameters behave as documented in
L<nbd_pwrite(3)>.";
+completed.  Other parameters behave as documented in
L<nbd_pwrite(3)>."
+^ strict_call_description;
     see_also = [SectionLink "Issuing asynchronous commands";
-                Link "is_read_only"; Link "pwrite"];
+                Link "is_read_only"; Link "pwrite"; Link
"set_strict_mode"];
   };

   "aio_disconnect", {
@@ -2132,9 +2211,10 @@ To check if the command completed, call
L<nbd_aio_command_completed(3)>.
 Or supply the optional C<completion_callback> which will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

-Other parameters behave as documented in L<nbd_flush(3)>.";
+Other parameters behave as documented in L<nbd_flush(3)>."
+^ strict_call_description;
     see_also = [SectionLink "Issuing asynchronous commands";
-                Link "can_flush"; Link "flush"];
+                Link "can_flush"; Link "flush"; Link
"set_strict_mode"];
   };

   "aio_trim", {
@@ -2152,9 +2232,10 @@ To check if the command completed, call
L<nbd_aio_command_completed(3)>.
 Or supply the optional C<completion_callback> which will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

-Other parameters behave as documented in L<nbd_trim(3)>.";
+Other parameters behave as documented in L<nbd_trim(3)>."
+^ strict_call_description;
     see_also = [SectionLink "Issuing asynchronous commands";
-                Link "can_trim"; Link "trim"];
+                Link "can_trim"; Link "trim"; Link
"set_strict_mode"];
   };

   "aio_cache", {
@@ -2172,9 +2253,10 @@ To check if the command completed, call
L<nbd_aio_command_completed(3)>.
 Or supply the optional C<completion_callback> which will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

-Other parameters behave as documented in L<nbd_cache(3)>.";
+Other parameters behave as documented in L<nbd_cache(3)>."
+^ strict_call_description;
     see_also = [SectionLink "Issuing asynchronous commands";
-                Link "can_cache"; Link "cache"];
+                Link "can_cache"; Link "cache"; Link
"set_strict_mode"];
   };

   "aio_zero", {
@@ -2193,10 +2275,11 @@ To check if the command completed, call
L<nbd_aio_command_completed(3)>.
 Or supply the optional C<completion_callback> which will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

-Other parameters behave as documented in L<nbd_zero(3)>.";
+Other parameters behave as documented in L<nbd_zero(3)>."
+^ strict_call_description;
     see_also = [SectionLink "Issuing asynchronous commands";
                 Link "can_zero"; Link "can_fast_zero";
-                Link "zero"];
+                Link "zero"; Link "set_strict_mode"];
   };

   "aio_block_status", {
@@ -2214,9 +2297,11 @@ To check if the command completed, call
L<nbd_aio_command_completed(3)>.
 Or supply the optional C<completion_callback> which will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

-Other parameters behave as documented in L<nbd_block_status(3)>.";
+Other parameters behave as documented in L<nbd_block_status(3)>."
+^ strict_call_description;
     see_also = [SectionLink "Issuing asynchronous commands";
-                Link "can_meta_context"; Link
"block_status"];
+                Link "can_meta_context"; Link
"block_status";
+                Link "set_strict_mode"];
   };

   "aio_get_fd", {
@@ -2663,6 +2748,10 @@ let first_version = [
   "aio_opt_list", (1, 4);
   "aio_opt_info", (1, 4);

+  (* Added in 1.5.x development cycle, will be stable and supported in 1.6. *)
+  "set_strict_mode", (1, 6);
+  "get_strict_mode", (1, 6);
+
   (* These calls are proposed for a future version of libnbd, but
    * have not been added to any released version so far.
   "get_tls_certificates", (1, ??);
diff --git a/lib/handle.c b/lib/handle.c
index 4d26842..2dc1fb5 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -74,6 +74,8 @@ nbd_create (void)
   s = getenv ("LIBNBD_DEBUG");
   h->debug = s && strcmp (s, "1") == 0;

+  h->strict = LIBNBD_STRICT_MASK;
+
   h->public_state = STATE_START;
   h->state = STATE_START;
   h->pid = -1;
@@ -349,6 +351,20 @@ nbd_unlocked_get_handshake_flags (struct nbd_handle *h)
   return h->gflags;
 }

+int
+nbd_unlocked_set_strict_mode (struct nbd_handle *h, uint32_t flags)
+{
+  h->strict = flags;
+  return 0;
+}
+
+/* NB: may_set_error = false. */
+uint32_t
+nbd_unlocked_get_strict_mode (struct nbd_handle *h)
+{
+  return h->strict;
+}
+
 const char *
 nbd_unlocked_get_package_name (struct nbd_handle *h)
 {
diff --git a/lib/rw.c b/lib/rw.c
index 95c002c..f49fe25 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -295,10 +295,12 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h,
void *buf,
   struct command_cb cb = { .fn.chunk = *chunk,
                            .completion = *completion };

-  if ((flags & LIBNBD_CMD_FLAG_DF) != 0 &&
-      nbd_unlocked_can_df (h) != 1) {
-    set_error (EINVAL, "server does not support the DF flag");
-    return -1;
+  if (h->strict & LIBNBD_STRICT_COMMANDS) {
+    if ((flags & LIBNBD_CMD_FLAG_DF) != 0 &&
+        nbd_unlocked_can_df (h) != 1) {
+      set_error (EINVAL, "server does not support the DF flag");
+      return -1;
+    }
   }

   SET_CALLBACK_TO_NULL (*chunk);
@@ -315,15 +317,17 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void
*buf,
 {
   struct command_cb cb = { .completion = *completion };

-  if (nbd_unlocked_is_read_only (h) == 1) {
-    set_error (EPERM, "server does not support write operations");
-    return -1;
-  }
+  if (h->strict & LIBNBD_STRICT_COMMANDS) {
+    if (nbd_unlocked_is_read_only (h) == 1) {
+      set_error (EPERM, "server does not support write operations");
+      return -1;
+    }

-  if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
-      nbd_unlocked_can_fua (h) != 1) {
-    set_error (EINVAL, "server does not support the FUA flag");
-    return -1;
+    if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
+        nbd_unlocked_can_fua (h) != 1) {
+      set_error (EINVAL, "server does not support the FUA flag");
+      return -1;
+    }
   }

   SET_CALLBACK_TO_NULL (*completion);
@@ -338,9 +342,11 @@ nbd_unlocked_aio_flush (struct nbd_handle *h,
 {
   struct command_cb cb = { .completion = *completion };

-  if (nbd_unlocked_can_flush (h) != 1) {
-    set_error (EINVAL, "server does not support flush operations");
-    return -1;
+  if (h->strict & LIBNBD_STRICT_COMMANDS) {
+    if (nbd_unlocked_can_flush (h) != 1) {
+      set_error (EINVAL, "server does not support flush operations");
+      return -1;
+    }
   }

   SET_CALLBACK_TO_NULL (*completion);
@@ -356,19 +362,21 @@ nbd_unlocked_aio_trim (struct nbd_handle *h,
 {
   struct command_cb cb = { .completion = *completion };

-  if (nbd_unlocked_can_trim (h) != 1) {
-    set_error (EINVAL, "server does not support trim operations");
-    return -1;
-  }
-  if (nbd_unlocked_is_read_only (h) == 1) {
-    set_error (EPERM, "server does not support write operations");
-    return -1;
-  }
+  if (h->strict & LIBNBD_STRICT_COMMANDS) {
+    if (nbd_unlocked_can_trim (h) != 1) {
+      set_error (EINVAL, "server does not support trim operations");
+      return -1;
+    }
+    if (nbd_unlocked_is_read_only (h) == 1) {
+      set_error (EPERM, "server does not support write operations");
+      return -1;
+    }

-  if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
-      nbd_unlocked_can_fua (h) != 1) {
-    set_error (EINVAL, "server does not support the FUA flag");
-    return -1;
+    if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
+        nbd_unlocked_can_fua (h) != 1) {
+      set_error (EINVAL, "server does not support the FUA flag");
+      return -1;
+    }
   }

   if (count == 0) {             /* NBD protocol forbids this. */
@@ -389,13 +397,15 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,
 {
   struct command_cb cb = { .completion = *completion };

-  /* Actually according to the NBD protocol document, servers do exist
-   * that support NBD_CMD_CACHE but don't advertise the
-   * NBD_FLAG_SEND_CACHE bit, but we ignore those.
-   */
-  if (nbd_unlocked_can_cache (h) != 1) {
-    set_error (EINVAL, "server does not support cache operations");
-    return -1;
+  if (h->strict & LIBNBD_STRICT_COMMANDS) {
+    /* Actually according to the NBD protocol document, servers do exist
+     * that support NBD_CMD_CACHE but don't advertise the
+     * NBD_FLAG_SEND_CACHE bit, but we ignore those.
+     */
+    if (nbd_unlocked_can_cache (h) != 1) {
+      set_error (EINVAL, "server does not support cache operations");
+      return -1;
+    }
   }

   SET_CALLBACK_TO_NULL (*completion);
@@ -411,25 +421,27 @@ nbd_unlocked_aio_zero (struct nbd_handle *h,
 {
   struct command_cb cb = { .completion = *completion };

-  if (nbd_unlocked_can_zero (h) != 1) {
-    set_error (EINVAL, "server does not support zero operations");
-    return -1;
-  }
-  if (nbd_unlocked_is_read_only (h) == 1) {
-    set_error (EPERM, "server does not support write operations");
-    return -1;
-  }
+  if (h->strict & LIBNBD_STRICT_COMMANDS) {
+    if (nbd_unlocked_can_zero (h) != 1) {
+      set_error (EINVAL, "server does not support zero operations");
+      return -1;
+    }
+    if (nbd_unlocked_is_read_only (h) == 1) {
+      set_error (EPERM, "server does not support write operations");
+      return -1;
+    }

-  if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
-      nbd_unlocked_can_fua (h) != 1) {
-    set_error (EINVAL, "server does not support the FUA flag");
-    return -1;
-  }
+    if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
+        nbd_unlocked_can_fua (h) != 1) {
+      set_error (EINVAL, "server does not support the FUA flag");
+      return -1;
+    }

-  if ((flags & LIBNBD_CMD_FLAG_FAST_ZERO) != 0 &&
-      nbd_unlocked_can_fast_zero (h) != 1) {
-    set_error (EINVAL, "server does not support the fast zero flag");
-    return -1;
+    if ((flags & LIBNBD_CMD_FLAG_FAST_ZERO) != 0 &&
+        nbd_unlocked_can_fast_zero (h) != 1) {
+      set_error (EINVAL, "server does not support the fast zero
flag");
+      return -1;
+    }
   }

   if (count == 0) {             /* NBD protocol forbids this. */
@@ -452,16 +464,18 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
   struct command_cb cb = { .fn.extent = *extent,
                            .completion = *completion };

-  if (!h->structured_replies) {
-    set_error (ENOTSUP, "server does not support structured
replies");
-    return -1;
-  }
+  if (h->strict & LIBNBD_STRICT_COMMANDS) {
+    if (!h->structured_replies) {
+      set_error (ENOTSUP, "server does not support structured
replies");
+      return -1;
+    }

-  if (h->meta_contexts == NULL) {
-    set_error (ENOTSUP, "did not negotiate any metadata contexts, "
-               "either you did not call nbd_add_meta_context before "
-               "connecting or the server does not support it");
-    return -1;
+    if (h->meta_contexts == NULL) {
+      set_error (ENOTSUP, "did not negotiate any metadata contexts, "
+                 "either you did not call nbd_add_meta_context before
"
+                 "connecting or the server does not support it");
+      return -1;
+    }
   }

   if (count == 0) {             /* NBD protocol forbids this. */
diff --git a/tests/errors.c b/tests/errors.c
index 8166429..0c4151a 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -86,6 +86,7 @@ main (int argc, char *argv[])
    */
   const char *cmd[] = { "nbdkit", "-s",
"--exit-with-parent", "sh",
                         script, NULL };
+  uint32_t strict;

   progname = argv[0];

@@ -269,7 +270,25 @@ main (int argc, char *argv[])
   }
   check (ERANGE, "nbd_aio_pwrite: ");

-  /* Use unadvertised command */
+  /* Use unadvertised command, client-side */
+  strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_COMMANDS;
+  if (nbd_set_strict_mode (nbd, strict) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  if (nbd_trim (nbd, 512, 0, 0) != -1) {
+    fprintf (stderr, "%s: test failed: "
+             "unpermitted nbd_trim did not fail\n",
+             argv[0]);
+    exit (EXIT_FAILURE);
+  }
+  check (EINVAL, "nbd_trim: ");
+  /* Use unadvertised command, server-side */
+  strict &= ~LIBNBD_STRICT_COMMANDS;
+  if (nbd_set_strict_mode (nbd, strict) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
   if (nbd_trim (nbd, 512, 0, 0) != -1) {
     fprintf (stderr, "%s: test failed: "
              "unpermitted nbd_trim did not fail\n",
-- 
2.28.0

The next strict knob: allow the user to pass unknown flags across the
wire (this is different than passing a known flag at the wrong time).

It is interesting to note that NBD only permits 16 bits of flags, but
we have a signature that takes uint32_t; if we wanted, we could pack
libnbd-specific flags in the upper bits that the NBD protocol would
never see.
---
 generator/API.ml  | 46 ++++++++++++++++++++++++++++++++++++++--------
 generator/API.mli |  1 +
 generator/C.ml    |  7 +++++--
 lib/disconnect.c  |  2 +-
 lib/rw.c          |  6 +++---
 tests/errors.c    | 22 ++++++++++++++++++++--
 6 files changed, 68 insertions(+), 16 deletions(-)

diff --git a/generator/API.ml b/generator/API.ml
index aa970e6..4cd425b 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -87,6 +87,7 @@ and enum = {
 }
 and flags = {
   flag_prefix : string;
+  guard : string option;
   flags : (string * int) list
 }
 and permitted_state @@ -165,6 +166,7 @@ let all_enums = [ tls_enum;
block_size_enum ]
 (* Flags. See also Constants below. *)
 let cmd_flags = {
   flag_prefix = "CMD_FLAG";
+  guard = Some "((h->strict & LIBNBD_STRICT_FLAGS) || flags >
UINT16_MAX)";
   flags = [
     "FUA",       1 lsl 0;
     "NO_HOLE",   1 lsl 1;
@@ -175,6 +177,7 @@ let cmd_flags = {
 }
 let handshake_flags = {
   flag_prefix = "HANDSHAKE_FLAG";
+  guard = None;
   flags = [
     "FIXED_NEWSTYLE", 1 lsl 0;
     "NO_ZEROES",      1 lsl 1;
@@ -182,12 +185,15 @@ let handshake_flags = {
 }
 let strict_flags = {
   flag_prefix = "STRICT";
+  guard = None;
   flags = [
     "COMMANDS",       1 lsl 0;
+    "FLAGS",          1 lsl 1;
   ]
 }
 let allow_transport_flags = {
   flag_prefix = "ALLOW_TRANSPORT";
+  guard = None;
   flags = [
     "TCP",   1 lsl 0;
     "UNIX",  1 lsl 1;
@@ -196,6 +202,7 @@ let allow_transport_flags = {
 }
 let shutdown_flags = {
   flag_prefix = "SHUTDOWN";
+  guard = None;
   flags = [
     "IMMEDIATE", 1 lsl 1;
   ]
@@ -749,6 +756,22 @@ a read-only server, or attempting to use
C<LIBNBD_CMD_FLAG_FUA> when
 L<nbd_can_fua(3)> returned false).  If clear, this flag relies on the
 server to reject unexpected commands.

+=item C<LIBNBD_STRICT_FLAGS> = 2
+
+If set, this flag rejects client requests that attempt to set a
+command flag not recognized by libnbd (those outside of
+C<LIBNBD_CMD_FLAG_MASK>).  If clear, this flag passes on unknown
+flags to the server.  One possible reason to relax this strictness
+knob is to send C<LIBNBD_CMD_FLAG_FUA> on a read command (libnbd
+normally prevents that, but the NBD protocol allows it to succeed);
+however, it is also possible that sending an unknown flag may
+cause the server to change its reply in a manner that confuses
+libnbd, perhaps causing deadlock or ending the connection.
+
+Note that the NBD protocol only supports 16 bits of command flags,
+even though the libnbd API uses C<uint32_t>; bits outside of the
+range permitted by the protocol are always a client-side error.
+
 =back

 For convenience, the constant C<LIBNBD_STRICT_MASK> is available to
@@ -1568,9 +1591,10 @@ required into the server's replies, or if you want to
use
 C<LIBNBD_CMD_FLAG_DF>.

 The C<flags> parameter must be C<0> for now (it exists for future
NBD
-protocol extensions).";
+protocol extensions)."
+^ strict_call_description;
     see_also = [Link "aio_pread"; Link "pread_structured";
-                Link "get_block_size"];
+                Link "get_block_size"; Link
"set_strict_mode"];
     example = Some "examples/fetch-first-sector.c";
   };

@@ -1646,9 +1670,11 @@ The C<flags> parameter may be C<0> for no
flags, or may contain
 C<LIBNBD_CMD_FLAG_DF> meaning that the server should not reply with
 more than one fragment (if that is supported - some servers cannot do
 this, see L<nbd_can_df(3)>). Libnbd does not validate that the server
-actually obeys the flag.";
+actually obeys the flag."
+^ strict_call_description;
     see_also = [Link "can_df"; Link "pread";
-                Link "aio_pread_structured"; Link
"get_block_size"];
+                Link "aio_pread_structured"; Link
"get_block_size";
+                Link "set_strict_mode"];
   };

   "pwrite", {
@@ -2123,10 +2149,12 @@ Or supply the optional C<completion_callback>
which will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

 Note that you must ensure C<buf> is valid until the command has
-completed.  Other parameters behave as documented in
L<nbd_pread(3)>.";
+completed.  Other parameters behave as documented in
L<nbd_pread(3)>."
+^ strict_call_description;
     example = Some "examples/aio-connect-read.c";
     see_also = [SectionLink "Issuing asynchronous commands";
-                Link "aio_pread_structured"; Link "pread"];
+                Link "aio_pread_structured"; Link "pread";
+                Link "set_strict_mode"];
   };

   "aio_pread_structured", {
@@ -2145,9 +2173,11 @@ To check if the command completed, call
L<nbd_aio_command_completed(3)>.
 Or supply the optional C<completion_callback> which will be invoked
 as described in L<libnbd(3)/Completion callbacks>.

-Other parameters behave as documented in
L<nbd_pread_structured(3)>.";
+Other parameters behave as documented in
L<nbd_pread_structured(3)>."
+^ strict_call_description;
     see_also = [SectionLink "Issuing asynchronous commands";
-                Link "aio_pread"; Link "pread_structured"];
+                Link "aio_pread"; Link "pread_structured";
+                Link "set_strict_mode"];
   };

   "aio_pwrite", {
diff --git a/generator/API.mli b/generator/API.mli
index db978ca..41e09f5 100644
--- a/generator/API.mli
+++ b/generator/API.mli
@@ -100,6 +100,7 @@ and enum = {
 }
 and flags = {
   flag_prefix : string;    (** prefix of each flag name *)
+  guard : string option;   (** additional gating for checking valid flags *)
   flags : (string * int) list (** flag names and their values in C *)
 }
 and permitted_state diff --git a/generator/C.ml b/generator/C.ml
index 86d9c5c..5f68b14 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -494,7 +494,7 @@ let generate_lib_api_c ()      );

     (* Check parameters are valid. *)
-    let print_flags_check n { flag_prefix; flags } subset +    let
print_flags_check n { flag_prefix; flags; guard } subset        let value =
match errcode with
         | Some value -> value
         | None -> assert false in
@@ -508,7 +508,10 @@ let generate_lib_api_c ()             ) flags;
            sprintf "0x%x" !v
         | None -> "LIBNBD_" ^ flag_prefix ^ "_MASK" in
-      pr "  if (unlikely ((%s & ~%s) != 0)) {\n" n mask;
+      let guard = match guard with
+        | Some value -> " && " ^ value
+        | None -> "" in
+      pr "  if (unlikely ((%s & ~%s) != 0)%s) {\n" n mask guard;
       pr "    set_error (EINVAL, \"%%s: invalid value for flag:
0x%%x\",\n";
       pr "               \"%s\", %s);\n" n n;
       pr "    ret = %s;\n" value;
diff --git a/lib/disconnect.c b/lib/disconnect.c
index 9de1e34..f99fbd0 100644
--- a/lib/disconnect.c
+++ b/lib/disconnect.c
@@ -64,7 +64,7 @@ nbd_unlocked_aio_disconnect (struct nbd_handle *h, uint32_t
flags)
 {
   int64_t id;

-  id = nbd_internal_command_common (h, 0, NBD_CMD_DISC, 0, 0, NULL, NULL);
+  id = nbd_internal_command_common (h, flags, NBD_CMD_DISC, 0, 0, NULL, NULL);
   if (id == -1)
     return -1;
   h->disconnect_request = true;
diff --git a/lib/rw.c b/lib/rw.c
index f49fe25..e3e80ad 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -281,7 +281,7 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf,
   struct command_cb cb = { .completion = *completion };

   SET_CALLBACK_TO_NULL (*completion);
-  return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count,
+  return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset, count,
                                       buf, &cb);
 }

@@ -350,7 +350,7 @@ nbd_unlocked_aio_flush (struct nbd_handle *h,
   }

   SET_CALLBACK_TO_NULL (*completion);
-  return nbd_internal_command_common (h, 0, NBD_CMD_FLUSH, 0, 0,
+  return nbd_internal_command_common (h, flags, NBD_CMD_FLUSH, 0, 0,
                                       NULL, &cb);
 }

@@ -409,7 +409,7 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,
   }

   SET_CALLBACK_TO_NULL (*completion);
-  return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count,
+  return nbd_internal_command_common (h, flags, NBD_CMD_CACHE, offset, count,
                                       NULL, &cb);
 }

diff --git a/tests/errors.c b/tests/errors.c
index 0c4151a..0cfcac5 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -84,7 +84,7 @@ main (int argc, char *argv[])
    * which delays responding to writes until a witness file no longer
    * exists.
    */
-  const char *cmd[] = { "nbdkit", "-s",
"--exit-with-parent", "sh",
+  const char *cmd[] = { "nbdkit", "-s", "-v",
"--exit-with-parent", "sh",
                         script, NULL };
   uint32_t strict;

@@ -244,7 +244,25 @@ main (int argc, char *argv[])
   }
   check (EINVAL, "nbd_pread: ");

-  /* Use unknown command flags */
+  /* Use unknown command flags, client-side */
+  strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_FLAGS;
+  if (nbd_set_strict_mode (nbd, strict) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
+    fprintf (stderr, "%s: test failed: "
+             "nbd_pread did not fail with bogus flags\n",
+             argv[0]);
+    exit (EXIT_FAILURE);
+  }
+  check (EINVAL, "nbd_pread: ");
+  /* Use unknown command flags, server-side */
+  strict &= ~(LIBNBD_STRICT_FLAGS | LIBNBD_STRICT_COMMANDS);
+  if (nbd_set_strict_mode (nbd, strict) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
   if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
     fprintf (stderr, "%s: test failed: "
              "nbd_pread did not fail with bogus flags\n",
-- 
2.28.0

The NBD protocol states that a 0-length request is undefined; we were
inconsistent in that we let it through for read, write, and cache, but
blocked it for trim, zero, and block_status.  The NBD protocol also
has documented rules on handling access beyond EOF, but we are
currently wasting traffic to the server when we can give the same
answer ourselves.  Exposing these as two more strictness knobs gives
the user finer-grained control over what behavior they want.
---
 lib/internal.h   |  3 ++-
 generator/API.ml | 14 ++++++++++++++
 lib/disconnect.c |  3 ++-
 lib/rw.c         | 47 +++++++++++++++++++++++------------------------
 tests/errors.c   | 20 +++++++++++++++++++-
 5 files changed, 60 insertions(+), 27 deletions(-)

diff --git a/lib/internal.h b/lib/internal.h
index 2a5147f..cde5dcd 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -432,7 +432,8 @@ extern const char *nbd_internal_name_of_nbd_cmd (uint16_t
type);
 extern int64_t nbd_internal_command_common (struct nbd_handle *h,
                                             uint16_t flags, uint16_t type,
                                             uint64_t offset, uint64_t count,
-                                            void *data, struct command_cb *cb);
+                                            int count_err, void *data,
+                                            struct command_cb *cb);

 /* socket.c */
 struct socket *nbd_internal_socket_create (int fd);
diff --git a/generator/API.ml b/generator/API.ml
index 4cd425b..d3b1d1b 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -189,6 +189,8 @@ let strict_flags = {
   flags = [
     "COMMANDS",       1 lsl 0;
     "FLAGS",          1 lsl 1;
+    "BOUNDS",         1 lsl 2;
+    "ZERO_SIZE",      1 lsl 3;
   ]
 }
 let allow_transport_flags = {
@@ -772,6 +774,18 @@ Note that the NBD protocol only supports 16 bits of command
flags,
 even though the libnbd API uses C<uint32_t>; bits outside of the
 range permitted by the protocol are always a client-side error.

+=item C<LIBNBD_STRICT_BOUNDS> = 3
+
+If set, this flag rejects client requests that would exceed the export
+bounds without sending any traffic to the server.  If clear, this flag
+relies on the server to detect out-of-bounds requests.
+
+=item C<LIBNBD_STRICT_ZERO_SIZE> = 4
+
+If set, this flag rejects client requests with length 0.  If clear,
+this permits zero-length requests to the server, which may produce
+undefined results.
+
 =back

 For convenience, the constant C<LIBNBD_STRICT_MASK> is available to
diff --git a/lib/disconnect.c b/lib/disconnect.c
index f99fbd0..822abc1 100644
--- a/lib/disconnect.c
+++ b/lib/disconnect.c
@@ -64,7 +64,8 @@ nbd_unlocked_aio_disconnect (struct nbd_handle *h, uint32_t
flags)
 {
   int64_t id;

-  id = nbd_internal_command_common (h, flags, NBD_CMD_DISC, 0, 0, NULL, NULL);
+  id = nbd_internal_command_common (h, flags, NBD_CMD_DISC, 0, 0, 0,
+                                    NULL, NULL);
   if (id == -1)
     return -1;
   h->disconnect_request = true;
diff --git a/lib/rw.c b/lib/rw.c
index e3e80ad..5dbc947 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -168,10 +168,11 @@ nbd_unlocked_block_status (struct nbd_handle *h,
   return wait_for_command (h, cookie);
 }

+/* count_err represents the errno to return if bounds check fail */
 int64_t
 nbd_internal_command_common (struct nbd_handle *h,
                              uint16_t flags, uint16_t type,
-                             uint64_t offset, uint64_t count,
+                             uint64_t offset, uint64_t count, int count_err,
                              void *data, struct command_cb *cb)
 {
   struct command *cmd;
@@ -185,6 +186,19 @@ nbd_internal_command_common (struct nbd_handle *h,
       goto err;
   }

+  if (count_err) {
+    if ((h->strict & LIBNBD_STRICT_ZERO_SIZE) && count == 0) {
+      set_error (EINVAL, "count cannot be 0");
+      goto err;
+    }
+
+    if ((h->strict & LIBNBD_STRICT_BOUNDS) &&
+        (offset > h->exportsize || offset + count > h->exportsize))
{
+      set_error (count_err, "request out of bounds");
+      return -1;
+    }
+  }
+
   switch (type) {
     /* Commands which send or receive data are limited to MAX_REQUEST_SIZE. */
   case NBD_CMD_READ:
@@ -282,7 +296,7 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void *buf,

   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset, count,
-                                      buf, &cb);
+                                      EINVAL, buf, &cb);
 }

 int64_t
@@ -306,7 +320,7 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle *h,
void *buf,
   SET_CALLBACK_TO_NULL (*chunk);
   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset, count,
-                                      buf, &cb);
+                                      EINVAL, buf, &cb);
 }

 int64_t
@@ -332,7 +346,7 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void
*buf,

   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_WRITE, offset, count,
-                                      (void *) buf, &cb);
+                                      ENOSPC, (void *) buf, &cb);
 }

 int64_t
@@ -351,7 +365,7 @@ nbd_unlocked_aio_flush (struct nbd_handle *h,

   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_FLUSH, 0, 0,
-                                      NULL, &cb);
+                                      0, NULL, &cb);
 }

 int64_t
@@ -379,14 +393,9 @@ nbd_unlocked_aio_trim (struct nbd_handle *h,
     }
   }

-  if (count == 0) {             /* NBD protocol forbids this. */
-    set_error (EINVAL, "count cannot be 0");
-    return -1;
-  }
-
   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_TRIM, offset, count,
-                                      NULL, &cb);
+                                      ENOSPC, NULL, &cb);
 }

 int64_t
@@ -410,7 +419,7 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,

   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_CACHE, offset, count,
-                                      NULL, &cb);
+                                      EINVAL, NULL, &cb);
 }

 int64_t
@@ -444,14 +453,9 @@ nbd_unlocked_aio_zero (struct nbd_handle *h,
     }
   }

-  if (count == 0) {             /* NBD protocol forbids this. */
-    set_error (EINVAL, "count cannot be 0");
-    return -1;
-  }
-
   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_WRITE_ZEROES, offset,
-                                      count, NULL, &cb);
+                                      count, ENOSPC, NULL, &cb);
 }

 int64_t
@@ -478,13 +482,8 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
     }
   }

-  if (count == 0) {             /* NBD protocol forbids this. */
-    set_error (EINVAL, "count cannot be 0");
-    return -1;
-  }
-
   SET_CALLBACK_TO_NULL (*extent);
   SET_CALLBACK_TO_NULL (*completion);
   return nbd_internal_command_common (h, flags, NBD_CMD_BLOCK_STATUS, offset,
-                                      count, NULL, &cb);
+                                      count, EINVAL, NULL, &cb);
 }
diff --git a/tests/errors.c b/tests/errors.c
index 0cfcac5..74bd17e 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -235,7 +235,25 @@ main (int argc, char *argv[])
   }
   check (EINVAL, "nbd_aio_command_completed: ");

-  /* Read from an invalid offset */
+  /* Read from an invalid offset, client-side */
+  strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_BOUNDS;
+  if (nbd_set_strict_mode (nbd, strict) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
+  if (nbd_pread (nbd, buf, 1, -1, 0) != -1) {
+    fprintf (stderr, "%s: test failed: "
+             "nbd_pread did not fail with bogus offset\n",
+             argv[0]);
+    exit (EXIT_FAILURE);
+  }
+  check (EINVAL, "nbd_pread: ");
+  /* Read from an invalid offset, server-side */
+  strict &= ~LIBNBD_STRICT_BOUNDS;
+  if (nbd_set_strict_mode (nbd, strict) == -1) {
+    fprintf (stderr, "%s\n", nbd_get_error ());
+    exit (EXIT_FAILURE);
+  }
   if (nbd_pread (nbd, buf, 1, -1, 0) != -1) {
     fprintf (stderr, "%s: test failed: "
              "nbd_pread did not fail with bogus offset\n",
-- 
2.28.0

On Fri, Sep 11, 2020 at 04:49:52PM -0500, Eric Blake
wrote:
> Document and make it easier for applications to determine which
> bitmask values were supported at compile time.  Also, generating
> bitmask values in hex tends to be more legible than in decimal, as it
> makes it obvious that the values are intended for bitwise operations.
> And it doesn't hurt that this reduces churn in some of the tests if
> new bits are added.
Yes this is a sensible bit of tidy-up.

ACK.

Rich.
>  docs/libnbd.pod                                     |  7 +++++++
>  generator/API.ml                                    | 13 +++++++++++--
>  generator/C.ml                                      | 13 ++++++++-----
>  generator/GoLang.ml                                 |  8 ++++++--
>  generator/OCaml.ml                                  |  9 +++++++++
>  generator/Python.ml                                 |  5 ++++-
>  lib/handle.c                                        |  5 ++---
>  python/t/110-defaults.py                            |  3 +--
>  python/t/120-set-non-defaults.py                    |  5 ++---
>  ocaml/tests/test_110_defaults.ml                    |  3 +--
>  ocaml/tests/test_120_set_non_defaults.ml            |  3 +--
>  tests/errors.c                                      |  9 +++------
>  .../libnbd/libnbd_110_defaults_test.go              |  2 +-
>  .../libnbd/libnbd_120_set_non_defaults_test.go      |  4 ++--
>  14 files changed, 58 insertions(+), 31 deletions(-)
> 
> diff --git a/docs/libnbd.pod b/docs/libnbd.pod
> index f2ba3bb..d8dffea 100644
> --- a/docs/libnbd.pod
> +++ b/docs/libnbd.pod
> @@ -520,6 +520,13 @@ To get the size of the export in bytes, use
L<nbd_get_size(3)>:
>  You can read and write data from the NBD server using
L<nbd_pread(3)>
>  and L<nbd_pwrite(3)> or their asynchronous equivalents.
> 
> +All data commands support a C<flags> argument (mandatory in C, but
> +optional in languages where it can default to 0).  For convenience,
> +the constant C<LIBNBD_CMD_FLAG_MASK> is defined with the set of
flags
> +currently recognized by libnbd, where future NBD protocol extensions
> +may result in additional flags being supported; but in general,
> +specific data commands only accept a subset of known flags.
> +
>  Some servers also support:
> 
>  =over 4
> diff --git a/generator/API.ml b/generator/API.ml
> index cc1a7ba..42eeac0 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -679,6 +679,8 @@ fewer all-zero padding bytes over the connection.
> 
>  =back
> 
> +For convenience, the constant C<LIBNBD_HANDSHAKE_FLAG_MASK> is
> +available to describe all flags supported by this build of libnbd.
>  Future NBD extensions may add further flags, which in turn may
>  be enabled by default in newer libnbd.  As such, when attempting
>  to disable only one specific bit, it is wiser to first call
> @@ -895,7 +897,11 @@ ORed together:
> 
>  =item C<LIBNBD_ALLOW_TRANSPORT_VSOCK>
> 
> -=back";
> +=back
> +
> +For convenience, the constant C<LIBNBD_ALLOW_TRANSPORT_MASK> is
> +available to describe all transports recognized by this build of
> +libnbd.  A future version of the library may add new flags.";
>      see_also = [Link "connect_uri"; Link
"set_uri_allow_tls"];
>    };
> 
> @@ -1629,7 +1635,10 @@ issuing those commands before informing the server
of the intent
>  to disconnect.
> 
>  =back
> -";
> +
> +For convenience, the constant C<LIBNBD_SHUTDOWN_MASK> is available
> +to describe all shutdown flags recognized by this build of libnbd.
> +A future version of the library may add new flags.";
>      see_also = [Link "close"; Link "aio_disconnect"];
>      example = Some "examples/reads-and-writes.c";
>    };
> diff --git a/generator/C.ml b/generator/C.ml
> index c9f0ff4..4d4958d 100644
> --- a/generator/C.ml
> +++ b/generator/C.ml
> @@ -349,11 +349,15 @@ let generate_include_libnbd_h () >    ) all_enums;
>    List.iter (
>      fun { flag_prefix; flags } ->
> +      let mask = ref 0 in
>        List.iter (
>          fun (flag, i) ->
>            let flag = sprintf "LIBNBD_%s_%s" flag_prefix flag in
> -          pr "#define %-40s %d\n" flag i
> +          pr "#define %-40s 0x%02x\n" flag i;
> +          mask := !mask lor i
>        ) flags;
> +      let flag = sprintf "LIBNBD_%s_MASK" flag_prefix in
> +      pr "#define %-40s 0x%02x\n" flag !mask;
>        pr "\n"
>    ) all_flags;
>    List.iter (
> @@ -490,13 +494,12 @@ let generate_lib_api_c () >      );
> 
>      (* Check parameters are valid. *)
> -    let print_flags_check n { flag_prefix; flags } > +    let
print_flags_check n { flag_prefix } >        let value = match errcode with
>          | Some value -> value
>          | None -> assert false in
> -      let mask = List.fold_left (lor) 0 (List.map snd flags) in
> -      pr "  if (unlikely ((%s & ~%d) != 0)) {\n" n mask;
> -      pr "    set_error (EINVAL, \"%%s: invalid value for flag:
%%d\",\n";
> +      pr "  if (unlikely ((%s & ~LIBNBD_%s_MASK) != 0)) {\n"
n flag_prefix;
> +      pr "    set_error (EINVAL, \"%%s: invalid value for flag:
0x%%x\",\n";
>        pr "               \"%s\", %s);\n" n n;
>        pr "    ret = %s;\n" value;
>        pr "    goto out;\n";
> diff --git a/generator/GoLang.ml b/generator/GoLang.ml
> index 359c7d0..f07b074 100644
> --- a/generator/GoLang.ml
> +++ b/generator/GoLang.ml
> @@ -447,12 +447,16 @@ import (
>  ";
>    List.iter (
>      fun { flag_prefix; flags } ->
> -      pr "type %s uint32\n" (camel_case flag_prefix);
> +      let flag_type = camel_case flag_prefix in
> +      let mask = ref 0 in
> +      pr "type %s uint32\n" flag_type;
>        pr "const (\n";
>        List.iter (
>          fun (flag, v) ->
> -          pr "    %s_%s = %s(%d)\n" flag_prefix flag (camel_case
flag_prefix) v
> +          pr "    %s_%s = %s(0x%02x)\n" flag_prefix flag
flag_type v;
> +          mask := !mask lor v
>        ) flags;
> +      pr "    %s_MASK = %s(0x%02x)\n" flag_prefix flag_type
!mask;
>        pr ")\n";
>        pr "\n"
>    ) all_flags;
> diff --git a/generator/OCaml.ml b/generator/OCaml.ml
> index 4bcd450..43b3679 100644
> --- a/generator/OCaml.ml
> +++ b/generator/OCaml.ml
> @@ -164,6 +164,8 @@ type cookie = int64
>            pr "  | %s\n" flag
>        ) flags;
>        pr "  | UNKNOWN of int\n";
> +      pr "\n";
> +      pr "  val mask : t list\n";
>        pr "end\n";
>        pr "\n"
>    ) all_flags;
> @@ -269,6 +271,13 @@ let () >            pr "  | %s\n" flag
>        ) flags;
>        pr "  | UNKNOWN of int\n";
> +      pr "\n";
> +      pr "  let mask = [\n";
> +      List.iter (
> +        fun (flag, _) ->
> +          pr "    %s;\n" flag
> +      ) flags;
> +      pr "  ]\n";
>        pr "end\n";
>        pr "\n"
>    ) all_flags;
> diff --git a/generator/Python.ml b/generator/Python.ml
> index 71368c6..fd09eae 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -696,11 +696,14 @@ Error.__str__ = _str
>    ) all_enums;
>    List.iter (
>      fun { flag_prefix; flags } ->
> +      let mask = ref 0 in
>        List.iter (
>          fun (flag, i) ->
>            let flag = sprintf "%s_%s" flag_prefix flag in
> -          pr "%s = %d\n" flag i
> +          pr "%s = 0x%02x\n" flag i;
> +          mask := !mask lor i
>        ) flags;
> +      pr "%s_MASK = 0x%02x\n" flag_prefix !mask;
>        pr "\n"
>    ) all_flags;
>    List.iter (fun (n, i) -> pr "%s = %d\n" n i) constants;
> diff --git a/lib/handle.c b/lib/handle.c
> index 7987d59..4d26842 100644
> --- a/lib/handle.c
> +++ b/lib/handle.c
> @@ -65,12 +65,11 @@ nbd_create (void)
>    h->tls_verify_peer = true;
>    h->request_sr = true;
> 
> -  h->uri_allow_transports = (uint32_t) -1;
> +  h->uri_allow_transports = LIBNBD_ALLOW_TRANSPORT_MASK;
>    h->uri_allow_tls = LIBNBD_TLS_ALLOW;
>    h->uri_allow_local_file = false;
> 
> -  h->gflags = (LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE |
> -               LIBNBD_HANDSHAKE_FLAG_NO_ZEROES);
> +  h->gflags = LIBNBD_HANDSHAKE_FLAG_MASK;
> 
>    s = getenv ("LIBNBD_DEBUG");
>    h->debug = s && strcmp (s, "1") == 0;
> diff --git a/python/t/110-defaults.py b/python/t/110-defaults.py
> index 47c1d02..fb961cf 100644
> --- a/python/t/110-defaults.py
> +++ b/python/t/110-defaults.py
> @@ -22,6 +22,5 @@ assert h.get_export_name() == ""
>  assert h.get_full_info() is False
>  assert h.get_tls() == nbd.TLS_DISABLE
>  assert h.get_request_structured_replies() is True
> -assert h.get_handshake_flags() == (nbd.HANDSHAKE_FLAG_FIXED_NEWSTYLE |
> -                                   nbd.HANDSHAKE_FLAG_NO_ZEROES)
> +assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK
>  assert h.get_opt_mode() is False
> diff --git a/python/t/120-set-non-defaults.py
b/python/t/120-set-non-defaults.py
> index 3963021..3da0c23 100644
> --- a/python/t/120-set-non-defaults.py
> +++ b/python/t/120-set-non-defaults.py
> @@ -34,11 +34,10 @@ if h.supports_tls():
>  h.set_request_structured_replies(False)
>  assert h.get_request_structured_replies() is False
>  try:
> -    h.set_handshake_flags(nbd.HANDSHAKE_FLAG_NO_ZEROES << 1)
> +    h.set_handshake_flags(nbd.HANDSHAKE_FLAG_MASK + 1)
>      assert False
>  except nbd.Error:
> -    assert h.get_handshake_flags() == (nbd.HANDSHAKE_FLAG_NO_ZEROES |
> -                                       nbd.HANDSHAKE_FLAG_FIXED_NEWSTYLE)
> +    assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK
>  h.set_handshake_flags(0)
>  assert h.get_handshake_flags() == 0
>  h.set_opt_mode(True)
> diff --git a/ocaml/tests/test_110_defaults.ml
b/ocaml/tests/test_110_defaults.ml
> index 54f2cbc..f5886fc 100644
> --- a/ocaml/tests/test_110_defaults.ml
> +++ b/ocaml/tests/test_110_defaults.ml
> @@ -28,8 +28,7 @@ let () >    let sr =
NBD.get_request_structured_replies nbd in
>    assert (sr = true);
>    let flags = NBD.get_handshake_flags nbd in
> -  assert (flags = [ NBD.HANDSHAKE_FLAG.FIXED_NEWSTYLE;
> -                    NBD.HANDSHAKE_FLAG.NO_ZEROES ]);
> +  assert (flags = NBD.HANDSHAKE_FLAG.mask);
>    let opt = NBD.get_opt_mode nbd in
>    assert (opt = false)
> 
> diff --git a/ocaml/tests/test_120_set_non_defaults.ml
b/ocaml/tests/test_120_set_non_defaults.ml
> index 79fe184..b660e5d 100644
> --- a/ocaml/tests/test_120_set_non_defaults.ml
> +++ b/ocaml/tests/test_120_set_non_defaults.ml
> @@ -46,8 +46,7 @@ let () >    with
>      NBD.Error _ -> ();
>    let flags = NBD.get_handshake_flags nbd in
> -  assert (flags = [ NBD.HANDSHAKE_FLAG.FIXED_NEWSTYLE;
> -                    NBD.HANDSHAKE_FLAG.NO_ZEROES ]);
> +  assert (flags = NBD.HANDSHAKE_FLAG.mask);
>    NBD.set_handshake_flags nbd [];
>    let flags = NBD.get_handshake_flags nbd in
>    assert (flags = []);
> diff --git a/tests/errors.c b/tests/errors.c
> index 906c7da..8166429 100644
> --- a/tests/errors.c
> +++ b/tests/errors.c
> @@ -86,7 +86,6 @@ main (int argc, char *argv[])
>     */
>    const char *cmd[] = { "nbdkit", "-s",
"--exit-with-parent", "sh",
>                          script, NULL };
> -  int i;
> 
>    progname = argv[0];
> 
> @@ -147,15 +146,13 @@ main (int argc, char *argv[])
>    }
> 
>    /* Attempt to set a bitmask with an unknown bit. */
> -  i = LIBNBD_HANDSHAKE_FLAG_NO_ZEROES << 1;
> -  if (nbd_set_handshake_flags (nbd, i) != -1) {
> +  if (nbd_set_handshake_flags (nbd, LIBNBD_HANDSHAKE_FLAG_MASK + 1) != -1)
{
>      fprintf (stderr, "%s: test failed: "
>               "nbd_set_handshake_flags did not reject invalid
bitmask\n",
>               argv[0]);
>      exit (EXIT_FAILURE);
>    }
> -  i = LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE |
LIBNBD_HANDSHAKE_FLAG_NO_ZEROES;
> -  if (nbd_get_handshake_flags (nbd) != i) {
> +  if (nbd_get_handshake_flags (nbd) != LIBNBD_HANDSHAKE_FLAG_MASK) {
>      fprintf (stderr, "%s: test failed: "
>               "nbd_get_handshake_flags not left at default
value\n",
>               argv[0]);
> @@ -247,7 +244,7 @@ main (int argc, char *argv[])
>    check (EINVAL, "nbd_pread: ");
> 
>    /* Use unknown command flags */
> -  if (nbd_pread (nbd, buf, 1, 0, -1) != -1) {
> +  if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
>      fprintf (stderr, "%s: test failed: "
>               "nbd_pread did not fail with bogus flags\n",
>               argv[0]);
> diff --git a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
> index 9af0ed8..9e782ac 100644
> --- a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
> +++ b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
> @@ -63,7 +63,7 @@ func Test110Defaults(t *testing.T) {
>  	if err != nil {
>  		t.Fatalf("could not get handshake flags: %s", err)
>  	}
> -	if flags != HANDSHAKE_FLAG_FIXED_NEWSTYLE | HANDSHAKE_FLAG_NO_ZEROES {
> +	if flags != HANDSHAKE_FLAG_MASK {
>  		t.Fatalf("unexpected handshake flags")
>  	}
> 
> diff --git
a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
> index 5d8cce7..b0b06bd 100644
> --- a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
> +++ b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
> @@ -93,7 +93,7 @@ func Test120SetNonDefaults(t *testing.T) {
>  		t.Fatalf("unexpected structured replies state")
>  	}
> 
> -	err = h.SetHandshakeFlags(HANDSHAKE_FLAG_NO_ZEROES << 1)
> +	err = h.SetHandshakeFlags(HANDSHAKE_FLAG_MASK + 1)
>  	if err == nil {
>  		t.Fatalf("expect failure for out-of-range flags")
>  	}
> @@ -101,7 +101,7 @@ func Test120SetNonDefaults(t *testing.T) {
>  	if err != nil {
>  		t.Fatalf("could not get handshake flags: %s", err)
>  	}
> -	if flags != HANDSHAKE_FLAG_FIXED_NEWSTYLE | HANDSHAKE_FLAG_NO_ZEROES {
> +	if flags != HANDSHAKE_FLAG_MASK {
>  		t.Fatalf("unexpected handshake flags")
>  	}
> 
> -- 
> 2.28.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Fri, Sep 11, 2020 at 04:49:53PM -0500, Eric Blake
wrote:
> Rather than having to open-code the list of accepted command flags in
> the unlocked version of each command, we can store that information in
> the generator to produce the check directly in the public API.
Also a nice simplification through generated code, ACK.

Rich.
>  generator/API.ml    | 53 +++++++++++++++++++++++++++++----------------
>  generator/API.mli   |  3 ++-
>  generator/C.ml      | 26 +++++++++++++++-------
>  generator/GoLang.ml | 10 ++++-----
>  generator/OCaml.ml  |  6 ++---
>  generator/Python.ml | 10 ++++-----
>  lib/disconnect.c    | 10 ---------
>  lib/rw.c            | 45 --------------------------------------
>  8 files changed, 67 insertions(+), 96 deletions(-)
> 
> diff --git a/generator/API.ml b/generator/API.ml
> index 42eeac0..b212e95 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -55,7 +55,7 @@ and arg >  | UInt64 of string
>  and optarg >  | OClosure of closure
> -| OFlags of string * flags
> +| OFlags of string * flags * string list option
>  and ret >  | RBool
>  | RStaticString
> @@ -1485,7 +1485,11 @@ Future NBD extensions may result in additional
C<size_type> values.
>    "pread", {
>      default_call with
>      args = [ BytesOut ("buf", "count"); UInt64
"offset" ];
> -    optargs = [ OFlags ("flags", cmd_flags) ];
> +    (* We could silently accept flag DF, but it really only makes sense
> +     * with callbacks, because otherwise there is no observable change
> +     * except that the server may fail where it would otherwise succeed.
> +     *)
> +    optargs = [ OFlags ("flags", cmd_flags, Some []) ];
>      ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "read from the NBD server";
> @@ -1509,7 +1513,7 @@ protocol extensions).";
>      default_call with
>      args = [ BytesOut ("buf", "count"); UInt64
"offset";
>               Closure chunk_closure ];
> -    optargs = [ OFlags ("flags", cmd_flags) ];
> +    optargs = [ OFlags ("flags", cmd_flags, Some
["DF"]) ];
>      ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "read from the NBD server";
> @@ -1585,7 +1589,7 @@ actually obeys the flag.";
>    "pwrite", {
>      default_call with
>      args = [ BytesIn ("buf", "count"); UInt64
"offset" ];
> -    optargs = [ OFlags ("flags", cmd_flags) ];
> +    optargs = [ OFlags ("flags", cmd_flags, Some
["FUA"]) ];
>      ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "write to the NBD server";
> @@ -1608,7 +1612,8 @@ L<nbd_can_fua(3)>).";
> 
>    "shutdown", {
>      default_call with
> -    args = []; optargs = [ OFlags ("flags", shutdown_flags) ];
ret = RErr;
> +    args = []; optargs = [ OFlags ("flags", shutdown_flags,
None) ];
> +    ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "disconnect from the NBD server";
>      longdesc = "\
> @@ -1645,7 +1650,7 @@ A future version of the library may add new
flags.";
> 
>    "flush", {
>      default_call with
> -    args = []; optargs = [ OFlags ("flags", cmd_flags) ]; ret =
RErr;
> +    args = []; optargs = [ OFlags ("flags", cmd_flags, Some [])
]; ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "send flush command to the NBD server";
>      longdesc = "\
> @@ -1662,7 +1667,7 @@ protocol extensions).";
>    "trim", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset" ];
> -    optargs = [ OFlags ("flags", cmd_flags) ];
> +    optargs = [ OFlags ("flags", cmd_flags, Some
["FUA"]) ];
>      ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "send trim command to the NBD server";
> @@ -1685,7 +1690,7 @@ L<nbd_can_fua(3)>).";
>    "cache", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset" ];
> -    optargs = [ OFlags ("flags", cmd_flags) ];
> +    optargs = [ OFlags ("flags", cmd_flags, Some []) ];
>      ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "send cache (prefetch) command to the NBD
server";
> @@ -1705,7 +1710,8 @@ protocol extensions).";
>    "zero", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset" ];
> -    optargs = [ OFlags ("flags", cmd_flags) ];
> +    optargs = [ OFlags ("flags", cmd_flags,
> +                        Some ["FUA"; "NO_HOLE";
"FAST_ZERO"]) ];
>      ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "send write zeroes command to the NBD server";
> @@ -1733,7 +1739,7 @@ cannot do this, see
L<nbd_can_fast_zero(3)>).";
>    "block_status", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset"; Closure
extent_closure ];
> -    optargs = [ OFlags ("flags", cmd_flags) ];
> +    optargs = [ OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
>      ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "send block status command to the NBD server";
> @@ -2026,7 +2032,8 @@ callback.";
>    "aio_pread", {
>      default_call with
>      args = [ BytesPersistOut ("buf", "count"); UInt64
"offset" ];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags, Some []) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "read from the NBD server";
> @@ -2048,7 +2055,8 @@ completed.  Other parameters behave as documented in
L<nbd_pread(3)>.";
>      default_call with
>      args = [ BytesPersistOut ("buf", "count"); UInt64
"offset";
>               Closure chunk_closure ];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags, Some
["DF"]) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "read from the NBD server";
> @@ -2067,7 +2075,8 @@ Other parameters behave as documented in
L<nbd_pread_structured(3)>.";
>    "aio_pwrite", {
>      default_call with
>      args = [ BytesPersistIn ("buf", "count"); UInt64
"offset" ];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags, Some
["FUA"]) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "write to the NBD server";
> @@ -2086,7 +2095,7 @@ completed.  Other parameters behave as documented in
L<nbd_pwrite(3)>.";
> 
>    "aio_disconnect", {
>      default_call with
> -    args = []; optargs = [ OFlags ("flags", cmd_flags) ]; ret =
RErr;
> +    args = []; optargs = [ OFlags ("flags", cmd_flags, Some [])
]; ret = RErr;
>      permitted_states = [ Connected ];
>      shortdesc = "disconnect from the NBD server";
>      longdesc = "\
> @@ -2111,7 +2120,8 @@ however, L<nbd_shutdown(3)> will call this
function if appropriate.";
>    "aio_flush", {
>      default_call with
>      args = [];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags, Some []) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "send flush command to the NBD server";
> @@ -2130,7 +2140,8 @@ Other parameters behave as documented in
L<nbd_flush(3)>.";
>    "aio_trim", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset" ];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags, Some
["FUA"]) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "send trim command to the NBD server";
> @@ -2149,7 +2160,8 @@ Other parameters behave as documented in
L<nbd_trim(3)>.";
>    "aio_cache", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset" ];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags, Some []) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "send cache (prefetch) command to the NBD
server";
> @@ -2168,7 +2180,9 @@ Other parameters behave as documented in
L<nbd_cache(3)>.";
>    "aio_zero", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset" ];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags,
> +                        Some ["FUA"; "NO_HOLE";
"FAST_ZERO"]) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "send write zeroes command to the NBD server";
> @@ -2188,7 +2202,8 @@ Other parameters behave as documented in
L<nbd_zero(3)>.";
>    "aio_block_status", {
>      default_call with
>      args = [ UInt64 "count"; UInt64 "offset"; Closure
extent_closure ];
> -    optargs = [ OClosure completion_closure; OFlags ("flags",
cmd_flags) ];
> +    optargs = [ OClosure completion_closure;
> +                OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
>      ret = RCookie;
>      permitted_states = [ Connected ];
>      shortdesc = "send block status command to the NBD server";
> diff --git a/generator/API.mli b/generator/API.mli
> index e45b5c0..db978ca 100644
> --- a/generator/API.mli
> +++ b/generator/API.mli
> @@ -65,7 +65,8 @@ and arg >  | UInt64 of string         (** 64 bit
unsigned int *)
>  and optarg >  | OClosure of closure      (** optional closure *)
> -| OFlags of string * flags (** optional flags, uint32_t in C *)
> +| OFlags of string * flags * string list option (** optional flags,
uint32_t
> +                                                    in C, and valid subset
*)
>  and ret >  | RBool                    (** return a boolean, or error *)
>  | RStaticString            (** return a static string (must be located in
> diff --git a/generator/C.ml b/generator/C.ml
> index 4d4958d..86d9c5c 100644
> --- a/generator/C.ml
> +++ b/generator/C.ml
> @@ -191,7 +191,7 @@ and print_arg_list' ?(handle = false) ?(types =
true) ?(closure_style = Direct)
>             | AddressOf -> "&"
>             | Pointer -> "*" in
>           pr "%s%s_callback" mark cbname
> -      | OFlags (n, _) ->
> +      | OFlags (n, _, _) ->
>           if types then pr "uint32_t ";
>           pr "%s" n
>    ) optargs
> @@ -494,11 +494,21 @@ let generate_lib_api_c () >      );
> 
>      (* Check parameters are valid. *)
> -    let print_flags_check n { flag_prefix } > +    let
print_flags_check n { flag_prefix; flags } subset >        let value = match
errcode with
>          | Some value -> value
>          | None -> assert false in
> -      pr "  if (unlikely ((%s & ~LIBNBD_%s_MASK) != 0)) {\n"
n flag_prefix;
> +      let mask = match subset with
> +        | Some [] -> "0"
> +        | Some subset ->
> +           let v = ref 0 in
> +           List.iter (
> +             fun (flag, i) ->
> +               if List.mem flag subset then v := !v lor i
> +           ) flags;
> +           sprintf "0x%x" !v
> +        | None -> "LIBNBD_" ^ flag_prefix ^ "_MASK"
in
> +      pr "  if (unlikely ((%s & ~%s) != 0)) {\n" n mask;
>        pr "    set_error (EINVAL, \"%%s: invalid value for flag:
0x%%x\",\n";
>        pr "               \"%s\", %s);\n" n n;
>        pr "    ret = %s;\n" value;
> @@ -536,7 +546,7 @@ let generate_lib_api_c () >           pr " 
}\n";
>           need_out_label := true
>        | Flags (n, flags) ->
> -         print_flags_check n flags
> +         print_flags_check n flags None
>        | String n ->
>           let value = match errcode with
>             | Some value -> value
> @@ -552,8 +562,8 @@ let generate_lib_api_c () >      List.iter (
>        function
>        | OClosure _ -> ()
> -      | OFlags (n, flags) ->
> -         print_flags_check n flags
> +      | OFlags (n, flags, subset) ->
> +         print_flags_check n flags subset
>      ) optargs;
> 
>      (* Make the call. *)
> @@ -635,7 +645,7 @@ let generate_lib_api_c () >      List.iter (
>        function
>        | OClosure { cbname } -> pr " %s=%%s" cbname
> -      | OFlags (n, _) -> pr " %s=0x%%x" n
> +      | OFlags (n, _, _) -> pr " %s=0x%%x" n
>      ) optargs;
>      pr "\"";
>      List.iter (
> @@ -660,7 +670,7 @@ let generate_lib_api_c () >        function
>        | OClosure { cbname } ->
>           pr ", CALLBACK_IS_NULL (%s_callback) ?
\"<fun>\" : \"NULL\"" cbname
> -      | OFlags (n, _) -> pr ", %s" n
> +      | OFlags (n, _, _) -> pr ", %s" n
>      ) optargs;
>      pr ");\n";
>      List.iter (
> diff --git a/generator/GoLang.ml b/generator/GoLang.ml
> index f07b074..81446a6 100644
> --- a/generator/GoLang.ml
> +++ b/generator/GoLang.ml
> @@ -83,7 +83,7 @@ let go_arg_type = function
> 
>  let go_name_of_optarg = function
>    | OClosure { cbname } -> sprintf "%sCallback" (camel_case
cbname)
> -  | OFlags (n, _) -> String.capitalize_ascii n
> +  | OFlags (n, _, _) -> String.capitalize_ascii n
> 
>  let go_ret_type = function
>    (* RErr returns only the error, with no return value. *)
> @@ -202,7 +202,7 @@ let print_binding (name, { args; optargs; ret;
shortdesc }) >          pr "  %s " fname;
>          (match optarg with
>           | OClosure { cbname } -> pr "%sCallback" (camel_case
cbname)
> -         | OFlags (_, {flag_prefix}) -> pr "%s" (camel_case
flag_prefix)
> +         | OFlags (_, {flag_prefix}, _) -> pr "%s"
(camel_case flag_prefix)
>          );
>          pr "\n"
>      ) optargs;
> @@ -298,7 +298,7 @@ let print_binding (name, { args; optargs; ret;
shortdesc }) >        function
>        | OClosure { cbname } -> pr "    var c_%s
C.nbd_%s_callback\n"
>                                   cbname cbname
> -      | OFlags (n, _) -> pr "    var c_%s C.uint32_t\n" n
> +      | OFlags (n, _, _) -> pr "    var c_%s C.uint32_t\n" n
>      ) optargs;
>      pr "    if optargs != nil {\n";
>      List.iter (
> @@ -312,7 +312,7 @@ let print_binding (name, { args; optargs; ret;
shortdesc }) >                 cbname cbname;
>               pr "            c_%s.user_data = unsafe.Pointer
(C.long_to_vp (C.long (registerCallbackId (optargs.%s))))\n"
>                 cbname (go_name_of_optarg optarg)
> -          | OFlags (n, _) ->
> +          | OFlags (n, _, _) ->
>               pr "            c_%s = C.uint32_t (optargs.%s)\n"
>                 n (go_name_of_optarg optarg);
>           );
> @@ -346,7 +346,7 @@ let print_binding (name, { args; optargs; ret;
shortdesc }) >    List.iter (
>      function
>      | OClosure { cbname} -> pr ", c_%s" cbname
> -    | OFlags (n, _) -> pr ", c_%s" n
> +    | OFlags (n, _, _) -> pr ", c_%s" n
>    ) optargs;
>    pr ")\n";
> 
> diff --git a/generator/OCaml.ml b/generator/OCaml.ml
> index 43b3679..28acb50 100644
> --- a/generator/OCaml.ml
> +++ b/generator/OCaml.ml
> @@ -72,7 +72,7 @@ and ocaml_ret_to_string = function
>  and ocaml_optarg_to_string = function
>    | OClosure { cbname; cbargs } ->
>       sprintf "?%s:(%s)" cbname (ocaml_closuredecl_to_string
cbargs)
> -  | OFlags (n, { flag_prefix }) -> sprintf "?%s:%s.t list" n
flag_prefix
> +  | OFlags (n, { flag_prefix }, _) -> sprintf "?%s:%s.t list"
n flag_prefix
> 
>  and ocaml_closuredecl_to_string cbargs >    let cbargs = List.map
ocaml_cbarg_to_string cbargs in
> @@ -112,7 +112,7 @@ let ocaml_name_of_arg = function
> 
>  let ocaml_name_of_optarg = function
>    | OClosure { cbname } -> cbname
> -  | OFlags (n, _) -> n
> +  | OFlags (n, _, _) -> n
> 
>  let num_params args optargs >    List.length optargs + 1 (* handle *) +
List.length args
> @@ -614,7 +614,7 @@ let print_ocaml_binding (name, { args; optargs; ret })
>         pr "  }\n";
>         pr "  %s_callback.user_data = %s_user_data;\n" cbname
cbname;
>         pr "  %s_callback.free = free_user_data;\n" cbname;
> -    | OFlags (n, { flag_prefix }) ->
> +    | OFlags (n, { flag_prefix }, _) ->
>         pr "  uint32_t %s;\n" n;
>         pr "  if (%sv != Val_int (0)) /* Some [ list of %s.t ]
*/\n"
>           n flag_prefix;
> diff --git a/generator/Python.ml b/generator/Python.ml
> index fd09eae..1705ad9 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -320,7 +320,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error } >         pr "  nbd_%s_callback %s = { .callback =
%s_wrapper,\n"
>           cbname cbname cbname;
>         pr "                         .free = free_user_data };\n"
> -    | OFlags (n, _) ->
> +    | OFlags (n, _, _) ->
>         pr "  uint32_t %s_u32;\n" n;
>         pr "  unsigned int %s; /* really uint32_t */\n" n
>    ) optargs;
> @@ -378,7 +378,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error } >    List.iter (
>      function
>      | OClosure { cbname } -> pr ", &py_%s_fn" cbname
> -    | OFlags (n, _) -> pr ", &%s" n
> +    | OFlags (n, _, _) -> pr ", &%s" n
>    ) optargs;
>    pr "))\n";
>    pr "    goto out;\n";
> @@ -402,7 +402,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error } >         pr "  }\n";
>         pr "  else\n";
>         pr "    %s.callback = NULL; /* we're not going to call it
*/\n" cbname
> -    | OFlags (n, _) -> pr "  %s_u32 = %s;\n" n n
> +    | OFlags (n, _, _) -> pr "  %s_u32 = %s;\n" n n
>    ) optargs;
>    List.iter (
>      function
> @@ -472,7 +472,7 @@ let print_python_binding name { args; optargs; ret;
may_set_error } >    List.iter (
>      function
>      | OClosure { cbname } -> pr ", %s" cbname
> -    | OFlags (n, _) -> pr ", %s_u32" n
> +    | OFlags (n, _, _) -> pr ", %s_u32" n
>    ) optargs;
>    pr ");\n";
>    List.iter (
> @@ -802,7 +802,7 @@ class NBD(object):
>          List.map (
>            function
>            | OClosure { cbname } -> cbname, Some "None", None
> -          | OFlags (n, _) -> n, Some "0", None
> +          | OFlags (n, _, _) -> n, Some "0", None
>          ) optargs in
>        let args = args @ optargs in
>        pr "    def %s(" name;
> diff --git a/lib/disconnect.c b/lib/disconnect.c
> index b8356b7..9de1e34 100644
> --- a/lib/disconnect.c
> +++ b/lib/disconnect.c
> @@ -30,11 +30,6 @@
>  int
>  nbd_unlocked_shutdown (struct nbd_handle *h, uint32_t flags)
>  {
> -  if ((flags & ~LIBNBD_SHUTDOWN_IMMEDIATE) != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    /* If IMMEDIATE, abort any commands that have not yet had any bytes
>     * sent to the server, so that NBD_CMD_DISC will be first in line.
>     */
> @@ -69,11 +64,6 @@ nbd_unlocked_aio_disconnect (struct nbd_handle *h,
uint32_t flags)
>  {
>    int64_t id;
> 
> -  if (flags != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    id = nbd_internal_command_common (h, 0, NBD_CMD_DISC, 0, 0, NULL, NULL);
>    if (id == -1)
>      return -1;
> diff --git a/lib/rw.c b/lib/rw.c
> index f3adb71..95c002c 100644
> --- a/lib/rw.c
> +++ b/lib/rw.c
> @@ -280,15 +280,6 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void
*buf,
>  {
>    struct command_cb cb = { .completion = *completion };
> 
> -  /* We could silently accept flag DF, but it really only makes sense
> -   * with callbacks, because otherwise there is no observable change
> -   * except that the server may fail where it would otherwise succeed.
> -   */
> -  if (flags != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    SET_CALLBACK_TO_NULL (*completion);
>    return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count,
>                                        buf, &cb);
> @@ -304,11 +295,6 @@ nbd_unlocked_aio_pread_structured (struct nbd_handle
*h, void *buf,
>    struct command_cb cb = { .fn.chunk = *chunk,
>                             .completion = *completion };
> 
> -  if ((flags & ~LIBNBD_CMD_FLAG_DF) != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    if ((flags & LIBNBD_CMD_FLAG_DF) != 0 &&
>        nbd_unlocked_can_df (h) != 1) {
>      set_error (EINVAL, "server does not support the DF flag");
> @@ -334,11 +320,6 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const
void *buf,
>      return -1;
>    }
> 
> -  if ((flags & ~LIBNBD_CMD_FLAG_FUA) != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
>        nbd_unlocked_can_fua (h) != 1) {
>      set_error (EINVAL, "server does not support the FUA flag");
> @@ -362,11 +343,6 @@ nbd_unlocked_aio_flush (struct nbd_handle *h,
>      return -1;
>    }
> 
> -  if (flags != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    SET_CALLBACK_TO_NULL (*completion);
>    return nbd_internal_command_common (h, 0, NBD_CMD_FLUSH, 0, 0,
>                                        NULL, &cb);
> @@ -389,11 +365,6 @@ nbd_unlocked_aio_trim (struct nbd_handle *h,
>      return -1;
>    }
> 
> -  if ((flags & ~LIBNBD_CMD_FLAG_FUA) != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
>        nbd_unlocked_can_fua (h) != 1) {
>      set_error (EINVAL, "server does not support the FUA flag");
> @@ -427,11 +398,6 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,
>      return -1;
>    }
> 
> -  if (flags != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    SET_CALLBACK_TO_NULL (*completion);
>    return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count,
>                                        NULL, &cb);
> @@ -454,12 +420,6 @@ nbd_unlocked_aio_zero (struct nbd_handle *h,
>      return -1;
>    }
> 
> -  if ((flags & ~(LIBNBD_CMD_FLAG_FUA | LIBNBD_CMD_FLAG_NO_HOLE |
> -                 LIBNBD_CMD_FLAG_FAST_ZERO)) != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    if ((flags & LIBNBD_CMD_FLAG_FUA) != 0 &&
>        nbd_unlocked_can_fua (h) != 1) {
>      set_error (EINVAL, "server does not support the FUA flag");
> @@ -504,11 +464,6 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
>      return -1;
>    }
> 
> -  if ((flags & ~LIBNBD_CMD_FLAG_REQ_ONE) != 0) {
> -    set_error (EINVAL, "invalid flag: %" PRIu32, flags);
> -    return -1;
> -  }
> -
>    if (count == 0) {             /* NBD protocol forbids this. */
>      set_error (EINVAL, "count cannot be 0");
>      return -1;
> -- 
> 2.28.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Fri, Sep 11, 2020 at 04:49:54PM -0500, Eric Blake
wrote:
> @@ -451,7 +464,7 @@ test whether this is the case with
L<nbd_supports_tls(3)>.";
> 
>    "get_tls", {
>      default_call with
> -    args = []; ret = REnum (tls_enum);
> +    args = []; ret = REnum tls_enum;
>      may_set_error = false;
>      shortdesc = "get the TLS request setting";
>      longdesc = "\
> @@ -692,7 +705,7 @@ blindly setting a constant value.";
> 
>    "get_handshake_flags", {
>      default_call with
> -    args = []; ret = RFlags (handshake_flags);
> +    args = []; ret = RFlags handshake_flags;
>      may_set_error = false;
>      shortdesc = "see which handshake flags are supported";
>      longdesc = "\
You might want to put these two hunks in a separate commit since I
believe it's a clean up and so could be a candidate for cherry-picking
to the stable branch.

- - -

I looked over the rest of this patch and it all looks sensible and a
nice new feature, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Fri, Sep 11, 2020 at 04:49:55PM -0500, Eric Blake
wrote:
> The next strict knob: allow the user to pass unknown flags across the
> wire (this is different than passing a known flag at the wrong time).
> 
> It is interesting to note that NBD only permits 16 bits of flags, but
> we have a signature that takes uint32_t; if we wanted, we could pack
> libnbd-specific flags in the upper bits that the NBD protocol would
> never see.
This is fine ACK.

About the "guard" change: I probably would have put that change in a
separate commit.  Also you could consider having a default_flags
parameter to avoid having to set guard = None everywhere, although you
would still have to make a one-off change to every *_flags.  It would
look like this:

  let default_flags = { flag_prefix = ""; guard = None; flags = [] }
  ...
  let handshake_flags = {
    default_flags with
      flag_prefix = "HANDSHAKE_FLAG";
      flags = [ ... ]
  }

(We already do this with "default_call").  Neither of these is a big
deal
though.

Rich.
>  generator/API.ml  | 46 ++++++++++++++++++++++++++++++++++++++--------
>  generator/API.mli |  1 +
>  generator/C.ml    |  7 +++++--
>  lib/disconnect.c  |  2 +-
>  lib/rw.c          |  6 +++---
>  tests/errors.c    | 22 ++++++++++++++++++++--
>  6 files changed, 68 insertions(+), 16 deletions(-)
> 
> diff --git a/generator/API.ml b/generator/API.ml
> index aa970e6..4cd425b 100644
> --- a/generator/API.ml
> +++ b/generator/API.ml
> @@ -87,6 +87,7 @@ and enum = {
>  }
>  and flags = {
>    flag_prefix : string;
> +  guard : string option;
>    flags : (string * int) list
>  }
>  and permitted_state > @@ -165,6 +166,7 @@ let all_enums = [ tls_enum;
block_size_enum ]
>  (* Flags. See also Constants below. *)
>  let cmd_flags = {
>    flag_prefix = "CMD_FLAG";
> +  guard = Some "((h->strict & LIBNBD_STRICT_FLAGS) || flags
> UINT16_MAX)";
>    flags = [
>      "FUA",       1 lsl 0;
>      "NO_HOLE",   1 lsl 1;
> @@ -175,6 +177,7 @@ let cmd_flags = {
>  }
>  let handshake_flags = {
>    flag_prefix = "HANDSHAKE_FLAG";
> +  guard = None;
>    flags = [
>      "FIXED_NEWSTYLE", 1 lsl 0;
>      "NO_ZEROES",      1 lsl 1;
> @@ -182,12 +185,15 @@ let handshake_flags = {
>  }
>  let strict_flags = {
>    flag_prefix = "STRICT";
> +  guard = None;
>    flags = [
>      "COMMANDS",       1 lsl 0;
> +    "FLAGS",          1 lsl 1;
>    ]
>  }
>  let allow_transport_flags = {
>    flag_prefix = "ALLOW_TRANSPORT";
> +  guard = None;
>    flags = [
>      "TCP",   1 lsl 0;
>      "UNIX",  1 lsl 1;
> @@ -196,6 +202,7 @@ let allow_transport_flags = {
>  }
>  let shutdown_flags = {
>    flag_prefix = "SHUTDOWN";
> +  guard = None;
>    flags = [
>      "IMMEDIATE", 1 lsl 1;
>    ]
> @@ -749,6 +756,22 @@ a read-only server, or attempting to use
C<LIBNBD_CMD_FLAG_FUA> when
>  L<nbd_can_fua(3)> returned false).  If clear, this flag relies on
the
>  server to reject unexpected commands.
> 
> +=item C<LIBNBD_STRICT_FLAGS> = 2
> +
> +If set, this flag rejects client requests that attempt to set a
> +command flag not recognized by libnbd (those outside of
> +C<LIBNBD_CMD_FLAG_MASK>).  If clear, this flag passes on unknown
> +flags to the server.  One possible reason to relax this strictness
> +knob is to send C<LIBNBD_CMD_FLAG_FUA> on a read command (libnbd
> +normally prevents that, but the NBD protocol allows it to succeed);
> +however, it is also possible that sending an unknown flag may
> +cause the server to change its reply in a manner that confuses
> +libnbd, perhaps causing deadlock or ending the connection.
> +
> +Note that the NBD protocol only supports 16 bits of command flags,
> +even though the libnbd API uses C<uint32_t>; bits outside of the
> +range permitted by the protocol are always a client-side error.
> +
>  =back
> 
>  For convenience, the constant C<LIBNBD_STRICT_MASK> is available to
> @@ -1568,9 +1591,10 @@ required into the server's replies, or if you
want to use
>  C<LIBNBD_CMD_FLAG_DF>.
> 
>  The C<flags> parameter must be C<0> for now (it exists for
future NBD
> -protocol extensions).";
> +protocol extensions)."
> +^ strict_call_description;
>      see_also = [Link "aio_pread"; Link
"pread_structured";
> -                Link "get_block_size"];
> +                Link "get_block_size"; Link
"set_strict_mode"];
>      example = Some "examples/fetch-first-sector.c";
>    };
> 
> @@ -1646,9 +1670,11 @@ The C<flags> parameter may be C<0> for
no flags, or may contain
>  C<LIBNBD_CMD_FLAG_DF> meaning that the server should not reply with
>  more than one fragment (if that is supported - some servers cannot do
>  this, see L<nbd_can_df(3)>). Libnbd does not validate that the
server
> -actually obeys the flag.";
> +actually obeys the flag."
> +^ strict_call_description;
>      see_also = [Link "can_df"; Link "pread";
> -                Link "aio_pread_structured"; Link
"get_block_size"];
> +                Link "aio_pread_structured"; Link
"get_block_size";
> +                Link "set_strict_mode"];
>    };
> 
>    "pwrite", {
> @@ -2123,10 +2149,12 @@ Or supply the optional C<completion_callback>
which will be invoked
>  as described in L<libnbd(3)/Completion callbacks>.
> 
>  Note that you must ensure C<buf> is valid until the command has
> -completed.  Other parameters behave as documented in
L<nbd_pread(3)>.";
> +completed.  Other parameters behave as documented in
L<nbd_pread(3)>."
> +^ strict_call_description;
>      example = Some "examples/aio-connect-read.c";
>      see_also = [SectionLink "Issuing asynchronous commands";
> -                Link "aio_pread_structured"; Link
"pread"];
> +                Link "aio_pread_structured"; Link
"pread";
> +                Link "set_strict_mode"];
>    };
> 
>    "aio_pread_structured", {
> @@ -2145,9 +2173,11 @@ To check if the command completed, call
L<nbd_aio_command_completed(3)>.
>  Or supply the optional C<completion_callback> which will be invoked
>  as described in L<libnbd(3)/Completion callbacks>.
> 
> -Other parameters behave as documented in
L<nbd_pread_structured(3)>.";
> +Other parameters behave as documented in
L<nbd_pread_structured(3)>."
> +^ strict_call_description;
>      see_also = [SectionLink "Issuing asynchronous commands";
> -                Link "aio_pread"; Link
"pread_structured"];
> +                Link "aio_pread"; Link
"pread_structured";
> +                Link "set_strict_mode"];
>    };
> 
>    "aio_pwrite", {
> diff --git a/generator/API.mli b/generator/API.mli
> index db978ca..41e09f5 100644
> --- a/generator/API.mli
> +++ b/generator/API.mli
> @@ -100,6 +100,7 @@ and enum = {
>  }
>  and flags = {
>    flag_prefix : string;    (** prefix of each flag name *)
> +  guard : string option;   (** additional gating for checking valid flags
*)
>    flags : (string * int) list (** flag names and their values in C *)
>  }
>  and permitted_state > diff --git a/generator/C.ml b/generator/C.ml
> index 86d9c5c..5f68b14 100644
> --- a/generator/C.ml
> +++ b/generator/C.ml
> @@ -494,7 +494,7 @@ let generate_lib_api_c () >      );
> 
>      (* Check parameters are valid. *)
> -    let print_flags_check n { flag_prefix; flags } subset > +    let
print_flags_check n { flag_prefix; flags; guard } subset >        let value =
match errcode with
>          | Some value -> value
>          | None -> assert false in
> @@ -508,7 +508,10 @@ let generate_lib_api_c () >             ) flags;
>             sprintf "0x%x" !v
>          | None -> "LIBNBD_" ^ flag_prefix ^ "_MASK"
in
> -      pr "  if (unlikely ((%s & ~%s) != 0)) {\n" n mask;
> +      let guard = match guard with
> +        | Some value -> " && " ^ value
> +        | None -> "" in
> +      pr "  if (unlikely ((%s & ~%s) != 0)%s) {\n" n mask
guard;
>        pr "    set_error (EINVAL, \"%%s: invalid value for flag:
0x%%x\",\n";
>        pr "               \"%s\", %s);\n" n n;
>        pr "    ret = %s;\n" value;
> diff --git a/lib/disconnect.c b/lib/disconnect.c
> index 9de1e34..f99fbd0 100644
> --- a/lib/disconnect.c
> +++ b/lib/disconnect.c
> @@ -64,7 +64,7 @@ nbd_unlocked_aio_disconnect (struct nbd_handle *h,
uint32_t flags)
>  {
>    int64_t id;
> 
> -  id = nbd_internal_command_common (h, 0, NBD_CMD_DISC, 0, 0, NULL, NULL);
> +  id = nbd_internal_command_common (h, flags, NBD_CMD_DISC, 0, 0, NULL,
NULL);
>    if (id == -1)
>      return -1;
>    h->disconnect_request = true;
> diff --git a/lib/rw.c b/lib/rw.c
> index f49fe25..e3e80ad 100644
> --- a/lib/rw.c
> +++ b/lib/rw.c
> @@ -281,7 +281,7 @@ nbd_unlocked_aio_pread (struct nbd_handle *h, void
*buf,
>    struct command_cb cb = { .completion = *completion };
> 
>    SET_CALLBACK_TO_NULL (*completion);
> -  return nbd_internal_command_common (h, 0, NBD_CMD_READ, offset, count,
> +  return nbd_internal_command_common (h, flags, NBD_CMD_READ, offset,
count,
>                                        buf, &cb);
>  }
> 
> @@ -350,7 +350,7 @@ nbd_unlocked_aio_flush (struct nbd_handle *h,
>    }
> 
>    SET_CALLBACK_TO_NULL (*completion);
> -  return nbd_internal_command_common (h, 0, NBD_CMD_FLUSH, 0, 0,
> +  return nbd_internal_command_common (h, flags, NBD_CMD_FLUSH, 0, 0,
>                                        NULL, &cb);
>  }
> 
> @@ -409,7 +409,7 @@ nbd_unlocked_aio_cache (struct nbd_handle *h,
>    }
> 
>    SET_CALLBACK_TO_NULL (*completion);
> -  return nbd_internal_command_common (h, 0, NBD_CMD_CACHE, offset, count,
> +  return nbd_internal_command_common (h, flags, NBD_CMD_CACHE, offset,
count,
>                                        NULL, &cb);
>  }
> 
> diff --git a/tests/errors.c b/tests/errors.c
> index 0c4151a..0cfcac5 100644
> --- a/tests/errors.c
> +++ b/tests/errors.c
> @@ -84,7 +84,7 @@ main (int argc, char *argv[])
>     * which delays responding to writes until a witness file no longer
>     * exists.
>     */
> -  const char *cmd[] = { "nbdkit", "-s",
"--exit-with-parent", "sh",
> +  const char *cmd[] = { "nbdkit", "-s",
"-v", "--exit-with-parent", "sh",
>                          script, NULL };
>    uint32_t strict;
> 
> @@ -244,7 +244,25 @@ main (int argc, char *argv[])
>    }
>    check (EINVAL, "nbd_pread: ");
> 
> -  /* Use unknown command flags */
> +  /* Use unknown command flags, client-side */
> +  strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_FLAGS;
> +  if (nbd_set_strict_mode (nbd, strict) == -1) {
> +    fprintf (stderr, "%s\n", nbd_get_error ());
> +    exit (EXIT_FAILURE);
> +  }
> +  if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
> +    fprintf (stderr, "%s: test failed: "
> +             "nbd_pread did not fail with bogus flags\n",
> +             argv[0]);
> +    exit (EXIT_FAILURE);
> +  }
> +  check (EINVAL, "nbd_pread: ");
> +  /* Use unknown command flags, server-side */
> +  strict &= ~(LIBNBD_STRICT_FLAGS | LIBNBD_STRICT_COMMANDS);
> +  if (nbd_set_strict_mode (nbd, strict) == -1) {
> +    fprintf (stderr, "%s\n", nbd_get_error ());
> +    exit (EXIT_FAILURE);
> +  }
>    if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) {
>      fprintf (stderr, "%s: test failed: "
>               "nbd_pread did not fail with bogus flags\n",
> -- 
> 2.28.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
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

ACK series, 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