[Libguestfs] [libnbd PATCH v2 1/5] api: Add xxx_MASK constant for each Flags type
Richard W.M. Jones
rjones at redhat.com
Thu Sep 17 13:23:30 UTC 2020
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 at 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/
More information about the Libguestfs
mailing list