Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 00/25] enable 64-bit extensions
Continuation of my v4 patches which started here: https://listman.redhat.com/archives/libguestfs/2023-July/032077.html and compared to my v3 patches here: https://listman.redhat.com/archives/libguestfs/2023-May/031617.html Most of the differences from the earlier version is in the front end: Laszlo had some good suggestions about reworking how 64-bit server replies are handled without doing an in-place widening or narrowing, and without introducing a callback shim. I also split up the OCaml patches to focus on one language binding at a time, instead of all at once. Many of the later patches are unchanged because the API addition is still the same, all that differs was how it was implemented internally. Also new at the end is an additional strictness flag that can be cleared to intentionally send or omit the PAYLOAD_LEN flag for the purposes of server integration testing. 001/25:[down] 'block_status: Add some sanity checking of server lengths' 002/25:[down] 'generator: Add Extent64 arg type for upcoming use' 003/25:[down] 'generator: Support Extent64 arg in C code' 004/25:[down] 'generator: Support Extent64 arg in Python code' 005/25:[down] 'golang: Change logic of copy_uint32_array' 006/25:[down] 'generator: Support Extent64 arg in Go code' 007/25:[down] 'generator: Support Extent64 arg in OCaml code' 008/25:[0214] [FC] 'block_status: Accept 64-bit extents during block status' 009/25:[down] 'generator: Prepare for extent64 callback' 010/25:[0387] [FC] 'api: Add [aio_]nbd_block_status_64' 011/25:[down] 'api: Add tests for [aio_]nbd_block_status_64' 012/25:[----] [--] 'api: Add several functions for controlling extended headers' 013/25:[----] [--] 'copy: Update nbdcopy to use 64-bit block status' 014/25:[----] [--] 'dump: Update nbddump to use 64-bit block status' 015/25:[down] 'info: Add --has alias for --can' 016/25:[0016] [FC] 'info: Expose extended-headers support through nbdinfo' 017/25:[----] [--] 'info: Update nbdinfo --map to use 64-bit block status' 018/25:[----] [--] 'examples: Update copy-libev to use 64-bit block status' 019/25:[----] [--] 'ocaml: Add example for 64-bit extents' 020/25:[----] [--] 'generator: Actually request extended headers' 021/25:[----] [--] 'api: Add nbd_[aio_]opt_extended_headers()' 022/25:[0012] [FC] 'interop: Add test of 64-bit block status' 023/25:[0040] [FC] 'api: Add nbd_can_block_status_payload()' 024/25:[0010] [FC] 'api: Add nbd_[aio_]block_status_filter()' 025/25:[down] 'api: Add LIBNBD_STRICT_AUTO_FLAG control to nbd_set_strict' Eric Blake (25): block_status: Add some sanity checking of server lengths generator: Add Extent64 arg type for upcoming use generator: Support Extent64 arg in C code generator: Support Extent64 arg in Python code golang: Change logic of copy_uint32_array generator: Support Extent64 arg in Go code generator: Support Extent64 arg in OCaml code block_status: Accept 64-bit extents during block status generator: Prepare for extent64 callback api: Add [aio_]nbd_block_status_64 api: Add tests for [aio_]nbd_block_status_64 api: Add several functions for controlling extended headers copy: Update nbdcopy to use 64-bit block status dump: Update nbddump to use 64-bit block status info: Add --has alias for --can info: Expose extended-headers support through nbdinfo info: Update nbdinfo --map to use 64-bit block status examples: Update copy-libev to use 64-bit block status ocaml: Add example for 64-bit extents generator: Actually request extended headers api: Add nbd_[aio_]opt_extended_headers() interop: Add test of 64-bit block status api: Add nbd_can_block_status_payload() api: Add nbd_[aio_]block_status_filter() api: Add LIBNBD_STRICT_AUTO_FLAG control to nbd_set_strict docs/libnbd.pod | 18 +- info/nbdinfo.pod | 46 +- sh/nbdsh.pod | 2 +- lib/internal.h | 26 +- lib/nbd-protocol.h | 7 + generator/API.mli | 1 + generator/API.ml | 542 +++++++++++++++--- generator/state_machine.ml | 41 ++ generator/states-newstyle.c | 3 + .../states-newstyle-opt-extended-headers.c | 110 ++++ generator/states-newstyle-opt-starttls.c | 7 +- .../states-newstyle-opt-structured-reply.c | 3 +- generator/states-issue-command.c | 4 +- generator/states-reply-chunk.c | 213 +++++-- generator/C.ml | 19 + generator/GoLang.ml | 40 +- generator/Makefile.am | 1 + generator/OCaml.ml | 20 +- generator/Python.ml | 23 +- lib/aio.c | 12 +- lib/flags.c | 12 + lib/handle.c | 26 +- lib/opt.c | 44 ++ lib/rw.c | 240 +++++++- python/t/110-defaults.py | 1 + python/t/120-set-non-defaults.py | 2 + python/t/465-block-status-64.py | 56 ++ ocaml/examples/Makefile.am | 1 + ocaml/examples/extents64.ml | 42 ++ ocaml/helpers.c | 21 + ocaml/nbd-c.h | 1 + ocaml/tests/Makefile.am | 1 + ocaml/tests/test_110_defaults.ml | 2 + ocaml/tests/test_120_set_non_defaults.ml | 3 + ocaml/tests/test_465_block_status_64.ml | 58 ++ tests/meta-base-allocation.c | 104 +++- examples/copy-libev.c | 21 +- examples/server-flags.c | 7 +- interop/Makefile.am | 18 + interop/block-status-64.c | 186 ++++++ interop/block-status-64.sh | 49 ++ interop/block-status-payload.c | 241 ++++++++ interop/block-status-payload.sh | 80 +++ interop/opt-extended-headers.c | 153 +++++ interop/opt-extended-headers.sh | 29 + .gitignore | 3 + copy/nbd-ops.c | 22 +- dump/dump.c | 27 +- fuzzing/libnbd-fuzz-wrapper.c | 20 +- golang/Makefile.am | 1 + golang/handle.go | 6 + golang/libnbd_110_defaults_test.go | 8 + golang/libnbd_120_set_non_defaults_test.go | 12 + golang/libnbd_465_block_status_64_test.go | 119 ++++ info/can.c | 16 +- info/info-can.sh | 36 +- info/info-packets.sh | 17 +- info/main.c | 11 +- info/map.c | 65 ++- info/show.c | 9 +- 60 files changed, 2632 insertions(+), 276 deletions(-) create mode 100644 generator/states-newstyle-opt-extended-headers.c create mode 100644 python/t/465-block-status-64.py create mode 100644 ocaml/examples/extents64.ml create mode 100644 ocaml/tests/test_465_block_status_64.ml create mode 100644 interop/block-status-64.c create mode 100755 interop/block-status-64.sh create mode 100644 interop/block-status-payload.c create mode 100755 interop/block-status-payload.sh create mode 100644 interop/opt-extended-headers.c create mode 100755 interop/opt-extended-headers.sh create mode 100644 golang/libnbd_465_block_status_64_test.go base-commit: 70329e9585297bc42cf3db3bf508263137dade8d -- 2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 01/25] block_status: Add some sanity checking of server lengths
Previously, we had not been doing any validation of server extent
responses, which means a client query at an offset near the end of the
export can result in a buggy server sending a response longer than the
export length and potentially confusing the client. The NBD spec also
says that an extent length should be non-zero so that a successful
block status call makes progress. It is easy enough to track that the
server has not overflowed the export size, and that we ensure an error
on no progress even when the buggy server claims success. Since the
spec says a client should be prepared for a block status result to be
truncated, the client should not care whether the truncation happened
at the server or at libnbd after validating the server's response.
In the process, this patch reorganizes some of the code so that early
exits are obvious, leading for less indentation in the success path.
Adding this sanity checking now makes it easier for future patches to
do orthogonal support for a server's 32- or 64-bit reply, vs. a
client's 32- or 64-bit API call. Once 64-bit replies are in play, we
will additionally have to worry about a 64-bit reply that overflows a
32-bit API callback without exceeding the exportsize. Similarly,
since nbd_get_size() already caps export size at 63 bits (based on
off_t limitations), we have guaranteed that a 64-bit API callback will
never see an extent length that could appear negative in a 64-bit
signed type (at least OCaml benefits from that guarantee, since its
only native 64-bit integer type is signed).
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: new patch
---
generator/states-reply-chunk.c | 78 +++++++++++++++++++++++++---------
1 file changed, 58 insertions(+), 20 deletions(-)
diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 17bb5149..735f9456 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -461,6 +461,11 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
struct command *cmd = h->reply_cmd;
size_t i;
uint32_t context_id;
+ int error;
+ const char *name;
+ uint32_t orig_len, len, flags;
+ uint64_t total, cap;
+ bool stop;
switch (recv_into_rbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
@@ -481,30 +486,63 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
if (context_id == h->meta_contexts.ptr[i].context_id)
break;
- if (i < h->meta_contexts.len) {
- int error = cmd->error;
- const char *name = h->meta_contexts.ptr[i].name;
-
- /* Need to byte-swap the entries returned, but apart from that
- * we don't validate them. Yes, our 32-bit public API foolishly
- * tracks the number of uint32_t instead of block descriptors;
- * see _block_desc_is_multiple_of_bs_entry above.
- */
- for (i = 0; i < h->bs_count * 2; ++i)
- h->bs_entries[i] = be32toh (h->bs_entries[i]);
-
- /* Call the caller's extent function. */
- if (CALL_CALLBACK (cmd->cb.fn.extent, name, cmd->offset,
- h->bs_entries, h->bs_count * 2, &error) ==
-1)
- if (cmd->error == 0)
- cmd->error = error ? error : EPROTO;
- }
- else
+ SET_NEXT_STATE (%FINISH);
+ if (i == h->meta_contexts.len) {
/* Emit a debug message, but ignore it. */
debug (h, "server sent unexpected meta context ID %" PRIu32,
context_id);
+ break;
+ }
- SET_NEXT_STATE (%FINISH);
+ name = h->meta_contexts.ptr[i].name;
+ total = 0;
+ cap = h->exportsize - cmd->offset;
+ assert (cap <= h->exportsize);
+
+ /* Need to byte-swap the entries returned. The NBD protocol
+ * allows truncation as long as progress is made; the client
+ * cannot tell the difference between a server's truncation or if
+ * we truncate on a length we don't like. We stop iterating on a
+ * zero-length extent (error only if it is the first extent), and
+ * on an extent beyond the exportsize (unconditional error after
+ * truncating to exportsize); but do not diagnose issues with the
+ * server's length alignments, flag values, nor compliance with
+ * the REQ_ONE command flag.
+ */
+ for (i = 0, stop = false; i < h->bs_count && !stop; ++i) {
+ orig_len = len = be32toh (h->bs_entries[i * 2]);
+ flags = be32toh (h->bs_entries[i * 2 + 1]);
+ total += len;
+ if (len == 0) {
+ stop = true;
+ if (i > 0)
+ break; /* Skip this and later extents; we already made progress */
+ /* Expose this extent as an error; we made no progress */
+ cmd->error = cmd->error ? : EPROTO;
+ }
+ else if (total > cap) {
+ /* Expose this extent as an error, after truncating to make progress */
+ stop = true;
+ cmd->error = cmd->error ? : EPROTO;
+ len -= total - cap;
+ }
+ h->bs_entries[i * 2] = len;
+ h->bs_entries[i * 2 + 1] = flags;
+ }
+
+ /* Call the caller's extent function. Yes, our 32-bit public API
+ * foolishly tracks the number of uint32_t instead of block
+ * descriptors; see _block_desc_is_multiple_of_bs_entry above.
+ */
+ if (stop)
+ debug (h, "truncating server's response at unexpected extent
length %"
+ PRIu32 " and total %" PRIu64 " near extent
%zu",
+ orig_len, total, i);
+ error = cmd->error;
+ if (CALL_CALLBACK (cmd->cb.fn.extent, name, cmd->offset,
+ h->bs_entries, i * 2, &error) == -1)
+ if (cmd->error == 0)
+ cmd->error = error ? error : EPROTO;
}
return 0;
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 02/25] generator: Add Extent64 arg type for upcoming use
The existing nbd_block_status() callback is permanently stuck with an
array of uint32_t pairs (each of the h->bs_count extents is
represented by a pair of uint32_t; we have to pass bs_count*2 as the
array size to the callback, and the user has to compute len/2 to
determine how many extents to process). Furthermore, the 32-bit
nature of both values adds some constraints: we cannot represent
extents larger than 4G, and status flags must fit in 32 bits. While
the "base:allocation" metacontext will never exceed 32 bits, it is not
hard to envision other future extension metacontexts where a 64-bit
status would be useful (for example, Zoned Block Devices expressing a
64-bit offset[1]).
Exposing 64-bit extents will require a new API; we now have the
decision of whether to copy the existing API practice of returning a
bare array containing h->bs_count*2 raw uint64_t (where the user
continues the same practice of pairing those off into extents), or
going with a saner idea of an array of h->bs_count structs directly
describing extents. Returning an array of structs has an advantage:
although both items in the 64-bit extent are 64-bit values over the
wire, they are conceptually different types (an extent length is
inherently bounded by 63-bit off_t limitations; while the extent flags
can be a full 64 bits of unsigned flags), and this difference in types
can be more precisely represented in non-C languages.
This patch starts the ball rolling by adding a new arg type for use in
the generator, although all match statements that process args are
minimally changed to merely assert that we aren't using the type yet.
Then the next few patches can provide an implementation per language
binding (for ease of review), prior to a final patch that finally adds
the new nbd_block_status_64() API that actually takes advantage of the
new type.
[1] https://zonedstorage.io/docs/linux/zbd-api
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: improve commit message [Laszlo], split into several smaller
patches [Laszlo]
---
generator/API.mli | 1 +
generator/API.ml | 12 +++++++++++-
generator/C.ml | 7 +++++++
generator/GoLang.ml | 4 ++++
generator/OCaml.ml | 4 ++++
generator/Python.ml | 5 +++++
6 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/generator/API.mli b/generator/API.mli
index c5bba8c2..80633018 100644
--- a/generator/API.mli
+++ b/generator/API.mli
@@ -52,6 +52,7 @@ and
| BytesPersistOut of string * string
| Closure of closure (** function pointer + void *opaque *)
| Enum of string * enum (** enum/union type, int in C *)
+| Extent64 of string (** extent descriptor *)
| Fd of string (** file descriptor *)
| Flags of string * flags (** flags, uint32_t in C *)
| Int of string (** small int *)
diff --git a/generator/API.ml b/generator/API.ml
index 72c81657..1fbf147b 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -42,6 +42,7 @@
| BytesPersistOut of string * string
| Closure of closure
| Enum of string * enum
+| Extent64 of string
| Fd of string
| Flags of string * flags
| Int of string
@@ -157,6 +158,14 @@ let extent_closure
"nr_entries");
CBMutable (Int "error") ]
}
+let extent64_closure = {
+ cbname = "extent64";
+ cbargs = [ CBString "metacontext";
+ CBUInt64 "offset";
+ CBArrayAndLen (Extent64 "entries",
+ "nr_entries");
+ CBMutable (Int "error") ]
+}
let list_closure = {
cbname = "list";
cbargs = [ CBString "name"; CBString "description" ]
@@ -166,7 +175,8 @@ let context_closure cbargs = [ CBString "name"
]
}
let all_closures = [ chunk_closure; completion_closure;
- debug_closure; extent_closure; list_closure;
+ debug_closure; extent_closure; (* extent64_closure; *)
+ list_closure;
context_closure ]
(* Enums. *)
diff --git a/generator/C.ml b/generator/C.ml
index a2670f70..606ba1e0 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -94,6 +94,7 @@ let
| Closure { cbname } ->
[ sprintf "%s_callback" cbname; sprintf "%s_user_data"
cbname ]
| Enum (n, _) -> [n]
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd n -> [n]
| Flags (n, _) -> [n]
| Int n -> [n]
@@ -132,6 +133,7 @@ let
| Bool _ | Closure _ | Enum _ | Fd _ | Flags _
| Int _ | Int64 _ | SizeT _
| UInt _ | UInt32 _ | UInt64 _ | UIntPtr _ -> [ false ]
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
let optarg_attr_nonnull (OClosure _ | OFlags _) = [ false ]
@@ -191,6 +193,7 @@ and
| Enum (n, _) ->
if types then pr "int ";
pr "%s" n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags (n, _) ->
if types then pr "uint32_t ";
pr "%s" n
@@ -751,6 +754,7 @@ let
| Bool _ | Closure _ | Enum _ | Flags _ | Fd _ | Int _
| Int64 _ | SizeT _
| SockAddrAndLen _ | UInt _ | UInt32 _ | UInt64 _ | UIntPtr _ -> ()
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
) args;
let indent if may_set_error then (
@@ -774,6 +778,7 @@ let
pr " %s=\\\"%%s\\\" %s=%%zu" n count
| Closure { cbname } -> pr " %s=%%s" cbname
| Enum (n, _) -> pr " %s=%%d" n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags (n, _) -> pr " %s=0x%%x" n
| Fd n | Int n -> pr " %s=%%d" n
| Int64 n -> pr " %s=%%\"PRIi64\"" n
@@ -812,6 +817,7 @@ let
pr "%s_printable ? %s_printable : \"\", %s" n
n count
| Closure _ -> pr "\"<fun>\""
| Enum (n, _) -> pr "%s" n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags (n, _) -> pr "%s" n
| Fd n | Int n | Int64 n | SizeT n -> pr "%s" n
| SockAddrAndLen (_, len) -> pr "(int) %s" len
@@ -847,6 +853,7 @@ let
| Bool _ | Closure _ | Enum _ | Flags _ | Fd _ | Int _
| Int64 _ | SizeT _
| SockAddrAndLen _ | UInt _ | UInt32 _ | UInt64 _ | UIntPtr _ -> ()
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
) args;
pr " }\n"
(* Print the trace when we leave a call with debugging enabled. *)
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 8a19f65a..73df5254 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -38,6 +38,7 @@ let
| BytesPersistOut (n, len) -> n
| Closure { cbname } -> cbname
| Enum (n, _) -> n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd n -> n
| Flags (n, _) -> n
| Int n -> n
@@ -60,6 +61,7 @@ let
| BytesPersistOut _ -> "AioBuffer"
| Closure { cbname } -> sprintf "%sCallback" (camel_case cbname)
| Enum (_, { enum_prefix }) -> camel_case enum_prefix
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd _ -> "int"
| Flags (_, { flag_prefix }) -> camel_case flag_prefix
| Int _ -> "int"
@@ -252,6 +254,7 @@ let
pr " c_%s.user_data = C.alloc_cbid(C.long(%s_cbid))\n"
cbname cbname
| Enum (n, _) ->
pr " c_%s := C.int(%s)\n" n n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd n ->
pr " c_%s := C.int(%s)\n" n n
| Flags (n, _) ->
@@ -324,6 +327,7 @@ let
| BytesPersistOut (n, len) -> pr ", c_%s, c_%s" n len
| Closure { cbname } -> pr ", c_%s" cbname
| Enum (n, _) -> pr ", c_%s" n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd n -> pr ", c_%s" n
| Flags (n, _) -> pr ", c_%s" n
| Int n -> pr ", c_%s" n
diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index efc1af22..7971ac40 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -44,6 +44,7 @@ and
| Closure { cbargs } ->
sprintf "(%s)" (ocaml_closuredecl_to_string cbargs)
| Enum (_, { enum_prefix }) -> sprintf "%s.t" enum_prefix
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd _ -> "Unix.file_descr"
| Flags (_, { flag_prefix }) -> sprintf "%s.t list" flag_prefix
| Int _ -> "int"
@@ -102,6 +103,7 @@ let
| BytesPersistOut (n, len) -> n
| Closure { cbname } -> cbname
| Enum (n, _) -> n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd n -> n
| Flags (n, _) -> n
| Int n -> n
@@ -695,6 +697,7 @@ let
pr " %s_callback.free = free_user_data;\n" cbname
| Enum (n, { enum_prefix }) ->
pr " int %s = %s_val (%sv);\n" n enum_prefix n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Fd n ->
pr " /* OCaml Unix.file_descr is just an int, at least on Unix.
*/\n";
pr " int %s = Int_val (%sv);\n" n n
@@ -792,6 +795,7 @@ let
| UInt32 _
| UInt64 _
| UIntPtr _ -> ()
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
) args;
pr " CAMLreturn (rv);\n";
diff --git a/generator/Python.ml b/generator/Python.ml
index beaeaa4c..3a77b81f 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -307,6 +307,7 @@ let
pr ".callback = %s_wrapper, .free = free_user_data"
cbname);
pr " };\n"
| Enum (n, _) -> pr " int %s;\n" n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags (n, _) ->
pr " uint32_t %s_u32;\n" n;
pr " unsigned int %s; /* really uint32_t */\n" n
@@ -363,6 +364,7 @@ let
"O", sprintf "&%s" n, sprintf
"py_%s->buf, py_%s->len" n n
| Closure { cbname } -> "O", sprintf
"&py_%s_fn" cbname, cbname
| Enum (n, _) -> "i", sprintf "&%s" n, n
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags (n, _) -> "I", sprintf "&%s" n,
sprintf "%s_u32" n
| Fd n | Int n -> "i", sprintf "&%s" n, n
| Int64 n -> "L", sprintf "&%s" n, sprintf
"%s_i64" n
@@ -454,6 +456,7 @@ let
pr " if (!chunk_user_data->view) goto out;\n"
)
| Enum _ -> ()
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags (n, _) -> pr " %s_u32 = %s;\n" n n
| Fd _ | Int _ -> ()
| Int64 n -> pr " %s_i64 = %s;\n" n n
@@ -552,6 +555,7 @@ let
| Closure { cbname } ->
pr " free_user_data (%s_user_data);\n" cbname
| Enum _ -> ()
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags _ -> ()
| Fd _ | Int _ -> ()
| Int64 _ -> ()
@@ -882,6 +886,7 @@ let
| BytesPersistOut (n, _) -> n, None
| Closure { cbname } -> cbname, None
| Enum (n, _) -> n, None
+ | Extent64 _ -> assert false (* only used in extent64_closure *)
| Flags (n, _) -> n, None
| Fd n | Int n -> n, None
| Int64 n -> n, None
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 03/25] generator: Support Extent64 arg in C code
See the earlier commit "Add Extent64 arg type" for rationale in
supporting a new generator arg type. This patch adds the C bindings
for use of Extent64: it exposes a new 'struct nbd_extent' in
<libnbd.h>, as well as the code to generate the C bindings for passing
an extent64_closure once a new API uses the type in an upcoming patch.
Note that 'struct nbd_block_descriptor_64' in lib/nbd-protocol.h is
exactly the same as what we want to use in C. But it is easier to
stick a new public type in <libnbd.h> than to figure out how to expose
just part of a header we only want to use internally, and leaves us
free to alter nbd-protocol.h (which is shared with nbdkit) without
affecting public API.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: split out of larger patch [Laszlo]
---
generator/API.mli | 2 +-
generator/C.ml | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/generator/API.mli b/generator/API.mli
index 80633018..96626c9b 100644
--- a/generator/API.mli
+++ b/generator/API.mli
@@ -52,7 +52,7 @@ and
| BytesPersistOut of string * string
| Closure of closure (** function pointer + void *opaque *)
| Enum of string * enum (** enum/union type, int in C *)
-| Extent64 of string (** extent descriptor *)
+| Extent64 of string (** extent descriptor, struct nbd_extent in C *)
| Fd of string (** file descriptor *)
| Flags of string * flags (** flags, uint32_t in C *)
| Int of string (** small int *)
diff --git a/generator/C.ml b/generator/C.ml
index 606ba1e0..e5a2879b 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -308,6 +308,11 @@ and
pr "%s, " n;
if types then pr "size_t ";
pr "%s" len
+ | CBArrayAndLen (Extent64 n, len) ->
+ if types then pr "nbd_extent *";
+ pr "%s, " n;
+ if types then pr "size_t ";
+ pr "%s" len
| CBArrayAndLen _ -> assert false
| CBBytesIn (n, len) ->
if types then pr "const void *";
@@ -488,6 +493,13 @@ let
pr "extern int nbd_get_errno (void);\n";
pr "#define LIBNBD_HAVE_NBD_GET_ERRNO 1\n";
pr "\n";
+ pr "/* This is used in the callback for nbd_block_status_64.\n";
+ pr " */\n";
+ pr "typedef struct {\n";
+ pr " uint64_t length;\n";
+ pr " uint64_t flags;\n";
+ pr "} nbd_extent;\n";
+ pr "\n";
print_closure_structs ();
List.iter (
fun (name, { args; optargs; ret }) ->
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 04/25] generator: Support Extent64 arg in Python code
See the earlier commit "Add Extent64 arg type" for rationale in
supporting a new generator arg type. This patch adds the Python
bindings for use of Extent64: it is fairly easy to construct a list of
2-item tuples in place with Py_BuildValue's 'K' argument for
unsigned
64-bit values, without needing to write any separate helper code.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: split out of larger patch [Laszlo], use K instead of O to avoid
memory issues while building list
---
generator/Python.ml | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index 3a77b81f..761f4511 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -169,7 +169,7 @@ let
pr " PyObject *py_args, *py_ret;\n";
List.iter (
function
- | CBArrayAndLen (UInt32 n, _)
+ | CBArrayAndLen ((UInt32 n | Extent64 n), _)
| CBBytesIn (n, _)
| CBMutable (Int n) ->
pr " PyObject *py_%s = NULL;\n" n
@@ -187,6 +187,18 @@ let
pr " if (!py_e_%s) { PyErr_PrintEx (0); goto out; }\n" n;
pr " PyList_SET_ITEM (py_%s, i_%s, py_e_%s);\n" n n n;
pr " }\n"
+ | CBArrayAndLen (Extent64 n, len) ->
+ pr " py_%s = PyList_New (%s);\n" n len;
+ pr " if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n;
+ pr " size_t i_%s;\n" n;
+ pr " for (i_%s = 0; i_%s < %s; ++i_%s) {\n" n n len n;
+ pr " PyObject *py_e_%s = Py_BuildValue (" n;
+ pr_wrap ',' (fun () ->
+ pr "\"KK\", %s[i_%s].length, %s[i_%s].flags" n n
n n);
+ pr ");\n";
+ pr " if (!py_e_%s) { PyErr_PrintEx (0); goto out; }\n" n;
+ pr " PyList_SET_ITEM (py_%s, i_%s, py_e_%s);\n" n n n;
+ pr " }\n"
| CBBytesIn (n, len) ->
pr " py_%s = nbd_internal_py_get_subview (data->view, %s,
%s);\n" n n len;
pr " if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n
@@ -205,7 +217,7 @@ let
let params List.map (
function
- | CBArrayAndLen (UInt32 n, _) -> "O", sprintf
"py_%s" n
+ | CBArrayAndLen ((UInt32 n | Extent64 n), _) -> "O", sprintf
"py_%s" n
| CBBytesIn (n, _) -> "O", sprintf "py_%s" n
| CBInt n -> "i", n
| CBInt64 n -> "L", n
@@ -254,7 +266,7 @@ let
pr " out:\n";
List.iter (
function
- | CBArrayAndLen (UInt32 n, _) ->
+ | CBArrayAndLen ((UInt32 n | Extent64 n), _) ->
pr " Py_XDECREF (py_%s);\n" n
| CBBytesIn (n, _) ->
pr " Py_XDECREF (py_%s);\n" n
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 05/25] golang: Change logic of copy_uint32_array
Commit 6725fa0e12 changed copy_uint32_array() to utilize a Go hack for
accessing a C array as a Go slice in order to potentially benefit from
any optimizations in Go's copy() for bulk transfer of memory over
naive one-at-a-time iteration. But that commit also acknowledged that
no benchmark timings were performed, which would have been useful to
demonstrat an actual benefit for using hack in the first place. And
since we are copying data anyways (rather than using the slice to
avoid a copy), and network transmission costs have a higher impact to
performance than in-memory copying speed, it's hard to justify keeping
the hack without hard data.
What's more, while using Go's copy() on an array of C uint32_t makes
sense for 32-bit extents, our corresponding 64-bit code uses a struct
which does not map as nicely to Go's copy(). Using a common style
between both list copying helpers is beneficial to mainenance.
Additionally, at face value, converting C.size_t to int may truncate;
we could avoid that risk if we were to uniformly use uint64 instead of
int. But we can equally just panic if the count is oversized: our
state machine guarantees that the server's response fits within 64M
bytes (count will be smaller than that, since it is multiple bytes per
extent entry).
Suggested-by: Laszlo Ersek <lersek at redhat.com>
CC: Nir Soffer <nsoffer at redhat.com>
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: new patch to the series, but previously posted as part of the
golang cleanups. Since then: rework the commit message as it is no
longer a true revert, and add a panic() if count exceeds expected
bounds.
---
generator/GoLang.ml | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index 73df5254..cc7d78b6 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -516,11 +516,16 @@ let
/* Closures. */
func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
+ if (uint64(count) > 64*1024*1024) {
+ panic(\"violation of state machine guarantee\")
+ }
ret := make([]uint32, int(count))
- // See
https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
- // TODO: Use unsafe.Slice() when we require Go 1.17.
- s := (*[1 << 30]uint32)(unsafe.Pointer(entries))[:count:count]
- copy(ret, s)
+ addr := uintptr(unsafe.Pointer(entries))
+ for i := 0; i < int(count); i++ {
+ ptr := (*C.uint32_t)(unsafe.Pointer(addr))
+ ret[i] = uint32(*ptr)
+ addr += unsafe.Sizeof(*ptr)
+ }
return ret
}
";
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 06/25] generator: Support Extent64 arg in Go code
See the earlier commit "Add Extent64 arg type" for rationale in
supporting a new generator arg type. This patch adds the Go bindings
for use of Extent64, which required adding a counterpart Go type and
introducing a helper function for copying from the C array to an
actual Go object, very similar to the existing copy_uint32_array
helper for 32-bit extents.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: split out of larger patch [Laszlo], avoid awkward slice since we
can't use copy() anyway [Laszlo]
---
generator/GoLang.ml | 23 +++++++++++++++++++++++
golang/handle.go | 6 ++++++
2 files changed, 29 insertions(+)
diff --git a/generator/GoLang.ml b/generator/GoLang.ml
index cc7d78b6..7a516366 100644
--- a/generator/GoLang.ml
+++ b/generator/GoLang.ml
@@ -528,6 +528,21 @@ let
}
return ret
}
+
+func copy_extent_array(entries *C.nbd_extent, count C.size_t) []LibnbdExtent {
+ if (uint64(count) > 64*1024*1024) {
+ panic(\"violation of state machine guarantee\")
+ }
+ ret := make([]LibnbdExtent, count)
+ addr := uintptr(unsafe.Pointer(entries))
+ for i := 0; i < int(count); i++ {
+ ptr := (*C.nbd_extent)(unsafe.Pointer(addr))
+ ret[i].Length = uint64((*ptr).length)
+ ret[i].Flags = uint64((*ptr).flags)
+ addr += unsafe.Sizeof(*ptr)
+ }
+ return ret
+}
";
List.iter (
@@ -542,6 +557,8 @@ let
match cbarg with
| CBArrayAndLen (UInt32 n, _) ->
pr "%s []uint32" n;
+ | CBArrayAndLen (Extent64 n, _) ->
+ pr "%s []LibnbdExtent" n;
| CBBytesIn (n, len) ->
pr "%s []byte" n;
| CBInt n ->
@@ -568,6 +585,8 @@ let
match cbarg with
| CBArrayAndLen (UInt32 n, count) ->
pr "%s *C.uint32_t, %s C.size_t" n count
+ | CBArrayAndLen (Extent64 n, count) ->
+ pr "%s *C.nbd_extent, %s C.size_t" n count
| CBBytesIn (n, len) ->
pr "%s unsafe.Pointer, %s C.size_t" n len
| CBInt n ->
@@ -610,6 +629,8 @@ let
match cbarg with
| CBArrayAndLen (UInt32 n, count) ->
pr "copy_uint32_array(%s, %s)" n count
+ | CBArrayAndLen (Extent64 n, count) ->
+ pr "copy_extent_array(%s, %s)" n count
| CBBytesIn (n, len) ->
pr "C.GoBytes(%s, C.int(%s))" n len
| CBInt n ->
@@ -760,6 +781,8 @@ let
match cbarg with
| CBArrayAndLen (UInt32 n, count) ->
pr "uint32_t *%s, size_t %s" n count
+ | CBArrayAndLen (Extent64 n, count) ->
+ pr "nbd_extent *%s, size_t %s" n count
| CBBytesIn (n, len) ->
pr "void *%s, size_t %s" n len
| CBInt n ->
diff --git a/golang/handle.go b/golang/handle.go
index 5fe4ed4f..2b0ee6d5 100644
--- a/golang/handle.go
+++ b/golang/handle.go
@@ -58,6 +58,12 @@ func (h *Libnbd) String() string {
return "&Libnbd{}"
}
+/* Used for block status callback. */
+type LibnbdExtent struct {
+ Length uint64 // length of the extent
+ Flags uint64 // flags describing properties of the extent
+}
+
/* All functions (except Close) return ([result,] LibnbdError). */
type LibnbdError struct {
Op string // operation which failed
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 07/25] generator: Support Extent64 arg in OCaml code
See the earlier commit "Add Extent64 arg type" for rationale in
supporting a new generator arg type. This patch adds the OCaml
bindings for use of Extent64, which required adding a counterpart
OCaml type.
Note that we intend to guarantee that the size is always a positive
value. If we were using using structured_reply_in_bounds() like we do
for REPLY_TYPE_DATA, this would be trivial; but in practice, the NBD
protocol says we have to accept server responses that extend beyond
the bounds of the client's request. Still, the state machine can
easily check that the server's cumulative size has not exceeded the
export's length, which in practice is limited by off_t; furthermore,
the NBD protocol allows truncation of the response as long as the
client makes progress, and the client does not have to know whether
that truncation happened at the server or in our state machine.
Therefore, our assertion of a positive size viable.
However, the flags can be negative: OCaml's only native 64-bit integer
is inherently signed, but at least it does have a logical shift
operator for performing unsigned bit-twiddling operations.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: split out of larger patch [Laszlo], avoid awkward slice since we
can't use copy() anyway [Laszlo]
---
generator/OCaml.ml | 18 +++++++++++++++---
ocaml/helpers.c | 21 +++++++++++++++++++++
ocaml/nbd-c.h | 1 +
3 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index 7971ac40..621a4348 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -44,7 +44,7 @@ and
| Closure { cbargs } ->
sprintf "(%s)" (ocaml_closuredecl_to_string cbargs)
| Enum (_, { enum_prefix }) -> sprintf "%s.t" enum_prefix
- | Extent64 _ -> assert false (* only used in extent64_closure *)
+ | Extent64 _ -> "extent"
| Fd _ -> "Unix.file_descr"
| Flags (_, { flag_prefix }) -> sprintf "%s.t list" flag_prefix
| Int _ -> "int"
@@ -149,6 +149,9 @@ let
type cookie = int64
+type extent = int64 * int64
+(** Length and flags of an extent in block_status_64 callback. *)
+
";
List.iter (
@@ -270,6 +273,7 @@ let
exception Error of string * Unix.error option
exception Closed of string
type cookie = int64
+type extent = int64 * int64
(* Give the exceptions names so that they can be raised from the C code. *)
let () @@ -500,7 +504,7 @@ let
let argnames List.map (
function
- | CBArrayAndLen (UInt32 n, _) | CBBytesIn (n, _)
+ | CBArrayAndLen ((UInt32 n | Extent64 n), _) | CBBytesIn (n, _)
| CBInt n | CBInt64 n
| CBMutable (Int n) | CBString n | CBUInt n | CBUInt64 n ->
n ^ "v"
@@ -533,6 +537,14 @@ let
pr "%s %s,\n" indent n;
pr "%s %s\n" indent count;
pr "%s);\n" indent
+ | CBArrayAndLen (Extent64 n, count) ->
+ pr " %sv = " n;
+ let fncol = output_column () in
+ let indent = spaces fncol in
+ pr "nbd_internal_ocaml_alloc_extent64_array (\n";
+ pr "%s %s,\n" indent n;
+ pr "%s %s\n" indent count;
+ pr "%s);\n" indent
| CBBytesIn (n, len) ->
pr " %sv = caml_alloc_initialized_string (%s, %s);\n" n len n
| CBInt n | CBUInt n ->
@@ -556,7 +568,7 @@ let
List.iter (
function
- | CBArrayAndLen (UInt32 _, _)
+ | CBArrayAndLen ((UInt32 _ | Extent64 _), _)
| CBBytesIn _
| CBInt _
| CBInt64 _
diff --git a/ocaml/helpers.c b/ocaml/helpers.c
index 3361a696..7f40534a 100644
--- a/ocaml/helpers.c
+++ b/ocaml/helpers.c
@@ -133,6 +133,27 @@ nbd_internal_ocaml_alloc_i64_from_u32_array (uint32_t *a,
size_t len)
CAMLreturn (rv);
}
+value
+nbd_internal_ocaml_alloc_extent64_array (nbd_extent *a, size_t len)
+{
+ CAMLparam0 ();
+ CAMLlocal3 (s, v, rv);
+ size_t i;
+
+ rv = caml_alloc (len, 0);
+ for (i = 0; i < len; ++i) {
+ s = caml_alloc (2, 0);
+ assert (a[i].length <= INT64_MAX); /* API ensures size fits in 63 bits
*/
+ v = caml_copy_int64 (a[i].length);
+ Store_field (s, 0, v);
+ v = caml_copy_int64 (a[i].flags);
+ Store_field (s, 1, v);
+ Store_field (rv, i, s);
+ }
+
+ CAMLreturn (rv);
+}
+
/* Convert a Unix.sockaddr to a C struct sockaddr. */
void
nbd_internal_unix_sockaddr_to_sa (value sockaddrv,
diff --git a/ocaml/nbd-c.h b/ocaml/nbd-c.h
index e3abb912..adcdd15a 100644
--- a/ocaml/nbd-c.h
+++ b/ocaml/nbd-c.h
@@ -62,6 +62,7 @@ extern void nbd_internal_ocaml_raise_closed (const char *func)
Noreturn;
extern const char **nbd_internal_ocaml_string_list (value);
extern value nbd_internal_ocaml_alloc_i64_from_u32_array (uint32_t *, size_t);
+extern value nbd_internal_ocaml_alloc_extent64_array (nbd_extent *, size_t);
extern void nbd_internal_unix_sockaddr_to_sa (value, struct sockaddr_storage *,
socklen_t *);
extern void nbd_internal_ocaml_exception_in_wrapper (const char *, value);
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 08/25] block_status: Accept 64-bit extents during block status
Support a server giving us a 64-bit extent. Note that the protocol
says a server should not give a 64-bit answer when extended headers
are not negotiated; we can handle that by reporting EPROTO but
otherwise accepting the information. Conversely, when extended
headers are in effect, the server must respond with the 64-bit extent
type, even if the extent length and flags both fit in 32 bits and the
client is using the 32-bit API (including the most-likely case where a
libnbd client was compiled against an older libnbd that lacked 64-bit
API, but is now running against a newer libnbd.so that negotiates
extended headers by deafult - although at the time of THIS patch,
there are still enough other things lacking that it is not yet
possible for extended headers to be enabled).
This patch updates the code to separate where the server data is read
(bs_raw) from what is provided to the API callback (bs_cooked, at this
point, still just 32-bit, although upcoming additions for the 64-bit
client API will make bs_cooked a union as well). Ensure that both
arrays are allocated at the same point within the state machine (even
if we later end up not using bs_cooked for other reasons), so that we
aren't having to figure out how to deal with ENOMEM at other points in
time. This puts twice as much memory pressure on a libnbd handle when
receiving a block status reply, but as we bound the server into
providing no more than 64M of payload (and the NBD spec recommends no
more than 1M extents), we are unlikely to see memory pressure failures
because of this change.
With this in place, the server's reply size and the client's API size
can be orthogonal; 32->64 and 64->64 will be implemented in later
patches adding new API, 32->32 continues to work as before, and we now
support 64->32, where we have to be careful of some narrowing
considerations:
- If the server replies with a 64-bit length but still within the
exportsize, we don't want the request to fail at all, but we do have
to narrow the length to fit into 32 bits. This is okay, because the
NBD protocol says a client must be prepared for a truncated result,
and the client can't tell whether the server or libnbd did the
truncation. And this scenario is actually likely: because we are
NOT planning on using symbol version aliasing, an app compiled
against an older libnbd but run against a newer libnbd.so will
automatically negotiate extended headers even though it can only use
the 32-bit API.
- If the server replies with a 64-bit length larger than the
exportsize, we want outright EPROTO failure. We do not want to risk
the server sending junk that could look like a negative number when
treated as an int64_t instead of a uint64_t; and nbd_get_size() has
already implicitly got us to the point that exportsize will never
exceed the 63 bits of off_t. Doing this also makes it easier to
prove that our math involving 'total' does not overflow. Note that
on this path, we may set 'stop = true' more than once, but that's
okay.
- If the server replies with flags larger than 32 bits, we can't
represent the flag to the 32-bit callback. If there were earlier
extents, we want the callback to see those without an error (back to
the idea of truncation being okay); but if this is the first extent,
we have to report EOVERFLOW failure.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: major rewrite of the earlier patch by this name: split the server
read buffer and callback data buffer into separate arrays, and merely
accept a server 64-bit answer that compresses back to 32-bit API
---
lib/internal.h | 12 ++-
generator/states-reply-chunk.c | 148 ++++++++++++++++++++++++++-------
lib/handle.c | 3 +-
3 files changed, 127 insertions(+), 36 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 9df6a685..9cad91e3 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -254,11 +254,12 @@ struct nbd_handle {
uint64_t cookie;
};
} hdr;
- union {
+ union chunk_payload {
uint64_t align_; /* Start reply.payload on an 8-byte alignment */
struct nbd_chunk_offset_data offset_data;
struct nbd_chunk_offset_hole offset_hole;
struct nbd_chunk_block_status_32 bs_hdr_32;
+ struct nbd_chunk_block_status_64 bs_hdr_64;
struct {
struct nbd_chunk_error error;
char msg[NBD_MAX_STRING]; /* Common to all error types */
@@ -316,8 +317,13 @@ struct nbd_handle {
size_t payload_left;
/* When receiving block status, this is used. */
- size_t bs_count; /* count of block descriptors (not array entries!) */
- uint32_t *bs_entries;
+ size_t bs_count; /* count of descriptors (not bytes or cooked entries!) */
+ union {
+ char *storage;
+ struct nbd_block_descriptor_32 *narrow;
+ struct nbd_block_descriptor_64 *wide;
+ } bs_raw;
+ uint32_t *bs_cooked; /* Note that this array has 2*bs_count entries */
/* Commands which are waiting to be issued [meaning the request
* packet is sent to the server]. This is used as a simple linked
diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 735f9456..66771fa6 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -22,6 +22,8 @@
#include <stdint.h>
#include <inttypes.h>
+#include "minmax.h"
+
/* Structured reply must be completely inside the bounds of the
* requesting command.
*/
@@ -48,10 +50,18 @@ structured_reply_in_bounds (uint64_t offset, uint32_t
length,
static bool
bs_reply_length_ok (uint16_t type, uint32_t length)
{
- /* TODO support 64-bit replies */
- size_t prefix_len = sizeof (struct nbd_chunk_block_status_32);
- size_t extent_len = sizeof (struct nbd_block_descriptor_32);
- assert (type == NBD_REPLY_TYPE_BLOCK_STATUS);
+ size_t prefix_len;
+ size_t extent_len;
+
+ if (type == NBD_REPLY_TYPE_BLOCK_STATUS) {
+ prefix_len = sizeof (struct nbd_chunk_block_status_32);
+ extent_len = sizeof (struct nbd_block_descriptor_32);
+ }
+ else {
+ assert (type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT);
+ prefix_len = sizeof (struct nbd_chunk_block_status_64);
+ extent_len = sizeof (struct nbd_block_descriptor_64);
+ }
/* At least one descriptor is needed after id prefix */
if (length < prefix_len + extent_len)
@@ -132,14 +142,24 @@ REPLY.CHUNK_REPLY.START:
break;
case NBD_REPLY_TYPE_BLOCK_STATUS:
+ case NBD_REPLY_TYPE_BLOCK_STATUS_EXT:
if (cmd->type != NBD_CMD_BLOCK_STATUS ||
!h->meta_valid || h->meta_contexts.len == 0 ||
!bs_reply_length_ok (type, length))
goto resync;
assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
+ if (h->extended_headers != (type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT)) {
+ debug (h, "wrong block status reply type detected, "
+ "this is probably a server bug");
+ if (cmd->error == 0)
+ cmd->error = EPROTO;
+ }
/* Start by reading the context ID. */
- h->rbuf = &h->sbuf.reply.payload.bs_hdr_32;
- h->rlen = sizeof h->sbuf.reply.payload.bs_hdr_32;
+ h->rbuf = &h->sbuf.reply.payload;
+ if (type == NBD_REPLY_TYPE_BLOCK_STATUS)
+ h->rlen = sizeof h->sbuf.reply.payload.bs_hdr_32;
+ else
+ h->rlen = sizeof h->sbuf.reply.payload.bs_hdr_64;
SET_NEXT_STATE (%RECV_BS_HEADER);
break;
@@ -437,20 +457,44 @@ REPLY.CHUNK_REPLY.RECV_BS_HEADER:
assert (cmd->type == NBD_CMD_BLOCK_STATUS);
assert (bs_reply_length_ok (type, h->payload_left));
STATIC_ASSERT (sizeof (struct nbd_block_descriptor_32) =-
2 * sizeof *h->bs_entries,
+ 2 * sizeof *h->bs_cooked,
_block_desc_is_multiple_of_bs_entry);
- h->payload_left -= sizeof h->sbuf.reply.payload.bs_hdr_32;
- assert (h->payload_left % sizeof (struct nbd_block_descriptor_32) == 0);
- h->bs_count = h->payload_left / sizeof (struct
nbd_block_descriptor_32);
+ ASSERT_MEMBER_ALIAS (union chunk_payload, bs_hdr_32.context_id,
+ bs_hdr_64.context_id);
- free (h->bs_entries);
- h->bs_entries = malloc (h->payload_left);
- if (h->bs_entries == NULL) {
+ if (type == NBD_REPLY_TYPE_BLOCK_STATUS) {
+ h->payload_left -= sizeof h->sbuf.reply.payload.bs_hdr_32;
+ assert (h->payload_left % sizeof *h->bs_raw.narrow == 0);
+ h->bs_count = h->payload_left / sizeof *h->bs_raw.narrow;
+ }
+ else {
+ assert (type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT);
+ h->payload_left -= sizeof h->sbuf.reply.payload.bs_hdr_64;
+ assert (h->payload_left % sizeof *h->bs_raw.wide == 0);
+ h->bs_count = h->payload_left / sizeof *h->bs_raw.wide;
+ if (h->bs_count != be32toh (h->sbuf.reply.payload.bs_hdr_64.count))
{
+ h->rbuf = NULL;
+ h->rlen = h->payload_left;
+ SET_NEXT_STATE (%RESYNC);
+ return 0;
+ }
+ }
+
+ free (h->bs_raw.storage);
+ free (h->bs_cooked);
+ h->bs_raw.storage = malloc (h->payload_left);
+ h->bs_cooked = malloc (2 * h->bs_count * sizeof *h->bs_cooked);
+ if (h->bs_raw.storage == NULL || h->bs_cooked == NULL) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "malloc");
+ free (h->bs_raw.storage);
+ free (h->bs_cooked);
+ h->bs_raw.storage = NULL;
+ h->bs_cooked = NULL;
return 0;
}
- h->rbuf = h->bs_entries;
+
+ h->rbuf = h->bs_raw.storage;
h->rlen = h->payload_left;
h->payload_left = 0;
SET_NEXT_STATE (%RECV_BS_ENTRIES);
@@ -459,11 +503,12 @@ REPLY.CHUNK_REPLY.RECV_BS_HEADER:
REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
struct command *cmd = h->reply_cmd;
+ uint16_t type;
size_t i;
uint32_t context_id;
int error;
const char *name;
- uint32_t orig_len, len, flags;
+ uint64_t orig_len, len, flags;
uint64_t total, cap;
bool stop;
@@ -474,13 +519,15 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
SET_NEXT_STATE (%.READY);
return 0;
case 0:
+ type = be16toh (h->sbuf.reply.hdr.structured.type);
+
assert (cmd); /* guaranteed by CHECK */
assert (cmd->type == NBD_CMD_BLOCK_STATUS);
assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
- assert (h->bs_count && h->bs_entries);
+ assert (h->bs_count && h->bs_raw.storage);
assert (h->meta_valid);
- /* Look up the context ID. */
+ /* Look up the context ID. Depends on ASSERT_MEMBER_ALIAS above. */
context_id = be32toh (h->sbuf.reply.payload.bs_hdr_32.context_id);
for (i = 0; i < h->meta_contexts.len; ++i)
if (context_id == h->meta_contexts.ptr[i].context_id)
@@ -498,20 +545,57 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
total = 0;
cap = h->exportsize - cmd->offset;
assert (cap <= h->exportsize);
+ assert (h->exportsize <= INT64_MAX);
- /* Need to byte-swap the entries returned. The NBD protocol
- * allows truncation as long as progress is made; the client
- * cannot tell the difference between a server's truncation or if
- * we truncate on a length we don't like. We stop iterating on a
- * zero-length extent (error only if it is the first extent), and
- * on an extent beyond the exportsize (unconditional error after
- * truncating to exportsize); but do not diagnose issues with the
- * server's length alignments, flag values, nor compliance with
- * the REQ_ONE command flag.
+ /* Need to byte-swap the entries returned into the callback size
+ * requested by the caller. The NBD protocol allows truncation as
+ * long as progress is made; the client cannot tell the difference
+ * between a server's truncation or if we truncate on a length we
+ * don't like. We stop iterating on a zero-length extent (error
+ * only if it is the first extent), on an extent beyond the
+ * exportsize (unconditional error after truncating to
+ * exportsize), and on an extent exceeding a 32-bit callback (no
+ * error, and to simplify alignment, we truncate to 4G-64M); but
+ * do not diagnose issues with the server's length alignments,
+ * flag values, nor compliance with the REQ_ONE command flag.
+ *
+ * FIXME: still need to add nbd_block_status_64 API
*/
for (i = 0, stop = false; i < h->bs_count && !stop; ++i) {
- orig_len = len = be32toh (h->bs_entries[i * 2]);
- flags = be32toh (h->bs_entries[i * 2 + 1]);
+ if (type == NBD_REPLY_TYPE_BLOCK_STATUS) {
+ orig_len = len = be32toh (h->bs_raw.narrow[i].length);
+ flags = be32toh (h->bs_raw.narrow[i].status_flags);
+ }
+ else {
+ orig_len = len = be64toh (h->bs_raw.wide[i].length);
+ if (len > h->exportsize) {
+ /* Since we already asserted exportsize is at most 63 bits,
+ * this ensures the extent length will appear positive even
+ * if treated as signed; treat this as an error now, rather
+ * than waiting for the comparison to cap later, to avoid
+ * arithmetic overflow.
+ */
+ stop = true;
+ cmd->error = cmd->error ? : EPROTO;
+ len = h->exportsize;
+ }
+ if (len > UINT32_MAX) {
+ /* Pick an aligned value rather than overflowing 32-bit
+ * callback; this does not require an error.
+ */
+ stop = true;
+ len = (uint32_t)-MAX_REQUEST_SIZE;
+ }
+ flags = be64toh (h->bs_raw.wide[i].status_flags);
+ if (flags > UINT32_MAX) {
+ stop = true;
+ if (i > 0)
+ break; /* Skip this and later extents; we already made progress */
+ /* Expose this extent as an error; we made no progress */
+ cmd->error = cmd->error ? : EOVERFLOW;
+ }
+ }
+
total += len;
if (len == 0) {
stop = true;
@@ -526,8 +610,8 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
cmd->error = cmd->error ? : EPROTO;
len -= total - cap;
}
- h->bs_entries[i * 2] = len;
- h->bs_entries[i * 2 + 1] = flags;
+ h->bs_cooked[i * 2] = len;
+ h->bs_cooked[i * 2 + 1] = flags;
}
/* Call the caller's extent function. Yes, our 32-bit public API
@@ -536,11 +620,11 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
*/
if (stop)
debug (h, "truncating server's response at unexpected extent
length %"
- PRIu32 " and total %" PRIu64 " near extent
%zu",
+ PRIu64 " and total %" PRIu64 " near extent
%zu",
orig_len, total, i);
error = cmd->error;
if (CALL_CALLBACK (cmd->cb.fn.extent, name, cmd->offset,
- h->bs_entries, i * 2, &error) == -1)
+ h->bs_cooked, i * 2, &error) == -1)
if (cmd->error == 0)
cmd->error = error ? error : EPROTO;
}
diff --git a/lib/handle.c b/lib/handle.c
index 1d66d585..1d3aae63 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -133,7 +133,8 @@ nbd_close (struct nbd_handle *h)
nbd_unlocked_clear_debug_callback (h);
string_vector_empty (&h->querylist);
- free (h->bs_entries);
+ free (h->bs_raw.storage);
+ free (h->bs_cooked);
nbd_internal_reset_size_and_flags (h);
for (i = 0; i < h->meta_contexts.len; ++i)
free (h->meta_contexts.ptr[i].name);
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 09/25] generator: Prepare for extent64 callback
Although we don't yet have an API that takes an extent64 callback,
it's easier to add the code here separately from the API. We need to
temporarily mark a couple of helper functions in language bindings
with '__attribute__((unused))' to keep the compiler from warning about
them.
The biggest change in this patch is that the array to be passed to the
callback is now populated based on which style API was in use, and
demonstrates that the API is orthogonal from the server's reply size.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: new patch
---
lib/internal.h | 10 +++++--
generator/API.ml | 2 +-
generator/states-reply-chunk.c | 49 +++++++++++++++++++++++-----------
generator/OCaml.ml | 1 +
generator/Python.ml | 1 +
lib/aio.c | 8 ++++--
lib/handle.c | 2 +-
lib/rw.c | 10 ++++---
8 files changed, 58 insertions(+), 25 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 9cad91e3..52a17128 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -80,12 +80,14 @@ struct export {
struct command_cb {
union {
- nbd_extent_callback extent;
+ nbd_extent_callback extent32;
+ nbd_extent64_callback extent64;
nbd_chunk_callback chunk;
nbd_list_callback list;
nbd_context_callback context;
} fn;
nbd_completion_callback completion;
+ bool wide;
};
struct nbd_handle {
@@ -323,7 +325,11 @@ struct nbd_handle {
struct nbd_block_descriptor_32 *narrow;
struct nbd_block_descriptor_64 *wide;
} bs_raw;
- uint32_t *bs_cooked; /* Note that this array has 2*bs_count entries */
+ union {
+ char *storage;
+ uint32_t *narrow; /* Note that this array has 2*bs_count entries... */
+ nbd_extent *wide; /* ...while this is just bs_count entries. */
+ } bs_cooked;
/* Commands which are waiting to be issued [meaning the request
* packet is sent to the server]. This is used as a simple linked
diff --git a/generator/API.ml b/generator/API.ml
index 1fbf147b..858d86df 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -175,7 +175,7 @@ let context_closure cbargs = [ CBString "name"
]
}
let all_closures = [ chunk_closure; completion_closure;
- debug_closure; extent_closure; (* extent64_closure; *)
+ debug_closure; extent_closure; extent64_closure;
list_closure;
context_closure ]
diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 66771fa6..796262d2 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -147,7 +147,8 @@ REPLY.CHUNK_REPLY.START:
!h->meta_valid || h->meta_contexts.len == 0 ||
!bs_reply_length_ok (type, length))
goto resync;
- assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
+ ASSERT_MEMBER_ALIAS (struct command_cb, fn.extent32, fn.extent64);
+ assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent32));
if (h->extended_headers != (type == NBD_REPLY_TYPE_BLOCK_STATUS_EXT)) {
debug (h, "wrong block status reply type detected, "
"this is probably a server bug");
@@ -457,7 +458,7 @@ REPLY.CHUNK_REPLY.RECV_BS_HEADER:
assert (cmd->type == NBD_CMD_BLOCK_STATUS);
assert (bs_reply_length_ok (type, h->payload_left));
STATIC_ASSERT (sizeof (struct nbd_block_descriptor_32) =-
2 * sizeof *h->bs_cooked,
+ 2 * sizeof *h->bs_cooked.narrow,
_block_desc_is_multiple_of_bs_entry);
ASSERT_MEMBER_ALIAS (union chunk_payload, bs_hdr_32.context_id,
bs_hdr_64.context_id);
@@ -481,16 +482,20 @@ REPLY.CHUNK_REPLY.RECV_BS_HEADER:
}
free (h->bs_raw.storage);
- free (h->bs_cooked);
+ free (h->bs_cooked.storage);
h->bs_raw.storage = malloc (h->payload_left);
- h->bs_cooked = malloc (2 * h->bs_count * sizeof *h->bs_cooked);
- if (h->bs_raw.storage == NULL || h->bs_cooked == NULL) {
+ if (cmd->cb.wide)
+ h->bs_cooked.storage = malloc (h->bs_count * sizeof
*h->bs_cooked.wide);
+ else
+ h->bs_cooked.storage = malloc (2 * h->bs_count *
+ sizeof *h->bs_cooked.narrow);
+ if (h->bs_raw.storage == NULL || h->bs_cooked.storage == NULL) {
SET_NEXT_STATE (%.DEAD);
set_error (errno, "malloc");
free (h->bs_raw.storage);
- free (h->bs_cooked);
+ free (h->bs_cooked.storage);
h->bs_raw.storage = NULL;
- h->bs_cooked = NULL;
+ h->bs_cooked.storage = NULL;
return 0;
}
@@ -511,6 +516,7 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
uint64_t orig_len, len, flags;
uint64_t total, cap;
bool stop;
+ int ret;
switch (recv_into_rbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
@@ -523,7 +529,7 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
assert (cmd); /* guaranteed by CHECK */
assert (cmd->type == NBD_CMD_BLOCK_STATUS);
- assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
+ assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent32));
assert (h->bs_count && h->bs_raw.storage);
assert (h->meta_valid);
@@ -579,7 +585,7 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
cmd->error = cmd->error ? : EPROTO;
len = h->exportsize;
}
- if (len > UINT32_MAX) {
+ if (len > UINT32_MAX && !cmd->cb.wide) {
/* Pick an aligned value rather than overflowing 32-bit
* callback; this does not require an error.
*/
@@ -587,7 +593,7 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
len = (uint32_t)-MAX_REQUEST_SIZE;
}
flags = be64toh (h->bs_raw.wide[i].status_flags);
- if (flags > UINT32_MAX) {
+ if (flags > UINT32_MAX && !cmd->cb.wide) {
stop = true;
if (i > 0)
break; /* Skip this and later extents; we already made progress */
@@ -610,8 +616,15 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
cmd->error = cmd->error ? : EPROTO;
len -= total - cap;
}
- h->bs_cooked[i * 2] = len;
- h->bs_cooked[i * 2 + 1] = flags;
+ if (cmd->cb.wide) {
+ h->bs_cooked.wide[i].length = len;
+ h->bs_cooked.wide[i].flags = flags;
+ }
+ else {
+ assert ((len | flags) <= UINT32_MAX);
+ h->bs_cooked.narrow[i * 2] = len;
+ h->bs_cooked.narrow[i * 2 + 1] = flags;
+ }
}
/* Call the caller's extent function. Yes, our 32-bit public API
@@ -623,10 +636,14 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
PRIu64 " and total %" PRIu64 " near extent
%zu",
orig_len, total, i);
error = cmd->error;
- if (CALL_CALLBACK (cmd->cb.fn.extent, name, cmd->offset,
- h->bs_cooked, i * 2, &error) == -1)
- if (cmd->error == 0)
- cmd->error = error ? error : EPROTO;
+ if (cmd->cb.wide)
+ ret = CALL_CALLBACK (cmd->cb.fn.extent64, name, cmd->offset,
+ h->bs_cooked.wide, i, &error);
+ else
+ ret = CALL_CALLBACK (cmd->cb.fn.extent32, name, cmd->offset,
+ h->bs_cooked.narrow, i * 2, &error);
+ if (ret == -1 && cmd->error == 0)
+ cmd->error = error ? error : EPROTO;
}
return 0;
diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index 621a4348..ba4c8403 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -592,6 +592,7 @@ let
pr "}\n";
pr "\n";
pr "static int\n";
+ pr "__attribute__ ((unused)) /* XXX temporary hack */\n";
pr "%s_wrapper " cbname;
C.print_cbarg_list ~wrap:true cbargs;
pr "\n";
diff --git a/generator/Python.ml b/generator/Python.ml
index 761f4511..199d0265 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -158,6 +158,7 @@ let
let print_python_closure_wrapper { cbname; cbargs } pr "/* Wrapper for
%s callback. */\n" cbname;
pr "static int\n";
+ pr "__attribute__ ((unused)) /* XXX temporary hack */\n";
pr "%s_wrapper " cbname;
C.print_cbarg_list ~wrap:true cbargs;
pr "\n";
diff --git a/lib/aio.c b/lib/aio.c
index a419ac32..cf9421ec 100644
--- a/lib/aio.c
+++ b/lib/aio.c
@@ -32,8 +32,12 @@ void
nbd_internal_retire_and_free_command (struct command *cmd)
{
/* Free the callbacks. */
- if (cmd->type == NBD_CMD_BLOCK_STATUS)
- FREE_CALLBACK (cmd->cb.fn.extent);
+ if (cmd->type == NBD_CMD_BLOCK_STATUS) {
+ if (cmd->cb.wide)
+ FREE_CALLBACK (cmd->cb.fn.extent64);
+ else
+ FREE_CALLBACK (cmd->cb.fn.extent32);
+ }
if (cmd->type == NBD_CMD_READ)
FREE_CALLBACK (cmd->cb.fn.chunk);
FREE_CALLBACK (cmd->cb.completion);
diff --git a/lib/handle.c b/lib/handle.c
index 1d3aae63..8adb9c73 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -134,7 +134,7 @@ nbd_close (struct nbd_handle *h)
string_vector_empty (&h->querylist);
free (h->bs_raw.storage);
- free (h->bs_cooked);
+ free (h->bs_cooked.storage);
nbd_internal_reset_size_and_flags (h);
for (i = 0; i < h->meta_contexts.len; ++i)
free (h->meta_contexts.ptr[i].name);
diff --git a/lib/rw.c b/lib/rw.c
index 8b2bd4cc..15b309ee 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -290,8 +290,12 @@ nbd_internal_command_common (struct nbd_handle *h,
err:
/* Since we did not queue the command, we must free the callbacks. */
if (cb) {
- if (type == NBD_CMD_BLOCK_STATUS)
- FREE_CALLBACK (cb->fn.extent);
+ if (type == NBD_CMD_BLOCK_STATUS) {
+ if (cb->wide)
+ FREE_CALLBACK (cb->fn.extent32);
+ else
+ FREE_CALLBACK (cb->fn.extent64);
+ }
if (type == NBD_CMD_READ)
FREE_CALLBACK (cb->fn.chunk);
FREE_CALLBACK (cb->completion);
@@ -487,7 +491,7 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
nbd_completion_callback *completion,
uint32_t flags)
{
- struct command_cb cb = { .fn.extent = *extent,
+ struct command_cb cb = { .fn.extent32 = *extent, .wide = false,
.completion = *completion };
if (h->strict & LIBNBD_STRICT_COMMANDS) {
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 10/25] api: Add [aio_]nbd_block_status_64
Overcome the inherent 32-bit limitation of our existing
nbd_block_status command by adding a 64-bit variant. The command sent
to the server does not change, but the user's callback is now handed
64-bit information regardless of whether the server replies with 32-
or 64-bit extents. Unit tests will be added in the next patch. We
can also get rid of the temporary hack added to appease the compiler
in the previous patch.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: major rework, split unit tests into subsequent patch
---
docs/libnbd.pod | 18 ++--
sh/nbdsh.pod | 2 +-
generator/API.ml | 146 +++++++++++++++++++++++++++++----
generator/states-reply-chunk.c | 2 -
generator/OCaml.ml | 1 -
generator/Python.ml | 1 -
lib/rw.c | 68 ++++++++++++---
fuzzing/libnbd-fuzz-wrapper.c | 20 ++++-
8 files changed, 218 insertions(+), 40 deletions(-)
diff --git a/docs/libnbd.pod b/docs/libnbd.pod
index 997da0df..2a26bbda 100644
--- a/docs/libnbd.pod
+++ b/docs/libnbd.pod
@@ -668,14 +668,14 @@ In order to utilize block status, the client must call
L<nbd_add_meta_context(3)> prior to connecting, for each meta context
in which it is interested, then check L<nbd_can_meta_context(3)> after
connection to see which contexts the server actually supports. If a
-context is supported, the client can then use L<nbd_block_status(3)>
-with a callback function that will receive an array of 32-bit integer
-pairs describing consecutive extents within a context. In each pair,
-the first integer is the length of the extent, the second is a bitmask
-description of that extent (for the "base:allocation" context, the
-bitmask may include C<LIBNBD_STATE_HOLE> for unallocated portions of
-the file, and/or C<LIBNBD_STATE_ZERO> for portions of the file known
-to read as zero).
+context is supported, the client can then use
+L<nbd_block_status_64(3)> with a callback function that will receive
+an array of structs describing consecutive extents within a context.
+Each struct gives the length of the extent, then a bitmask description
+of that extent (for the "base:allocation" context, the bitmask may
+include C<LIBNBD_STATE_HOLE> for unallocated portions of the file,
+and/or C<LIBNBD_STATE_ZERO> for portions of the file known to read as
+zero).
There is a full example of requesting meta context and using block
status available at
@@ -933,7 +933,7 @@ will tie up resources until L<nbd_close(3)> is
eventually reached).
=head2 Callbacks with C<int *error> parameter
Some of the high-level commands (L<nbd_pread_structured(3)>,
-L<nbd_block_status(3)>) involve the use of a callback function invoked
+L<nbd_block_status_64(3)>) involve the use of a callback function invoked
by the state machine at appropriate points in the server's reply
before the overall command is complete. These callback functions,
along with all of the completion callbacks, include a parameter
diff --git a/sh/nbdsh.pod b/sh/nbdsh.pod
index d50b738a..d222c0ff 100644
--- a/sh/nbdsh.pod
+++ b/sh/nbdsh.pod
@@ -59,7 +59,7 @@ Display brief command line help and exit.
=item B<--base-allocation>
Request the use of the "base:allocation" meta context, which is the
-most common context used with L<nbd_block_status(3)>. This is
+most common context used with L<nbd_block_status_64(3)>. This is
equivalent to calling
S<C<h.set_meta_context(nbd.CONTEXT_BASE_ALLOCATION)>> in the shell
prior to connecting, and works even when combined with C<--uri> (while
diff --git a/generator/API.ml b/generator/API.ml
index 858d86df..6843223f 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -1565,7 +1565,7 @@ "add_meta_context", {
During connection libnbd can negotiate zero or more metadata
contexts with the server. Metadata contexts are features (such
as C<\"base:allocation\">) which describe information returned
-by the L<nbd_block_status(3)> command (for
C<\"base:allocation\">
+by the L<nbd_block_status_64(3)> command (for
C<\"base:allocation\">
this is whether blocks of data are allocated, zero or sparse).
This call adds one metadata context to the list to be negotiated.
@@ -1592,7 +1592,7 @@ "add_meta_context", {
Other metadata contexts are server-specific, but include
C<\"qemu:dirty-bitmap:...\"> and
C<\"qemu:allocation-depth\"> for
qemu-nbd (see qemu-nbd I<-B> and I<-A> options).";
- see_also = [Link "block_status"; Link
"can_meta_context";
+ see_also = [Link "block_status_64"; Link
"can_meta_context";
Link "get_nr_meta_contexts"; Link
"get_meta_context";
Link "clear_meta_contexts"];
};
@@ -1605,14 +1605,14 @@ "get_nr_meta_contexts", {
During connection libnbd can negotiate zero or more metadata
contexts with the server. Metadata contexts are features (such
as C<\"base:allocation\">) which describe information returned
-by the L<nbd_block_status(3)> command (for
C<\"base:allocation\">
+by the L<nbd_block_status_64(3)> command (for
C<\"base:allocation\">
this is whether blocks of data are allocated, zero or sparse).
This command returns how many meta contexts have been added to
the list to request from the server via L<nbd_add_meta_context(3)>.
The server is not obligated to honor all of the requests; to see
what it actually supports, see L<nbd_can_meta_context(3)>.";
- see_also = [Link "block_status"; Link
"can_meta_context";
+ see_also = [Link "block_status_64"; Link
"can_meta_context";
Link "add_meta_context"; Link
"get_meta_context";
Link "clear_meta_contexts"];
};
@@ -1625,13 +1625,13 @@ "get_meta_context", {
During connection libnbd can negotiate zero or more metadata
contexts with the server. Metadata contexts are features (such
as C<\"base:allocation\">) which describe information returned
-by the L<nbd_block_status(3)> command (for
C<\"base:allocation\">
+by the L<nbd_block_status_64(3)> command (for
C<\"base:allocation\">
this is whether blocks of data are allocated, zero or sparse).
This command returns the i'th meta context request, as added by
L<nbd_add_meta_context(3)>, and bounded by
L<nbd_get_nr_meta_contexts(3)>.";
- see_also = [Link "block_status"; Link
"can_meta_context";
+ see_also = [Link "block_status_64"; Link
"can_meta_context";
Link "add_meta_context"; Link
"get_nr_meta_contexts";
Link "clear_meta_contexts"];
};
@@ -1645,7 +1645,7 @@ "clear_meta_contexts", {
During connection libnbd can negotiate zero or more metadata
contexts with the server. Metadata contexts are features (such
as C<\"base:allocation\">) which describe information returned
-by the L<nbd_block_status(3)> command (for
C<\"base:allocation\">
+by the L<nbd_block_status_64(3)> command (for
C<\"base:allocation\">
this is whether blocks of data are allocated, zero or sparse).
This command resets the list of meta contexts to request back to
@@ -1654,7 +1654,7 @@ "clear_meta_contexts", {
negotiation mode is selected (see L<nbd_set_opt_mode(3)>), for
altering the list of attempted contexts between subsequent export
queries.";
- see_also = [Link "block_status"; Link
"can_meta_context";
+ see_also = [Link "block_status_64"; Link
"can_meta_context";
Link "add_meta_context"; Link
"get_nr_meta_contexts";
Link "get_meta_context"; Link
"set_opt_mode"];
};
@@ -2281,7 +2281,7 @@ "can_meta_context", {
^ non_blocking_test_call_description;
see_also = [SectionLink "Flag calls"; Link "opt_info";
Link "add_meta_context";
- Link "block_status"; Link
"aio_block_status";
+ Link "block_status_64"; Link
"aio_block_status_64";
Link "set_request_meta_context"; Link
"opt_set_meta_context"];
};
@@ -2712,7 +2712,7 @@ "block_status", {
optargs = [ OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
ret = RErr;
permitted_states = [ Connected ];
- shortdesc = "send block status command to the NBD server";
+ shortdesc = "send block status command, with 32-bit callback";
longdesc = "\
Issue the block status command to the NBD server. If
supported by the server, this causes metadata context
@@ -2727,7 +2727,15 @@ "block_status", {
The NBD protocol does not yet have a way for a client to learn if
the server will enforce an even smaller maximum block status size,
although a future extension may add a constraint visible in
-L<nbd_get_block_size(3)>.
+L<nbd_get_block_size(3)>. Furthermore, this function is inherently
+limited to 32-bit values. If the server replies with a larger
+extent, the length of that extent will be truncated to just
+below 32 bits and any further extents from the server will be
+ignored. If the server replies with a status value larger than
+32 bits (only possible when extended headers are in use), the
+callback function will be passed an C<EOVERFLOW> error. To get
+the full extent information from a server that supports 64-bit
+extents, you must use L<nbd_block_status_64(3)>.
Depending on which metadata contexts were enabled before
connecting (see L<nbd_add_meta_context(3)>) and which are
@@ -2747,7 +2755,7 @@ "block_status", {
array is an array of pairs of integers with the first entry in each
pair being the length (in bytes) of the block and the second entry
being a status/flags field which is specific to the metadata context.
-(The number of pairs passed to the function is C<nr_entries/2>.) The
+The number of pairs passed to the function is C<nr_entries/2>. The
NBD protocol document in the section about
C<NBD_REPLY_TYPE_BLOCK_STATUS> describes the meaning of this array;
for contexts known to libnbd, B<E<lt>libnbd.hE<gt>> contains
constants
@@ -2771,10 +2779,79 @@ "block_status", {
does not exceed C<count> bytes; however, libnbd does not
validate that the server obeyed the flag."
^ strict_call_description;
- see_also = [Link "add_meta_context"; Link
"can_meta_context";
+ see_also = [Link "block_status_64";
+ Link "add_meta_context"; Link
"can_meta_context";
Link "aio_block_status"; Link
"set_strict_mode"];
};
+ "block_status_64", {
+ default_call with
+ args = [ UInt64 "count"; UInt64 "offset"; Closure
extent64_closure ];
+ optargs = [ OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
+ ret = RErr;
+ permitted_states = [ Connected ];
+ shortdesc = "send block status command, with 64-bit callback";
+ longdesc = "\
+Issue the block status command to the NBD server. If
+supported by the server, this causes metadata context
+information about blocks beginning from the specified
+offset to be returned. The C<count> parameter is a hint: the
+server may choose to return less status, or the final block
+may extend beyond the requested range. If multiple contexts
+are supported, the number of blocks and cumulative length
+of those blocks need not be identical between contexts.
+
+Note that not all servers can support a C<count> of 4GiB or larger.
+The NBD protocol does not yet have a way for a client to learn if
+the server will enforce an even smaller maximum block status size,
+although a future extension may add a constraint visible in
+L<nbd_get_block_size(3)>.
+
+Depending on which metadata contexts were enabled before
+connecting (see L<nbd_add_meta_context(3)>) and which are
+supported by the server (see L<nbd_can_meta_context(3)>) this call
+returns information about extents by calling back to the
+C<extent64> function. The callback cannot call C<nbd_*> APIs on
the
+same handle since it holds the handle lock and will
+cause a deadlock. If the callback returns C<-1>, and no earlier
+error has been detected, then the overall block status command
+will fail with any non-zero value stored into the callback's
+C<error> parameter (with a default of C<EPROTO>); but any further
+contexts will still invoke the callback.
+
+The C<extent64> function is called once per type of metadata available,
+with the C<user_data> passed to this function. The C<metacontext>
+parameter is a string such as C<\"base:allocation\">. The
C<entries>
+array is an array of B<nbd_extent> structs, containing length (in bytes)
+of the block and a status/flags field which is specific to the metadata
+context. The number of array entries passed to the function is
+C<nr_entries>. The NBD protocol document in the section about
+C<NBD_REPLY_TYPE_BLOCK_STATUS> describes the meaning of this array;
+for contexts known to libnbd, B<E<lt>libnbd.hE<gt>> contains
constants
+beginning with C<LIBNBD_STATE_> that may help decipher the values.
+On entry to the callback, the C<error> parameter contains the errno
+value of any previously detected error.
+
+It is possible for the extent function to be called
+more times than you expect (if the server is buggy),
+so always check the C<metacontext> field to ensure you
+are receiving the data you expect. It is also possible
+that the extent function is not called at all, even for
+metadata contexts that you requested. This indicates
+either that the server doesn't support the context
+or for some other reason cannot return the data.
+
+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."
+^ strict_call_description;
+ see_also = [Link "block_status";
+ Link "add_meta_context"; Link
"can_meta_context";
+ Link "aio_block_status_64"; Link
"set_strict_mode"];
+ };
+
"poll", {
default_call with
args = [ Int "timeout" ]; ret = RInt;
@@ -3369,7 +3446,7 @@ "aio_block_status", {
OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
ret = RCookie;
permitted_states = [ Connected ];
- shortdesc = "send block status command to the NBD server";
+ shortdesc = "send block status command, with 32-bit callback";
longdesc = "\
Send the block status command to the NBD server.
@@ -3377,13 +3454,48 @@ "aio_block_status", {
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)>.
+
+This function is inherently limited to 32-bit values. If the
+server replies with a larger extent, the length of that extent
+will be truncated to just below 32 bits and any further extents
+from the server will be ignored. If the server replies with a
+status value larger than 32 bits (only possible when extended
+headers are in use), the callback function will be passed an
+C<EOVERFLOW> error. To get the full extent information from a
+server that supports 64-bit extents, you must use
+L<nbd_aio_block_status_64(3)>.
+"
^ strict_call_description;
see_also = [SectionLink "Issuing asynchronous commands";
+ Link "aio_block_status_64";
Link "can_meta_context"; Link
"block_status";
Link "set_strict_mode"];
};
+ "aio_block_status_64", {
+ default_call with
+ args = [ UInt64 "count"; UInt64 "offset"; Closure
extent64_closure ];
+ optargs = [ OClosure completion_closure;
+ OFlags ("flags", cmd_flags, Some
["REQ_ONE"]) ];
+ ret = RCookie;
+ permitted_states = [ Connected ];
+ shortdesc = "send block status command, with 64-bit callback";
+ longdesc = "\
+Send the block status command to the NBD server.
+
+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_64(3)>."
+^ strict_call_description;
+ see_also = [SectionLink "Issuing asynchronous commands";
+ Link "aio_block_status";
+ Link "can_meta_context"; Link
"block_status_64";
+ Link "set_strict_mode"];
+ };
+
"aio_get_fd", {
default_call with
args = []; ret = RFd;
@@ -3909,6 +4021,10 @@ let first_version
"set_socket_activation_name", (1, 16);
"get_socket_activation_name", (1, 16);
+ (* Added in 1.17.x development cycle, will be stable and supported in 1.18.
*)
+ "block_status_64", (1, 18);
+ "aio_block_status_64", (1, 18);
+
(* 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/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 796262d2..2cebe456 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -564,8 +564,6 @@ REPLY.CHUNK_REPLY.RECV_BS_ENTRIES:
* error, and to simplify alignment, we truncate to 4G-64M); but
* do not diagnose issues with the server's length alignments,
* flag values, nor compliance with the REQ_ONE command flag.
- *
- * FIXME: still need to add nbd_block_status_64 API
*/
for (i = 0, stop = false; i < h->bs_count && !stop; ++i) {
if (type == NBD_REPLY_TYPE_BLOCK_STATUS) {
diff --git a/generator/OCaml.ml b/generator/OCaml.ml
index ba4c8403..621a4348 100644
--- a/generator/OCaml.ml
+++ b/generator/OCaml.ml
@@ -592,7 +592,6 @@ let
pr "}\n";
pr "\n";
pr "static int\n";
- pr "__attribute__ ((unused)) /* XXX temporary hack */\n";
pr "%s_wrapper " cbname;
C.print_cbarg_list ~wrap:true cbargs;
pr "\n";
diff --git a/generator/Python.ml b/generator/Python.ml
index 199d0265..761f4511 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -158,7 +158,6 @@ let
let print_python_closure_wrapper { cbname; cbargs } pr "/* Wrapper for
%s callback. */\n" cbname;
pr "static int\n";
- pr "__attribute__ ((unused)) /* XXX temporary hack */\n";
pr "%s_wrapper " cbname;
C.print_cbarg_list ~wrap:true cbargs;
pr "\n";
diff --git a/lib/rw.c b/lib/rw.c
index 15b309ee..8e8c024c 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -150,7 +150,7 @@ nbd_unlocked_zero (struct nbd_handle *h,
return wait_for_command (h, cookie);
}
-/* Issue a block status command and wait for the reply. */
+/* Issue a block status command and wait for the reply, 32-bit callback. */
int
nbd_unlocked_block_status (struct nbd_handle *h,
uint64_t count, uint64_t offset,
@@ -168,6 +168,25 @@ nbd_unlocked_block_status (struct nbd_handle *h,
return wait_for_command (h, cookie);
}
+/* Issue a block status command and wait for the reply, 64-bit callback. */
+int
+nbd_unlocked_block_status_64 (struct nbd_handle *h,
+ uint64_t count, uint64_t offset,
+ nbd_extent64_callback *extent64,
+ uint32_t flags)
+{
+ int64_t cookie;
+ nbd_completion_callback c = NBD_NULL_COMPLETION;
+
+ cookie = nbd_unlocked_aio_block_status_64 (h, count, offset, extent64,
&c,
+ flags);
+ if (cookie == -1)
+ return -1;
+
+ assert (CALLBACK_IS_NULL (*extent64));
+ 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,
@@ -484,16 +503,10 @@ nbd_unlocked_aio_zero (struct nbd_handle *h,
count, ENOSPC, NULL, &cb);
}
-int64_t
-nbd_unlocked_aio_block_status (struct nbd_handle *h,
- uint64_t count, uint64_t offset,
- nbd_extent_callback *extent,
- nbd_completion_callback *completion,
- uint32_t flags)
+static int
+check_aio_block_status (struct nbd_handle *h, uint64_t count, uint64_t offset,
+ uint32_t flags)
{
- struct command_cb cb = { .fn.extent32 = *extent, .wide = false,
- .completion = *completion };
-
if (h->strict & LIBNBD_STRICT_COMMANDS) {
if (!h->structured_replies) {
set_error (ENOTSUP, "server does not support structured
replies");
@@ -508,8 +521,43 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
}
}
+ return 0;
+}
+
+int64_t
+nbd_unlocked_aio_block_status (struct nbd_handle *h,
+ uint64_t count, uint64_t offset,
+ nbd_extent_callback *extent,
+ nbd_completion_callback *completion,
+ uint32_t flags)
+{
+ struct command_cb cb = { .fn.extent32 = *extent, .wide = false,
+ .completion = *completion };
+
+ if (check_aio_block_status (h, count, offset, flags) == -1)
+ 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, EINVAL, NULL, &cb);
}
+
+int64_t
+nbd_unlocked_aio_block_status_64 (struct nbd_handle *h,
+ uint64_t count, uint64_t offset,
+ nbd_extent64_callback *extent64,
+ nbd_completion_callback *completion,
+ uint32_t flags)
+{
+ struct command_cb cb = { .fn.extent64 = *extent64, .wide = true,
+ .completion = *completion };
+
+ if (check_aio_block_status (h, count, offset, flags) == -1)
+ return -1;
+
+ SET_CALLBACK_TO_NULL (*extent64);
+ SET_CALLBACK_TO_NULL (*completion);
+ return nbd_internal_command_common (h, flags, NBD_CMD_BLOCK_STATUS, offset,
+ count, EINVAL, NULL, &cb);
+}
diff --git a/fuzzing/libnbd-fuzz-wrapper.c b/fuzzing/libnbd-fuzz-wrapper.c
index e111866f..cbd55380 100644
--- a/fuzzing/libnbd-fuzz-wrapper.c
+++ b/fuzzing/libnbd-fuzz-wrapper.c
@@ -165,6 +165,17 @@ extent_callback (void *user_data,
return 0;
}
+/* Block status (extents64) callback, does nothing. */
+static int
+extent64_callback (void *user_data,
+ const char *metacontext,
+ uint64_t offset, nbd_extent *entries,
+ size_t nr_entries, int *error)
+{
+ //fprintf (stderr, "extent64 called, nr_entries = %zu\n",
nr_entries);
+ return 0;
+}
+
/* This is the client (parent process) running libnbd. */
static char buf[512];
static char prbuf[65536];
@@ -213,7 +224,7 @@ client (int sock)
},
0);
- /* Test block status. */
+ /* Test both sizes of block status. */
nbd_block_status (nbd, length, 0,
(nbd_extent_callback) {
.callback = extent_callback,
@@ -221,6 +232,13 @@ client (int sock)
.free = NULL
},
0);
+ nbd_block_status_64 (nbd, length, 0,
+ (nbd_extent64_callback) {
+ .callback = extent64_callback,
+ .user_data = NULL,
+ .free = NULL
+ },
+ 0);
nbd_shutdown (nbd, 0);
}
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 11/25] api: Add tests for [aio_]nbd_block_status_64
Add unit test coverage to prove that the new API works in each of C,
Python, OCaml, and Go bindings.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: new patch, split out from API addition itself
---
python/t/465-block-status-64.py | 56 ++++++++++
ocaml/tests/Makefile.am | 1 +
ocaml/tests/test_465_block_status_64.ml | 58 +++++++++++
tests/meta-base-allocation.c | 104 ++++++++++++++++---
golang/Makefile.am | 1 +
golang/libnbd_465_block_status_64_test.go | 119 ++++++++++++++++++++++
6 files changed, 326 insertions(+), 13 deletions(-)
create mode 100644 python/t/465-block-status-64.py
create mode 100644 ocaml/tests/test_465_block_status_64.ml
create mode 100644 golang/libnbd_465_block_status_64_test.go
diff --git a/python/t/465-block-status-64.py b/python/t/465-block-status-64.py
new file mode 100644
index 00000000..a8a8eaea
--- /dev/null
+++ b/python/t/465-block-status-64.py
@@ -0,0 +1,56 @@
+# libnbd Python bindings
+# Copyright Red Hat
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+import os
+
+import nbd
+
+script = "%s/../tests/meta-base-allocation.sh" %
os.getenv("srcdir", ".")
+
+h = nbd.NBD()
+h.add_meta_context("base:allocation")
+h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v", "sh", script])
+
+entries = []
+
+
+def f(user_data, metacontext, offset, e, err):
+ global entries
+ assert user_data == 42
+ assert err.value == 0
+ if metacontext != "base:allocation":
+ return
+ entries = e
+
+
+h.block_status_64(65536, 0, lambda *args: f(42, *args))
+print("entries = %r" % entries)
+assert entries == [(8192, 0),
+ (8192, 1),
+ (16384, 3),
+ (16384, 2),
+ (16384, 0)]
+
+h.block_status_64(1024, 32256, lambda *args: f(42, *args))
+print("entries = %r" % entries)
+assert entries == [(512, 3),
+ (16384, 2)]
+
+h.block_status_64(1024, 32256, lambda *args: f(42, *args),
+ nbd.CMD_FLAG_REQ_ONE)
+print("entries = %r" % entries)
+assert entries == [(512, 3)]
diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am
index 20546350..4ddd0f9f 100644
--- a/ocaml/tests/Makefile.am
+++ b/ocaml/tests/Makefile.am
@@ -40,6 +40,7 @@ ML_TESTS = \
test_405_pread_structured.ml \
test_410_pwrite.ml \
test_460_block_status.ml \
+ test_465_block_status_64.ml \
test_500_aio_pread.ml \
test_505_aio_pread_structured_callback.ml \
test_510_aio_pwrite.ml \
diff --git a/ocaml/tests/test_465_block_status_64.ml
b/ocaml/tests/test_465_block_status_64.ml
new file mode 100644
index 00000000..0634a025
--- /dev/null
+++ b/ocaml/tests/test_465_block_status_64.ml
@@ -0,0 +1,58 @@
+(* hey emacs, this is OCaml code: -*- tuareg -*- *)
+(* libnbd OCaml test case
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *)
+
+open Printf
+
+let script + try
+ let srcdir = Sys.getenv "srcdir" in
+ sprintf "%s/../../tests/meta-base-allocation.sh" srcdir
+ with
+ Not_found -> failwith "error: srcdir is not defined"
+
+let entries = ref [||]
+let f user_data metacontext offset e err + assert (user_data = 42);
+ assert (!err = 0);
+ if metacontext = "base:allocation" then
+ entries := e;
+ 0
+
+let () + let nbd = NBD.create () in
+ NBD.add_meta_context nbd "base:allocation";
+ NBD.connect_command nbd ["nbdkit"; "-s";
"--exit-with-parent"; "-v";
+ "sh"; script];
+
+ NBD.block_status_64 nbd 65536_L 0_L (f 42);
+ assert (!entries = [| 8192_L, 0_L;
+ 8192_L, 1_L;
+ 16384_L, 3_L;
+ 16384_L, 2_L;
+ 16384_L, 0_L |]);
+
+ NBD.block_status_64 nbd 1024_L 32256_L (f 42);
+ assert (!entries = [| 512_L, 3_L;
+ 16384_L, 2_L |]);
+
+ let flags = let open NBD.CMD_FLAG in [REQ_ONE] in
+ NBD.block_status_64 nbd 1024_L 32256_L (f 42) ~flags;
+ assert (!entries = [| 512_L, 3_L |])
+
+let () = Gc.compact ()
diff --git a/tests/meta-base-allocation.c b/tests/meta-base-allocation.c
index a7b3af09..7697b5da 100644
--- a/tests/meta-base-allocation.c
+++ b/tests/meta-base-allocation.c
@@ -32,10 +32,13 @@
#define BOGUS_CONTEXT "x-libnbd:nosuch"
-static int check_extent (void *data,
- const char *metacontext,
- uint64_t offset,
- uint32_t *entries, size_t nr_entries, int *error);
+static int check_extent32 (void *data, const char *metacontext,
+ uint64_t offset,
+ uint32_t *entries, size_t nr_entries, int *error);
+
+static int check_extent64 (void *data, const char *metacontext,
+ uint64_t offset,
+ nbd_extent *entries, size_t nr_entries, int *error);
int
main (int argc, char *argv[])
@@ -43,8 +46,10 @@ main (int argc, char *argv[])
struct nbd_handle *nbd;
char plugin_path[256];
int id;
- nbd_extent_callback extent_callback = { .callback = check_extent,
- .user_data = &id };
+ nbd_extent_callback extent32_callback = { .callback = check_extent32,
+ .user_data = &id };
+ nbd_extent64_callback extent64_callback = { .callback = check_extent64,
+ .user_data = &id };
int r;
const char *s;
char *tmp;
@@ -150,23 +155,36 @@ main (int argc, char *argv[])
/* Read the block status. */
id = 1;
- if (nbd_block_status (nbd, 65536, 0, extent_callback, 0) == -1) {
+ if (nbd_block_status (nbd, 65536, 0, extent32_callback, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_block_status_64 (nbd, 65536, 0, extent64_callback, 0) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
id = 2;
- if (nbd_block_status (nbd, 1024, 32768-512, extent_callback, 0) == -1) {
+ if (nbd_block_status (nbd, 1024, 32768-512, extent32_callback, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_block_status_64 (nbd, 1024, 32768-512, extent64_callback, 0) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
id = 3;
- if (nbd_block_status (nbd, 1024, 32768-512, extent_callback,
+ if (nbd_block_status (nbd, 1024, 32768-512, extent32_callback,
LIBNBD_CMD_FLAG_REQ_ONE) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
+ if (nbd_block_status_64 (nbd, 1024, 32768-512, extent64_callback,
+ LIBNBD_CMD_FLAG_REQ_ONE) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
if (nbd_shutdown (nbd, 0) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
@@ -178,10 +196,8 @@ main (int argc, char *argv[])
}
static int
-check_extent (void *data,
- const char *metacontext,
- uint64_t offset,
- uint32_t *entries, size_t nr_entries, int *error)
+check_extent32 (void *data, const char *metacontext, uint64_t offset,
+ uint32_t *entries, size_t nr_entries, int *error)
{
size_t i;
int id;
@@ -235,3 +251,65 @@ check_extent (void *data,
return 0;
}
+
+static int
+check_extent64 (void *data, const char *metacontext, uint64_t offset,
+ nbd_extent *entries, size_t nr_entries, int *error)
+{
+ size_t i;
+ int id;
+
+ id = * (int *)data;
+
+ printf ("extent: id=%d, metacontext=%s, offset=%" PRIu64 ",
"
+ "nr_entries=%zu, error=%d\n",
+ id, metacontext, offset, nr_entries, *error);
+
+ assert (*error == 0);
+ if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
+ for (i = 0; i < nr_entries; i++) {
+ printf ("\t%zu\tlength=%" PRIu64 ", status=%" PRIu64
"\n",
+ i, entries[i].length, entries[i].flags);
+ }
+ fflush (stdout);
+
+ switch (id) {
+ case 1:
+ assert (nr_entries == 5);
+ assert (entries[0].length == 8192);
+ assert (entries[0].flags == 0);
+ assert (entries[1].length == 8192);
+ assert (entries[1].flags == LIBNBD_STATE_HOLE);
+ assert (entries[2].length == 16384);
+ assert (entries[2].flags == (LIBNBD_STATE_HOLE|LIBNBD_STATE_ZERO));
+ assert (entries[3].length == 16384);
+ assert (entries[3].flags == LIBNBD_STATE_ZERO);
+ assert (entries[4].length == 16384);
+ assert (entries[4].flags == 0);
+ break;
+
+ case 2:
+ assert (nr_entries == 2);
+ assert (entries[0].length == 512);
+ assert (entries[0].flags == (LIBNBD_STATE_HOLE|LIBNBD_STATE_ZERO));
+ assert (entries[1].length == 16384);
+ assert (entries[1].flags == LIBNBD_STATE_ZERO);
+ break;
+
+ case 3:
+ assert (nr_entries == 1);
+ assert (entries[0].length == 512);
+ assert (entries[0].flags == (LIBNBD_STATE_HOLE|LIBNBD_STATE_ZERO));
+ break;
+
+ default:
+ abort ();
+ }
+
+ }
+ else
+ fprintf (stderr, "warning: ignored unexpected meta context %s\n",
+ metacontext);
+
+ return 0;
+}
diff --git a/golang/Makefile.am b/golang/Makefile.am
index fac65248..b4151250 100644
--- a/golang/Makefile.am
+++ b/golang/Makefile.am
@@ -43,6 +43,7 @@ source_files = \
libnbd_405_pread_structured_test.go \
libnbd_410_pwrite_test.go \
libnbd_460_block_status_test.go \
+ libnbd_465_block_status_64_test.go \
libnbd_500_aio_pread_test.go \
libnbd_510_aio_pwrite_test.go \
libnbd_590_aio_copy_test.go \
diff --git a/golang/libnbd_465_block_status_64_test.go
b/golang/libnbd_465_block_status_64_test.go
new file mode 100644
index 00000000..7659a21d
--- /dev/null
+++ b/golang/libnbd_465_block_status_64_test.go
@@ -0,0 +1,119 @@
+/* libnbd golang tests
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+package libnbd
+
+import (
+ "fmt"
+ "os"
+ "strings"
+ "testing"
+)
+
+var entries64 []LibnbdExtent
+
+func mcf64(metacontext string, offset uint64, e []LibnbdExtent, error *int) int
{
+ if *error != 0 {
+ panic("expected *error == 0")
+ }
+ if metacontext == "base:allocation" {
+ entries64 = e
+ }
+ return 0
+}
+
+// Seriously WTF?
+func mc64_compare(a1 []LibnbdExtent, a2 []LibnbdExtent) bool {
+ if len(a1) != len(a2) {
+ return false
+ }
+ for i := 0; i < len(a1); i++ {
+ if a1[i] != a2[i] {
+ return false
+ }
+ }
+ return true
+}
+
+func mc64_to_string(a []LibnbdExtent) string {
+ ss := make([]string, len(a))
+ for i := 0; i < len(a); i++ {
+ ss[i] = fmt.Sprintf("%#v", a[i])
+ }
+ return strings.Join(ss, ", ")
+}
+
+func Test465BlockStatus64(t *testing.T) {
+ srcdir := os.Getenv("abs_top_srcdir")
+ script := srcdir + "/tests/meta-base-allocation.sh"
+
+ h, err := Create()
+ if err != nil {
+ t.Fatalf("could not create handle: %s", err)
+ }
+ defer h.Close()
+
+ err = h.AddMetaContext("base:allocation")
+ if err != nil {
+ t.Fatalf("%s", err)
+ }
+ err = h.ConnectCommand([]string{
+ "nbdkit", "-s", "--exit-with-parent",
"-v",
+ "sh", script,
+ })
+ if err != nil {
+ t.Fatalf("%s", err)
+ }
+
+ err = h.BlockStatus64(65536, 0, mcf64, nil)
+ if err != nil {
+ t.Fatalf("%s", err)
+ }
+ if !mc64_compare(entries64, []LibnbdExtent{
+ {8192, 0},
+ {8192, 1},
+ {16384, 3},
+ {16384, 2},
+ {16384, 0},
+ }) {
+ t.Fatalf("unexpected entries (1): %s", mc64_to_string(entries64))
+ }
+
+ err = h.BlockStatus64(1024, 32256, mcf64, nil)
+ if err != nil {
+ t.Fatalf("%s", err)
+ }
+ if !mc64_compare(entries64, []LibnbdExtent{
+ {512, 3},
+ {16384, 2},
+ }) {
+ t.Fatalf("unexpected entries (2): %s", mc64_to_string(entries64))
+ }
+
+ var optargs BlockStatus64Optargs
+ optargs.FlagsSet = true
+ optargs.Flags = CMD_FLAG_REQ_ONE
+ err = h.BlockStatus64(1024, 32256, mcf64, &optargs)
+ if err != nil {
+ t.Fatalf("%s", err)
+ }
+ if !mc64_compare(entries64, []LibnbdExtent{{512, 3}}) {
+ t.Fatalf("unexpected entries (3): %s", mc64_to_string(entries64))
+ }
+
+}
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 12/25] api: Add several functions for controlling extended headers
The new NBD_OPT_EXTENDED_HEADERS feature is worth using by default,
but there may be cases where the user explicitly wants to stick with
the older 32-bit headers. nbd_set_request_extended_headers() will let
the client override the default, nbd_get_request_extended_headers()
determines the current state of the request, and
nbd_get_extended_headers_negotiated() determines what the client and
server actually settled on. These use
nbd_set_request_structured_replies() and friends as a template.
Note that this patch just adds the API for controlling the defaults,
but ignores the state variable; a later one will then tweak the state
machine to actually request structured headers when the state variable
is set, as well as add nbd_opt_extended_headers() for manual control.
The intent is that because extended headers takes priority over
structured replies, the functions will interact as follows during the
automatic handshaking that takes place prior to nbd_opt_set_mode()
relinquishing control to the client in negotiation mode:
1. client default: request_eh=1, request_sr=1
=> try EXTENDED_HEADERS
- server supports it: nothing further to do, use extended headers,
but also report structured replies as active
- server lacks it: behave like case 2
2. request_eh=0 (explicit client downgrade), request_sr=1 (left at default)
=> try STRUCTURED_REPLY
- server supports it: expect structured replies
- server lacks it: expect simple replies
3. request_sr=0 (explicit client downgrade), request_eh ignored
=> don't try either handshake
- expect simple replies
Client code that wants to manually force simple replies only has to
disable structured replies; and only code that wants to disable
extended headers but still use structured replies has to use the new
nbd_set_request_extended_headers() API. Until a later patch adds an
explicit API nbd_opt_extended_headers(), there is no way to request
extended headers after structured replies are already negotiated.
Signed-off-by: Eric Blake <eblake at redhat.com>
Acked-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: no code changes
---
lib/internal.h | 1 +
generator/API.ml | 115 ++++++++++++++++--
generator/states-newstyle-opt-starttls.c | 1 +
.../states-newstyle-opt-structured-reply.c | 3 +-
lib/handle.c | 23 ++++
python/t/110-defaults.py | 1 +
python/t/120-set-non-defaults.py | 2 +
ocaml/tests/test_110_defaults.ml | 2 +
ocaml/tests/test_120_set_non_defaults.ml | 3 +
golang/libnbd_110_defaults_test.go | 8 ++
golang/libnbd_120_set_non_defaults_test.go | 12 ++
11 files changed, 160 insertions(+), 11 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index 52a17128..efbb5abb 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -113,6 +113,7 @@ struct nbd_handle {
char *tls_psk_file; /* PSK filename, NULL = no PSK */
/* Extended headers. */
+ bool request_eh; /* Whether to request extended headers */
bool extended_headers; /* If we negotiated NBD_OPT_EXTENDED_HEADERS */
/* Desired metadata contexts. */
diff --git a/generator/API.ml b/generator/API.ml
index 6843223f..36033817 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -808,6 +808,77 @@ "get_tls_psk_file", {
};
*)
+ "set_request_extended_headers", {
+ default_call with
+ args = [Bool "request"]; ret = RErr;
+ permitted_states = [ Created ];
+ shortdesc = "control use of extended headers";
+ longdesc = "\
+By default, libnbd tries to negotiate extended headers with the
+server, as this protocol extension permits the use of 64-bit
+zero, trim, and block status actions. However,
+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.
+
+For backwards compatibility, the setting of this knob is ignored
+if L<nbd_set_request_structured_replies(3)> is also set to false,
+since the use of extended headers implies structured replies.";
+ see_also = [Link "get_request_extended_headers";
+ Link "set_handshake_flags"; Link
"set_strict_mode";
+ Link "get_extended_headers_negotiated";
+ Link "zero"; Link "trim"; Link
"cache";
+ Link "block_status_64";
+ Link "set_request_structured_replies"];
+ };
+
+ "get_request_extended_headers", {
+ default_call with
+ args = []; ret = RBool;
+ may_set_error = false;
+ shortdesc = "see if extended headers are attempted";
+ longdesc = "\
+Return the state of the request extended headers flag on this
+handle.
+
+B<Note:> If you want to find out if extended headers were actually
+negotiated on a particular connection use
+L<nbd_get_extended_headers_negotiated(3)> instead.";
+ see_also = [Link "set_request_extended_headers";
+ Link "get_extended_headers_negotiated";
+ Link "get_request_extended_headers"];
+ };
+
+ "get_extended_headers_negotiated", {
+ default_call with
+ args = []; ret = RBool;
+ permitted_states = [ Negotiating; Connected; Closed ];
+ shortdesc = "see if extended headers are in use";
+ longdesc = "\
+After connecting you may call this to find out if the connection is
+using extended headers.
+
+When extended headers are not in use, commands are limited to a
+32-bit length, even when the libnbd API uses a 64-bit parameter
+to express the length. But even when extended headers are
+supported, the server may enforce other limits, visible through
+L<nbd_get_block_size(3)>.
+
+Note that when extended headers are negotiated, you should
+prefer the use of L<nbd_block_status_64(3)> instead of
+L<nbd_block_status(3)> if any of the meta contexts you requested
+via L<nbd_add_meta_context(3)> might return 64-bit status
+values; however, all of the well-known meta contexts covered
+by current C<LIBNBD_CONTEXT_*> constants only return 32-bit
+status.";
+ see_also = [Link "set_request_extended_headers";
+ Link "get_request_extended_headers";
+ Link "zero"; Link "trim"; Link
"cache";
+ Link "block_status_64"; Link
"get_block_size";
+ Link "get_protocol";
+ Link "get_structured_replies_negotiated"];
+ };
+
"set_request_structured_replies", {
default_call with
args = [Bool "request"]; ret = RErr;
@@ -821,12 +892,16 @@ "set_request_structured_replies", {
rather than find a way to alter the server to fail the negotiation
request. It is also useful to set this to false prior to using
L<nbd_set_opt_mode(3)> if it is desired to control when to send
-L<nbd_opt_structured_reply(3)> during negotiation.";
+L<nbd_opt_structured_reply(3)> during negotiation.
+
+Note that setting this knob to false also disables any automatic
+request for extended headers.";
see_also = [Link "get_request_structured_replies";
Link "set_handshake_flags"; Link
"set_strict_mode";
Link "opt_structured_reply";
Link "get_structured_replies_negotiated";
- Link "can_meta_context"; Link "can_df"];
+ Link "can_meta_context"; Link "can_df";
+ Link "set_request_extended_headers"];
};
"get_request_structured_replies", {
@@ -842,7 +917,8 @@ "get_request_structured_replies", {
negotiated on a particular connection use
L<nbd_get_structured_replies_negotiated(3)> instead.";
see_also = [Link "set_request_structured_replies";
- Link "get_structured_replies_negotiated"];
+ Link "get_structured_replies_negotiated";
+ Link "get_request_extended_headers"];
};
"get_structured_replies_negotiated", {
@@ -854,11 +930,17 @@ "get_structured_replies_negotiated", {
After connecting you may call this to find out if the connection is
using structured replies. Note that this setting is sticky; this
can return true even after a second L<nbd_opt_structured_reply(3)>
-returns false because the server detected a duplicate request.";
+returns false because the server detected a duplicate request.
+
+Note that if the connection negotiates extended headers, this
+function returns true (as extended headers imply structured
+replies) even if no explicit request for structured replies was
+attempted.";
see_also = [Link "set_request_structured_replies";
Link "get_request_structured_replies";
Link "opt_structured_reply";
- Link "get_protocol"];
+ Link "get_protocol";
+ Link "get_extended_headers_negotiated"];
};
"set_request_meta_context", {
@@ -2623,7 +2705,9 @@ "trim", {
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.
-Note that not all servers can support a C<count> of 4GiB or larger.
+Note that not all servers can support a C<count> of 4GiB or larger;
+L<nbd_get_extended_headers_negotiated(3)> indicates which servers
+will parse a request larger than 32 bits.
The NBD protocol does not yet have a way for a client to learn if
the server will enforce an even smaller maximum trim size, although
a future extension may add a constraint visible in
@@ -2654,7 +2738,9 @@ "cache", {
this command. Note this will generally return an error if
L<nbd_can_cache(3)> is false.
-Note that not all servers can support a C<count> of 4GiB or larger.
+Note that not all servers can support a C<count> of 4GiB or larger;
+L<nbd_get_extended_headers_negotiated(3)> indicates which servers
+will parse a request larger than 32 bits.
The NBD protocol does not yet have a way for a client to learn if
the server will enforce an even smaller maximum cache size, although
a future extension may add a constraint visible in
@@ -2683,7 +2769,9 @@ "zero", {
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.
-Note that not all servers can support a C<count> of 4GiB or larger.
+Note that not all servers can support a C<count> of 4GiB or larger;
+L<nbd_get_extended_headers_negotiated(3)> indicates which servers
+will parse a request larger than 32 bits.
The NBD protocol does not yet have a way for a client to learn if
the server will enforce an even smaller maximum zero size, although
a future extension may add a constraint visible in
@@ -2723,7 +2811,9 @@ "block_status", {
are supported, the number of blocks and cumulative length
of those blocks need not be identical between contexts.
-Note that not all servers can support a C<count> of 4GiB or larger.
+Note that not all servers can support a C<count> of 4GiB or larger;
+L<nbd_get_extended_headers_negotiated(3)> indicates which servers
+will parse a request larger than 32 bits.
The NBD protocol does not yet have a way for a client to learn if
the server will enforce an even smaller maximum block status size,
although a future extension may add a constraint visible in
@@ -2801,7 +2891,9 @@ "block_status_64", {
are supported, the number of blocks and cumulative length
of those blocks need not be identical between contexts.
-Note that not all servers can support a C<count> of 4GiB or larger.
+Note that not all servers can support a C<count> of 4GiB or larger;
+L<nbd_get_extended_headers_negotiated(3)> indicates which servers
+will parse a request larger than 32 bits.
The NBD protocol does not yet have a way for a client to learn if
the server will enforce an even smaller maximum block status size,
although a future extension may add a constraint visible in
@@ -4024,6 +4116,9 @@ let first_version (* Added in 1.17.x development cycle,
will be stable and supported in 1.18. *)
"block_status_64", (1, 18);
"aio_block_status_64", (1, 18);
+ "set_request_extended_headers", (1, 18);
+ "get_request_extended_headers", (1, 18);
+ "get_extended_headers_negotiated", (1, 18);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/generator/states-newstyle-opt-starttls.c
b/generator/states-newstyle-opt-starttls.c
index 700604dd..e497548c 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -86,6 +86,7 @@ NEWSTYLE.OPT_STARTTLS.CHECK_REPLY:
}
nbd_internal_reset_size_and_flags (h);
h->structured_replies = false;
+ h->extended_headers = false;
h->meta_valid = false;
new_sock = nbd_internal_crypto_create_session (h, h->sock);
if (new_sock == NULL) {
diff --git a/generator/states-newstyle-opt-structured-reply.c
b/generator/states-newstyle-opt-structured-reply.c
index 18245054..bb94b70a 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -25,7 +25,7 @@ NEWSTYLE.OPT_STRUCTURED_REPLY.START:
assert (h->opt_mode);
else {
assert (CALLBACK_IS_NULL (h->opt_cb.completion));
- if (!h->request_sr) {
+ if (!h->request_sr || h->structured_replies) {
if (h->opt_mode)
SET_NEXT_STATE (%.NEGOTIATING);
else
@@ -84,6 +84,7 @@ NEWSTYLE.OPT_STRUCTURED_REPLY.CHECK_REPLY:
err = 0;
break;
case NBD_REP_ERR_INVALID:
+ case NBD_REP_ERR_EXT_HEADER_REQD:
err = EINVAL;
/* fallthrough */
default:
diff --git a/lib/handle.c b/lib/handle.c
index 8adb9c73..802dcf53 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -67,6 +67,7 @@ nbd_create (void)
h->unique = 1;
h->tls_verify_peer = true;
+ h->request_eh = true;
h->request_sr = true;
h->request_meta = true;
h->request_block_size = true;
@@ -440,6 +441,28 @@ nbd_unlocked_clear_meta_contexts (struct nbd_handle *h)
return 0;
}
+
+int
+nbd_unlocked_set_request_extended_headers (struct nbd_handle *h,
+ bool request)
+{
+ h->request_eh = request;
+ return 0;
+}
+
+/* NB: may_set_error = false. */
+int
+nbd_unlocked_get_request_extended_headers (struct nbd_handle *h)
+{
+ return h->request_eh;
+}
+
+int
+nbd_unlocked_get_extended_headers_negotiated (struct nbd_handle *h)
+{
+ return h->extended_headers;
+}
+
int
nbd_unlocked_set_request_structured_replies (struct nbd_handle *h,
bool request)
diff --git a/python/t/110-defaults.py b/python/t/110-defaults.py
index ed60b786..2e59bf39 100644
--- a/python/t/110-defaults.py
+++ b/python/t/110-defaults.py
@@ -21,6 +21,7 @@
assert h.get_export_name() == ""
assert h.get_full_info() is False
assert h.get_tls() == nbd.TLS_DISABLE
+assert h.get_request_extended_headers() is True
assert h.get_request_structured_replies() is True
assert h.get_request_meta_context() is True
assert h.get_request_block_size() is True
diff --git a/python/t/120-set-non-defaults.py b/python/t/120-set-non-defaults.py
index a2fafe0c..3320f8cf 100644
--- a/python/t/120-set-non-defaults.py
+++ b/python/t/120-set-non-defaults.py
@@ -31,6 +31,8 @@
if h.supports_tls():
h.set_tls(nbd.TLS_ALLOW)
assert h.get_tls() == nbd.TLS_ALLOW
+h.set_request_extended_headers(False)
+assert h.get_request_extended_headers() is False
h.set_request_structured_replies(False)
assert h.get_request_structured_replies() is False
h.set_request_meta_context(False)
diff --git a/ocaml/tests/test_110_defaults.ml b/ocaml/tests/test_110_defaults.ml
index b95cd1f0..d7b0463f 100644
--- a/ocaml/tests/test_110_defaults.ml
+++ b/ocaml/tests/test_110_defaults.ml
@@ -26,6 +26,8 @@ let
assert (not info);
let tls = NBD.get_tls nbd in
assert (tls = NBD.TLS.DISABLE);
+ let eh = NBD.get_request_extended_headers nbd in
+ assert (eh = true);
let sr = NBD.get_request_structured_replies nbd in
assert sr;
let meta = NBD.get_request_meta_context nbd in
diff --git a/ocaml/tests/test_120_set_non_defaults.ml
b/ocaml/tests/test_120_set_non_defaults.ml
index 478a88f3..4f5c2eb5 100644
--- a/ocaml/tests/test_120_set_non_defaults.ml
+++ b/ocaml/tests/test_120_set_non_defaults.ml
@@ -39,6 +39,9 @@ let
let tls = NBD.get_tls nbd in
assert (tls = NBD.TLS.ALLOW);
);
+ NBD.set_request_extended_headers nbd false;
+ let eh = NBD.get_request_extended_headers nbd in
+ assert (eh = false);
NBD.set_request_structured_replies nbd false;
let sr = NBD.get_request_structured_replies nbd in
assert (not sr);
diff --git a/golang/libnbd_110_defaults_test.go
b/golang/libnbd_110_defaults_test.go
index 119cb7ca..510baa39 100644
--- a/golang/libnbd_110_defaults_test.go
+++ b/golang/libnbd_110_defaults_test.go
@@ -51,6 +51,14 @@ func Test110Defaults(t *testing.T) {
t.Fatalf("unexpected tls state")
}
+ eh, err := h.GetRequestExtendedHeaders()
+ if err != nil {
+ t.Fatalf("could not get extended headers state: %s", err)
+ }
+ if eh != true {
+ t.Fatalf("unexpected extended headers state")
+ }
+
sr, err := h.GetRequestStructuredReplies()
if err != nil {
t.Fatalf("could not get structured replies state: %s", err)
diff --git a/golang/libnbd_120_set_non_defaults_test.go
b/golang/libnbd_120_set_non_defaults_test.go
index b096c4f6..4c6bb3ce 100644
--- a/golang/libnbd_120_set_non_defaults_test.go
+++ b/golang/libnbd_120_set_non_defaults_test.go
@@ -81,6 +81,18 @@ func Test120SetNonDefaults(t *testing.T) {
}
}
+ err = h.SetRequestExtendedHeaders(false)
+ if err != nil {
+ t.Fatalf("could not set extended headers state: %s", err)
+ }
+ eh, err := h.GetRequestExtendedHeaders()
+ if err != nil {
+ t.Fatalf("could not get extended headers state: %s", err)
+ }
+ if eh != false {
+ t.Fatalf("unexpected extended headers state")
+ }
+
err = h.SetRequestStructuredReplies(false)
if err != nil {
t.Fatalf("could not set structured replies state: %s", err)
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 13/25] copy: Update nbdcopy to use 64-bit block status
Although our use of "base:allocation" doesn't require the use of
the
64-bit API for flags, we might perform slightly faster for a server
that does give us 64-bit extent lengths and honors larger nbd_zero
lengths.
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: no code changes
---
copy/nbd-ops.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/copy/nbd-ops.c b/copy/nbd-ops.c
index f3b3bed3..71ee0a3b 100644
--- a/copy/nbd-ops.c
+++ b/copy/nbd-ops.c
@@ -428,7 +428,7 @@ nbd_ops_asynch_notify_write (struct rw *rw, size_t index)
* request for extents in a single round trip.
*/
static int add_extent (void *vp, const char *metacontext,
- uint64_t offset, uint32_t *entries, size_t nr_entries,
+ uint64_t offset, nbd_extent *entries, size_t nr_entries,
int *error);
static void
@@ -449,11 +449,11 @@ nbd_ops_get_extents (struct rw *rw, size_t index,
size_t i;
exts.len = 0;
- if (nbd_block_status (nbd, count, offset,
- (nbd_extent_callback) {
- .user_data = &exts,
- .callback = add_extent
- }, 0) == -1) {
+ if (nbd_block_status_64 (nbd, count, offset,
+ (nbd_extent64_callback) {
+ .user_data = &exts,
+ .callback = add_extent
+ }, 0) == -1) {
/* XXX We could call default_get_extents, but unclear if it's
* the right thing to do if the server is returning errors.
*/
@@ -496,7 +496,7 @@ nbd_ops_get_extents (struct rw *rw, size_t index,
static int
add_extent (void *vp, const char *metacontext,
- uint64_t offset, uint32_t *entries, size_t nr_entries,
+ uint64_t offset, nbd_extent *entries, size_t nr_entries,
int *error)
{
extent_list *ret = vp;
@@ -505,25 +505,25 @@ add_extent (void *vp, const char *metacontext,
if (strcmp (metacontext, "base:allocation") != 0 || *error)
return 0;
- for (i = 0; i < nr_entries; i += 2) {
+ for (i = 0; i < nr_entries; i++) {
struct extent e;
e.offset = offset;
- e.length = entries[i];
+ e.length = entries[i].length;
/* Note we deliberately don't care about the HOLE flag. There is
* no need to read extent that reads as zeroes. We will convert
* to it to a hole or allocated extents based on the command line
* arguments.
*/
- e.zero = (entries[i+1] & LIBNBD_STATE_ZERO) != 0;
+ e.zero = (entries[i].flags & LIBNBD_STATE_ZERO) != 0;
if (extent_list_append (ret, e) == -1) {
perror ("realloc");
exit (EXIT_FAILURE);
}
- offset += entries[i];
+ offset += entries[i].length;
}
return 0;
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 14/25] dump: Update nbddump to use 64-bit block status
Although our use of "base:allocation" doesn't require the use of
the
64-bit API for flags, we might perform slightly faster for a server
that does give us 64-bit extent lengths.
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: no changes
---
dump/dump.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/dump/dump.c b/dump/dump.c
index b4aebe84..71053277 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -38,7 +38,7 @@
#include "version.h"
#include "vector.h"
-DEFINE_VECTOR_TYPE (uint32_vector, uint32_t);
+DEFINE_VECTOR_TYPE (uint64_vector, uint64_t);
static const char *progname;
static struct nbd_handle *nbd;
@@ -262,10 +262,10 @@ catch_signal (int sig)
static int
extent_callback (void *user_data, const char *metacontext,
uint64_t offset,
- uint32_t *entries, size_t nr_entries,
+ nbd_extent *entries, size_t nr_entries,
int *error)
{
- uint32_vector *list = user_data;
+ uint64_vector *list = user_data;
size_t i;
if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) != 0)
@@ -273,7 +273,8 @@ extent_callback (void *user_data, const char *metacontext,
/* Just append the entries we got to the list. */
for (i = 0; i < nr_entries; ++i) {
- if (uint32_vector_append (list, entries[i]) == -1) {
+ if (uint64_vector_append (list, entries[i].length) == -1 ||
+ uint64_vector_append (list, entries[i].flags) == -1) {
perror ("realloc");
exit (EXIT_FAILURE);
}
@@ -284,7 +285,7 @@ extent_callback (void *user_data, const char *metacontext,
static bool
test_all_zeroes (uint64_t offset, size_t count)
{
- uint32_vector entries = empty_vector;
+ uint64_vector entries = empty_vector;
size_t i;
uint64_t count_read;
@@ -296,22 +297,22 @@ test_all_zeroes (uint64_t offset, size_t count)
* false, causing the main code to do a full read. We could be
* smarter and keep asking the server (XXX).
*/
- if (nbd_block_status (nbd, count, offset,
- (nbd_extent_callback) {
- .callback = extent_callback,
- .user_data = &entries },
- 0) == -1) {
+ if (nbd_block_status_64 (nbd, count, offset,
+ (nbd_extent64_callback) {
+ .callback = extent_callback,
+ .user_data = &entries },
+ 0) == -1) {
fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
exit (EXIT_FAILURE);
}
count_read = 0;
for (i = 0; i < entries.len; i += 2) {
- uint32_t len = entries.ptr[i];
- uint32_t type = entries.ptr[i+1];
+ uint64_t len = entries.ptr[i];
+ uint64_t type = entries.ptr[i+1];
count_read += len;
- if (!(type & 2)) /* not zero */
+ if (!(type & LIBNBD_STATE_ZERO)) /* not zero */
return false;
}
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 15/25] info: Add --has alias for --can
'nbdinfo --has structured-reply' reads a bit more naturally than
'nbdinfo --can structured-reply'. Even though the latter mirrors the
API name, it is not hard to add another alias to make our command-line
usage easier.
Suggested-by: Richard W.M. Jones <rjones at redhat.com>
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: new patch
---
info/nbdinfo.pod | 29 +++++++++++++++++++----------
info/can.c | 2 +-
info/info-can.sh | 6 +++---
info/main.c | 4 +++-
4 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/info/nbdinfo.pod b/info/nbdinfo.pod
index 7eb3c1a0..72193c24 100644
--- a/info/nbdinfo.pod
+++ b/info/nbdinfo.pod
@@ -160,9 +160,9 @@ Test if we can connect to the NBD URI.
Test if the NBD URI connection is using TLS.
-=item nbdinfo --can structured-reply URI
+=item nbdinfo --has structured-reply URI
-Test if server can respond with structured replies (a prerequisite
+Test if server has support for structured replies (a prerequisite
for supporting block status commands).
=item nbdinfo --is rotational URI
@@ -322,26 +322,23 @@ Display brief command line help and exit.
=item B<--can read>
-=item B<--can structured-reply>
-
=item B<--can trim>
=item B<--can write>
=item B<--can zero>
-Test properties of the NBD server export or the connection itself.
-The command does not print anything. Instead it exits with success
-(S<exit code 0>) if true, or failure (S<exit code 2>) if false.
-(Other exit codes indicate an error querying the flag).
+Test properties of the NBD server export. The command does not print
+anything. Instead it exits with success (S<exit code 0>) if true, or
+failure (S<exit code 2>) if false. (Other exit codes indicate an
+error querying the flag).
For further information see the L<NBD
protocol|https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>
and the following libnbd functions: L<nbd_can_cache(3)>,
L<nbd_can_df(3)>, L<nbd_can_fast_zero(3)>,
L<nbd_can_flush(3)>,
L<nbd_can_fua(3)>, L<nbd_can_multi_conn(3)>,
L<nbd_can_trim(3)>,
-L<nbd_can_zero(3)>, L<nbd_is_read_only(3)>,
-L<nbd_get_structured_replies_negotiated(3)>.
+L<nbd_can_zero(3)>, L<nbd_is_read_only(3)>.
=item B<--color>
@@ -370,6 +367,18 @@ When using I<--list>, the default is
I<--no-content> (since
downloading from each export is expensive). To enable content probing
use I<--list --content>.
+=item B<--has structured-reply>
+
+Test properties of the NBD server connection. The command does not
+print anything. Instead it exits with success (S<exit code 0>) if
+true, or failure (S<exit code 2>) if false. (Other exit codes
+indicate an error querying the flag).
+
+For further information see the L<NBD
+protocol|https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>
+and the following libnbd functions:
+L<nbd_get_structured_replies_negotiated(3)>.
+
=item B<--is read-only>
=item B<--is rotational>
diff --git a/info/can.c b/info/can.c
index 01ab4806..8514fd5a 100644
--- a/info/can.c
+++ b/info/can.c
@@ -92,7 +92,7 @@ do_can (void)
feature = nbd_can_zero (nbd);
else {
- fprintf (stderr, "%s: unknown --can or --is option: %s\n",
+ fprintf (stderr, "%s: unknown --can/--is/--has option: %s\n",
progname, can);
exit (EXIT_FAILURE);
}
diff --git a/info/info-can.sh b/info/info-can.sh
index 6cc8cbf4..13ef1032 100755
--- a/info/info-can.sh
+++ b/info/info-can.sh
@@ -38,11 +38,11 @@ requires bash -c "nbdkit sh --dump-plugin | grep
has_can_cache=1"
# and oldstyle never, but that feels like depending a bit too much on
# the implementation.
-# --can structured-reply is not a per-export setting, but rather
+# --has structured-reply is not a per-export setting, but rather
# something set on the server as a whole.
nbdkit -v -U - sh - \
- --run '$VG nbdinfo --can structured-reply
"nbd+unix:///?socket=$unixsocket"' <<'EOF'
+ --run '$VG nbdinfo --has structured-reply
"nbd+unix:///?socket=$unixsocket"' <<'EOF'
case "$1" in
get_size) echo 1024 ;;
pread) ;;
@@ -52,7 +52,7 @@ EOF
st=0
nbdkit -v -U - --no-sr sh - \
- --run '$VG nbdinfo --can structured-reply
"nbd+unix:///?socket=$unixsocket"' <<'EOF' || st=$?
+ --run '$VG nbdinfo --has structured-reply
"nbd+unix:///?socket=$unixsocket"' <<'EOF' || st=$?
case "$1" in
get_size) echo 1024 ;;
pread) ;;
diff --git a/info/main.c b/info/main.c
index c6b3fca0..dbcc5a14 100644
--- a/info/main.c
+++ b/info/main.c
@@ -119,6 +119,8 @@ main (int argc, char *argv[])
{ "no-colours", no_argument, NULL, NO_COLOUR_OPTION
},
{ "content", no_argument, NULL, CONTENT_OPTION
},
{ "no-content", no_argument, NULL,
NO_CONTENT_OPTION },
+ { "has", required_argument, NULL, CAN_OPTION },
+ { "have", required_argument, NULL, CAN_OPTION },
{ "is", required_argument, NULL, CAN_OPTION },
{ "json", no_argument, NULL, JSON_OPTION },
{ "list", no_argument, NULL, 'L' },
@@ -296,7 +298,7 @@ main (int argc, char *argv[])
if (size_only) /* --size (!list_all) */
do_size ();
- else if (can) /* --is/--can (!list_all) */
+ else if (can) /* --is/--can/--has (!list_all) */
do_can ();
else if (map) /* --map (!list_all) */
do_map ();
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 16/25] info: Expose extended-headers support through nbdinfo
Add another bit of overall server information, as well as a '--has
extended-headers' silent query. For now, the testsuite is written
assuming that when nbdkit finally adds extended headers support, it
will also add a --no-eh kill switch comparable to its existing --no-sr
switch.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: s/can/has/ [Rich]
---
info/nbdinfo.pod | 9 +++++++++
info/can.c | 9 +++++++++
info/info-can.sh | 27 +++++++++++++++++++++++++++
info/info-packets.sh | 17 ++++++++++++++++-
info/main.c | 7 ++++++-
5 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/info/nbdinfo.pod b/info/nbdinfo.pod
index 72193c24..2bdd573f 100644
--- a/info/nbdinfo.pod
+++ b/info/nbdinfo.pod
@@ -86,6 +86,7 @@ the I<--json> parameter:
"protocol": "newstyle-fixed",
"TLS": false,
"structured": true,
+ "extended": false,
"exports": [
{
"export-name": "",
@@ -165,6 +166,11 @@ Test if the NBD URI connection is using TLS.
Test if server has support for structured replies (a prerequisite
for supporting block status commands).
+=item nbdinfo --has extended-headers URI
+
+Test if server supports extended headers (a prerequisite for
+supporting 64-bit commands; implies structured replies as well).
+
=item nbdinfo --is rotational URI
Test if the server export is backed by something which behaves like a
@@ -367,6 +373,8 @@ When using I<--list>, the default is
I<--no-content> (since
downloading from each export is expensive). To enable content probing
use I<--list --content>.
+=item B<--has extended-headers>
+
=item B<--has structured-reply>
Test properties of the NBD server connection. The command does not
@@ -377,6 +385,7 @@ indicate an error querying the flag).
For further information see the L<NBD
protocol|https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>
and the following libnbd functions:
+L<nbd_get_extended_headers_negotiated(3)>,
L<nbd_get_structured_replies_negotiated(3)>.
=item B<--is read-only>
diff --git a/info/can.c b/info/can.c
index 8514fd5a..8547ef04 100644
--- a/info/can.c
+++ b/info/can.c
@@ -50,6 +50,15 @@ do_can (void)
strcasecmp (can, "structured_replies") == 0)
feature = nbd_get_structured_replies_negotiated (nbd);
+ else if (strcasecmp (can, "eh") == 0 ||
+ strcasecmp (can, "extended header") == 0 ||
+ strcasecmp (can, "extended-header") == 0 ||
+ strcasecmp (can, "extended_header") == 0 ||
+ strcasecmp (can, "extended headers") == 0 ||
+ strcasecmp (can, "extended-headers") == 0 ||
+ strcasecmp (can, "extended_headers") == 0)
+ feature = nbd_get_extended_headers_negotiated (nbd);
+
else if (strcasecmp (can, "readonly") == 0 ||
strcasecmp (can, "read-only") == 0 ||
strcasecmp (can, "read_only") == 0)
diff --git a/info/info-can.sh b/info/info-can.sh
index 13ef1032..5d9f2e89 100755
--- a/info/info-can.sh
+++ b/info/info-can.sh
@@ -61,6 +61,33 @@ esac
EOF
test $st = 2
+# --has extended-headers cannot be positively tested until nbdkit gains
+# --no-eh support. Otherwise, it is similar to --has structured-reply.
+
+no_eh+if nbdkit --no-eh --help >/dev/null 2>/dev/null; then
+ no_eh=--no-eh
+ nbdkit -v -U - sh - \
+ --run '$VG nbdinfo --has extended-headers
"nbd+unix:///?socket=$unixsocket"' <<'EOF'
+case "$1" in
+ get_size) echo 1024 ;;
+ pread) ;;
+ *) exit 2 ;;
+esac
+EOF
+fi
+
+st=0
+nbdkit -v -U - $no_eh sh - \
+ --run '$VG nbdinfo --has extended-headers
"nbd+unix:///?socket=$unixsocket"' <<'EOF' || st=$?
+case "$1" in
+ get_size) echo 1024 ;;
+ pread) ;;
+ *) exit 2 ;;
+esac
+EOF
+test $st = 2
+
# --can cache and --can fua require special handling because in
# nbdkit-sh-plugin we must print "native" or "none". Also
the can_fua
# flag is only sent if the export is writable (hence can_write below).
diff --git a/info/info-packets.sh b/info/info-packets.sh
index 2460052e..410faef8 100755
--- a/info/info-packets.sh
+++ b/info/info-packets.sh
@@ -27,12 +27,27 @@ requires nbdkit --no-sr memory --version
out=info-packets.out
cleanup_fn rm -f $out
+# Older nbdkit does not support extended headers; --no-eh is a reliable
+# witness of whether nbdkit is new enough.
+
+no_eh+if nbdkit --no-eh --help >/dev/null 2>/dev/null; then
+ no_eh=--no-eh
+fi
+
nbdkit --no-sr -U - memory size=1M \
--run '$VG nbdinfo "nbd+unix:///?socket=$unixsocket"'
> $out
cat $out
grep "protocol: .*using simple packets" $out
-nbdkit -U - memory size=1M \
+nbdkit $no_eh -U - memory size=1M \
--run '$VG nbdinfo "nbd+unix:///?socket=$unixsocket"'
> $out
cat $out
grep "protocol: .*using structured packets" $out
+
+if test x != "x$no_eh"; then
+ nbdkit -U - memory size=1M \
+ --run '$VG nbdinfo
"nbd+unix:///?socket=$unixsocket"' > $out
+ cat $out
+ grep "protocol: .*using extended packets" $out
+fi
diff --git a/info/main.c b/info/main.c
index dbcc5a14..572dd536 100644
--- a/info/main.c
+++ b/info/main.c
@@ -306,11 +306,13 @@ main (int argc, char *argv[])
const char *protocol;
int tls_negotiated;
int sr_negotiated;
+ int eh_negotiated;
/* Print per-connection fields. */
protocol = nbd_get_protocol (nbd);
tls_negotiated = nbd_get_tls_negotiated (nbd);
sr_negotiated = nbd_get_structured_replies_negotiated (nbd);
+ eh_negotiated = nbd_get_extended_headers_negotiated (nbd);
if (!json_output) {
if (protocol) {
@@ -318,8 +320,9 @@ main (int argc, char *argv[])
fprintf (fp, "protocol: %s", protocol);
if (tls_negotiated >= 0)
fprintf (fp, " %s TLS", tls_negotiated ? "with" :
"without");
- if (sr_negotiated >= 0)
+ if (eh_negotiated >= 0 && sr_negotiated >= 0)
fprintf (fp, ", using %s packets",
+ eh_negotiated ? "extended" :
sr_negotiated ? "structured" :
"simple");
fprintf (fp, "\n");
ansi_restore (fp);
@@ -337,6 +340,8 @@ main (int argc, char *argv[])
fprintf (fp, "\"TLS\": %s,\n", tls_negotiated ?
"true" : "false");
if (sr_negotiated >= 0)
fprintf (fp, "\"structured\": %s,\n", sr_negotiated
? "true" : "false");
+ if (eh_negotiated >= 0)
+ fprintf (fp, "\"extended\": %s,\n", eh_negotiated ?
"true" : "false");
}
if (!list_all)
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 17/25] info: Update nbdinfo --map to use 64-bit block status
Although we usually map "base:allocation" which doesn't require
the
use of the 64-bit API for flags, this application IS intended to map
out other metacontexts that might have 64-bit flags. And when
extended headers are in use, we might as well ask for the server to
give us extents as large as it wants, rather than breaking things up
at 4G boundaries.
At the time this patch was written, there are no known servers that
actually provide a metacontext with 64-bit flags. However, that is
planned for the nbdkit v3 protocol.
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: no change
---
info/map.c | 65 ++++++++++++++++++++++++++++--------------------------
1 file changed, 34 insertions(+), 31 deletions(-)
diff --git a/info/map.c b/info/map.c
index 1169fce4..50b058f2 100644
--- a/info/map.c
+++ b/info/map.c
@@ -36,13 +36,13 @@
#include "nbdinfo.h"
-DEFINE_VECTOR_TYPE (uint32_vector, uint32_t);
+DEFINE_VECTOR_TYPE (uint64_vector, uint64_t);
-static void print_extents (uint32_vector *entries);
-static void print_totals (uint32_vector *entries, int64_t size);
+static void print_extents (uint64_vector *entries);
+static void print_totals (uint64_vector *entries, int64_t size);
static int extent_callback (void *user_data, const char *metacontext,
uint64_t offset,
- uint32_t *entries, size_t nr_entries,
+ nbd_extent *entries, size_t nr_entries,
int *error);
void
@@ -50,7 +50,7 @@ do_map (void)
{
size_t i;
int64_t size;
- uint32_vector entries = empty_vector;
+ uint64_vector entries = empty_vector;
uint64_t offset, align, max_len;
size_t prev_entries_size;
@@ -69,14 +69,16 @@ do_map (void)
fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
exit (EXIT_FAILURE);
}
+ if (nbd_get_extended_headers_negotiated (nbd) == 1)
+ max_len = size;
for (offset = 0; offset < size;) {
prev_entries_size = entries.len;
- if (nbd_block_status (nbd, MIN (size - offset, max_len), offset,
- (nbd_extent_callback) {
- .callback = extent_callback,
- .user_data = &entries },
- 0) == -1) {
+ if (nbd_block_status_64 (nbd, MIN (size - offset, max_len), offset,
+ (nbd_extent64_callback) {
+ .callback = extent_callback,
+ .user_data = &entries },
+ 0) == -1) {
fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
exit (EXIT_FAILURE);
}
@@ -99,18 +101,18 @@ do_map (void)
}
/* Callback handling --map. */
-static void print_one_extent (uint64_t offset, uint64_t len, uint32_t type);
-static void extent_description (const char *metacontext, uint32_t type,
+static void print_one_extent (uint64_t offset, uint64_t len, uint64_t type);
+static void extent_description (const char *metacontext, uint64_t type,
char **descr, bool *free_descr,
const char **fg, const char **bg);
static int
extent_callback (void *user_data, const char *metacontext,
uint64_t offset,
- uint32_t *entries, size_t nr_entries,
+ nbd_extent *entries, size_t nr_entries,
int *error)
{
- uint32_vector *list = user_data;
+ uint64_vector *list = user_data;
size_t i;
if (strcmp (metacontext, map) != 0)
@@ -120,7 +122,8 @@ extent_callback (void *user_data, const char *metacontext,
* print_extents below.
*/
for (i = 0; i < nr_entries; ++i) {
- if (uint32_vector_append (list, entries[i]) == -1) {
+ if (uint64_vector_append (list, entries[i].length) == -1 ||
+ uint64_vector_append (list, entries[i].flags) == -1) {
perror ("realloc");
exit (EXIT_FAILURE);
}
@@ -129,7 +132,7 @@ extent_callback (void *user_data, const char *metacontext,
}
static void
-print_extents (uint32_vector *entries)
+print_extents (uint64_vector *entries)
{
size_t i, j;
uint64_t offset = 0; /* end of last extent printed + 1 */
@@ -138,7 +141,7 @@ print_extents (uint32_vector *entries)
if (json_output) fprintf (fp, "[\n");
for (i = 0; i < entries->len; i += 2) {
- uint32_t type = entries->ptr[last+1];
+ uint64_t type = entries->ptr[last+1];
/* If we're coalescing and the current type is different from the
* previous one then we should print everything up to this entry.
@@ -157,7 +160,7 @@ print_extents (uint32_vector *entries)
/* Print the last extent if there is one. */
if (last != i) {
- uint32_t type = entries->ptr[last+1];
+ uint64_t type = entries->ptr[last+1];
uint64_t len;
for (j = last, len = 0; j < i; j += 2)
@@ -169,7 +172,7 @@ print_extents (uint32_vector *entries)
}
static void
-print_one_extent (uint64_t offset, uint64_t len, uint32_t type)
+print_one_extent (uint64_t offset, uint64_t len, uint64_t type)
{
static bool comma = false;
char *descr;
@@ -185,7 +188,7 @@ print_one_extent (uint64_t offset, uint64_t len, uint32_t
type)
ansi_colour (bg, fp);
fprintf (fp, "%10" PRIu64 " "
"%10" PRIu64 " "
- "%3" PRIu32,
+ "%3" PRIu64,
offset, len, type);
if (descr)
fprintf (fp, " %s", descr);
@@ -199,7 +202,7 @@ print_one_extent (uint64_t offset, uint64_t len, uint32_t
type)
fprintf (fp, "{ \"offset\": %" PRIu64 ", "
"\"length\": %" PRIu64 ", "
- "\"type\": %" PRIu32,
+ "\"type\": %" PRIu64,
offset, len, type);
if (descr) {
fprintf (fp, ", \"description\": ");
@@ -215,9 +218,9 @@ print_one_extent (uint64_t offset, uint64_t len, uint32_t
type)
/* --map --totals suboption */
static void
-print_totals (uint32_vector *entries, int64_t size)
+print_totals (uint64_vector *entries, int64_t size)
{
- uint32_t type;
+ uint64_t type;
bool comma = false;
/* This is necessary to avoid a divide by zero below, but if the
@@ -237,16 +240,16 @@ print_totals (uint32_vector *entries, int64_t size)
*/
type = 0;
for (;;) {
- uint64_t next_type = (uint64_t)UINT32_MAX + 1;
+ uint64_t next_type = 0;
uint64_t c = 0;
size_t i;
for (i = 0; i < entries->len; i += 2) {
- uint32_t t = entries->ptr[i+1];
+ uint64_t t = entries->ptr[i+1];
if (t == type)
c += entries->ptr[i];
- else if (type < t && t < next_type)
+ else if (type < t && (next_type == 0 || t < next_type))
next_type = t;
}
@@ -263,7 +266,7 @@ print_totals (uint32_vector *entries, int64_t size)
ansi_colour (fg, fp);
if (bg)
ansi_colour (bg, fp);
- fprintf (fp, "%10" PRIu64 " %5.1f%% %3" PRIu32,
+ fprintf (fp, "%10" PRIu64 " %5.1f%% %3" PRIu64,
c, percent, type);
if (descr)
fprintf (fp, " %s", descr);
@@ -278,7 +281,7 @@ print_totals (uint32_vector *entries, int64_t size)
fprintf (fp,
"{ \"size\": %" PRIu64 ", "
"\"percent\": %g, "
- "\"type\": %" PRIu32,
+ "\"type\": %" PRIu64,
c, percent, type);
if (descr) {
fprintf (fp, ", \"description\": ");
@@ -292,7 +295,7 @@ print_totals (uint32_vector *entries, int64_t size)
free (descr);
}
- if (next_type == (uint64_t)UINT32_MAX + 1)
+ if (next_type == 0)
break;
type = next_type;
}
@@ -301,7 +304,7 @@ print_totals (uint32_vector *entries, int64_t size)
}
static void
-extent_description (const char *metacontext, uint32_t type,
+extent_description (const char *metacontext, uint64_t type,
char **descr, bool *free_descr,
const char **fg, const char **bg)
{
@@ -348,7 +351,7 @@ extent_description (const char *metacontext, uint32_t type,
*fg = ANSI_FG_BRIGHT_WHITE; *bg = ANSI_BG_BLACK;
return;
default:
- if (asprintf (descr, "backing depth %u", type) == -1) {
+ if (asprintf (descr, "backing depth %" PRIu64, type) == -1) {
perror ("asprintf");
exit (EXIT_FAILURE);
}
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 18/25] examples: Update copy-libev to use 64-bit block status
Although our use of "base:allocation" doesn't require the use of
the
64-bit API for flags, we might perform slightly faster for a server
that does give us 64-bit extent lengths and honors larger nbd_zero
lengths.
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: no change
---
examples/copy-libev.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/examples/copy-libev.c b/examples/copy-libev.c
index 32cb46b3..9346faf7 100644
--- a/examples/copy-libev.c
+++ b/examples/copy-libev.c
@@ -96,7 +96,7 @@ struct request {
};
struct extent {
- uint32_t length;
+ uint64_t length;
bool zero;
};
@@ -184,7 +184,7 @@ get_events (struct connection *c)
static int
extent_callback (void *user_data, const char *metacontext, uint64_t offset,
- uint32_t *entries, size_t nr_entries, int *error)
+ nbd_extent *entries, size_t nr_entries, int *error)
{
struct request *r = user_data;
@@ -199,22 +199,21 @@ extent_callback (void *user_data, const char *metacontext,
uint64_t offset,
return 1;
}
- /* Libnbd returns uint32_t pair (length, flags) for each extent. */
- extents_len = nr_entries / 2;
+ extents_len = nr_entries;
extents = malloc (extents_len * sizeof *extents);
if (extents == NULL)
FAIL ("Cannot allocated extents: %s", strerror (errno));
/* Copy libnbd entries to extents array. */
- for (int i = 0, j = 0; i < extents_len; i++, j=i*2) {
- extents[i].length = entries[j];
+ for (int i = 0; i < extents_len; i++) {
+ extents[i].length = entries[i].length;
/* Libnbd exposes both ZERO and HOLE flags. We care only about
* ZERO status, meaning we can copy this extent using efficinet
* zero method.
*/
- extents[i].zero = (entries[j + 1] & LIBNBD_STATE_ZERO) != 0;
+ extents[i].zero = (entries[i].flags & LIBNBD_STATE_ZERO) != 0;
}
DEBUG ("r%zu: received %zu extents for %s",
@@ -286,10 +285,10 @@ start_extents (struct request *r)
DEBUG ("r%zu: start extents offset=%" PRIi64 "
count=%zu",
r->index, offset, count);
- cookie = nbd_aio_block_status (
+ cookie = nbd_aio_block_status_64 (
src.nbd, count, offset,
- (nbd_extent_callback) { .callback=extent_callback,
- .user_data=r },
+ (nbd_extent64_callback) { .callback=extent_callback,
+ .user_data=r },
(nbd_completion_callback) { .callback=extents_completed,
.user_data=r },
0);
@@ -324,7 +323,7 @@ next_extent (struct request *r)
limit = MIN (REQUEST_SIZE, size - offset);
while (length < limit) {
- DEBUG ("e%zu: offset=%" PRIi64 " len=%" PRIu32
" zero=%d",
+ DEBUG ("e%zu: offset=%" PRIi64 " len=%" PRIu64
" zero=%d",
extents_pos, offset, extents[extents_pos].length, is_zero);
/* If this extent is too large, steal some data from it to
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 19/25] ocaml: Add example for 64-bit extents
Since our example program for 32-bit extents is inherently limited to 32-bit lengths, it is also worth demonstrating the 64-bit extent API, including the difference in the array indexing being saner. Signed-off-by: Eric Blake <eblake at redhat.com> Reviewed-by: Richard W.M. Jones <rjones at redhat.com> --- v4: no change --- ocaml/examples/Makefile.am | 1 + ocaml/examples/extents64.ml | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 ocaml/examples/extents64.ml diff --git a/ocaml/examples/Makefile.am b/ocaml/examples/Makefile.am index 28b4ab94..a4eb47a5 100644 --- a/ocaml/examples/Makefile.am +++ b/ocaml/examples/Makefile.am @@ -20,6 +20,7 @@ include $(top_srcdir)/subdir-rules.mk ml_examples = \ asynch_copy.ml \ extents.ml \ + extents64.ml \ get_size.ml \ open_qcow2.ml \ server_flags.ml \ diff --git a/ocaml/examples/extents64.ml b/ocaml/examples/extents64.ml new file mode 100644 index 00000000..8ee7e218 --- /dev/null +++ b/ocaml/examples/extents64.ml @@ -0,0 +1,42 @@ +open Printf + +let () + NBD.with_handle ( + fun nbd -> + NBD.add_meta_context nbd "base:allocation"; + NBD.connect_command nbd + ["nbdkit"; "-s"; "--exit-with-parent"; "-r"; + "sparse-random"; "8G"]; + + (* Read the extents and print them. *) + let size = NBD.get_size nbd in + let cap + match NBD.get_extended_headers_negotiated nbd with + | true -> size + | false -> 0x8000_0000_L in + let fetch_offset = ref 0_L in + while !fetch_offset < size do + let remaining = Int64.sub size !fetch_offset in + let fetch_size = min remaining cap in + NBD.block_status_64 nbd fetch_size !fetch_offset ( + fun meta _ entries err -> + printf "nbd_block_status callback: meta=%s err=%d\n" meta !err; + if meta = "base:allocation" then ( + printf "index\t%16s %16s %s\n" "offset" "length" "flags"; + for i = 0 to Array.length entries - 1 do + let len = fst entries.(i) + and flags + match snd entries.(i) with + | 0_L -> "data" + | 1_L -> "hole" + | 2_L -> "zero" + | 3_L -> "hole+zero" + | unknown -> sprintf "unknown (%Ld)" unknown in + printf "%d:\t%16Ld %16Ld %s\n" i !fetch_offset len flags; + fetch_offset := Int64.add !fetch_offset len + done; + ); + 0 + ) (* NBD.block_status *) + done + ) -- 2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 20/25] generator: Actually request extended headers
This is the culmination of the previous patches' preparation work for
using extended headers when possible. The new states in the state
machine are copied extensively from our handling of
OPT_STRUCTURED_REPLY. The next patch will then expose a new API
nbd_opt_extended_headers() for manual control.
At the same time I posted this patch, I had patches for qemu-nbd to
support extended headers as server (nbdkit is a bit tougher). The
next patches will add some interop tests that pass when using a new
enough qemu-nbd, showing that we have cross-project interoperability
and therefore an extension worth standardizing.
NOTE: This patch causes an observable (albeit uncommon) change in
behavior to a specific corner case involving a meta-context with
64-bit flags (so far, no such meta-context exists, but I'll use
"x-context:big" as a placeholder for such a meta-context). An
application compiled and run with libnbd 1.16 that requests
nbd_add_meta_context(h, "x-context:big") will fail to negotiate that
context, but can still succeed at negotiating "base:allocation".
What's more, that application was compiled at a time when the
nbd_block_status_64() API did not exist, so it will necessarily be
using the older 32-bit nbd_block_status() API. With the approach done
in this patch (that is, the same client now linking against libnbd
1.18 defaults to unconditionally requesting extended headers), the
negotiation for "x-context:big" will now succeed, but calls to
nbd_block_status() will encounter an EOVERFLOW error when the server's
64-bit flags cannot be passed back to the caller's 32-bit callback.
The caller will still see the "base:allocation" results, but the
overall API will fail. Thus, we have taken a case that used to pass
into something that now fails.
Still, it is easy to make the following mitigating arguments: 1) most
applications _don't_ blindly request "x-context:big" while
insisting
on a 32-bit API; since interpreting the context bits is
context-specific, any app that knows what those bits actually mean (or
which want to support arbitrary user input for meta contexts, like
'nbdinfo --map') will want to be compiled against libnbd 1.18 in the
first place, to take advantage of nbd_block_status_64(). 2) If the
application REALLY needs to preserve 1.16 behavior, even when compiled
against 1.18 or newer, it is easy enough to add the following escape
hatch:
#if LIBNBD_HAVE_NBD_SET_REQUEST_EXTENDED_HEADERS
nbd_set_request_extended_headers(h, 0);
#endif
And since such an escape hatch is only needed in the finite (possibly
empty?) set of programs that need to preserve the older behavior, it
does not introduce scaling problems.
Because my approach is a subtle change, I also evaluated two other
potential solutions, documented here, and why I did not go with them:
Approach 2) No change to the default. ALL applications that want to
use extended headers have to explicitly opt-in (possibly taking
advantage of LIBNBD_HAVE_... witness macros to allow successful
compilation to fallback APIs when compiling against older libnbd).
This avoids the subtle change, but comes at an open-ended cost: all
future libnbd clients that WANT to benefit from extended headers will
have to add boilerplate to their connection setup, which does not
scale well, just to benefit the few applications that depended on the
older behavior. It becomes highly likely that someone will write a
new client but does not use the new boilerplate, and thereby
inadvertently suffers from worse performace because of the older
defaults being set in stone.
Approach 3) Resort to versioned symbols, to make the default of
whether to request extended headers (in the absence of an explicit
nbd_set_request_extended_headers() call) depend on which version of
libnbd one compiles against. That is, teach our linker script to
produce aliased symbols, where nbd_create becomes an alias to either
__nbd_create@@1_16 (default off) for backwards compatibility to apps
compiled against older libnbd but run against the new .so, or to
__nbd_create@@1_18 (default on) with preprocessor magic in <libnbd.h>
that you get this alias by default when compiling against newer
libnbd. This scales well for clients (old apps continue to run with
identical behavior, new apps automatically get new features unless
they request preprocessor magic to specifically favor the older
aliasing styles), but is dreadful to maintain (look at glibc for the
hoops they have to jump through for such versioned-symbol shenanigans
needed to provide transparent multi-standard support). I don't see
libnbd as such a critical or high-user-base project that we need to
take on this extra level of work.
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: expand commit message to document other alternatives I did not
code up
---
generator/API.ml | 87 ++++++++---------
generator/state_machine.ml | 41 ++++++++
.../states-newstyle-opt-extended-headers.c | 94 +++++++++++++++++++
generator/states-newstyle-opt-starttls.c | 6 +-
generator/Makefile.am | 1 +
5 files changed, 184 insertions(+), 45 deletions(-)
create mode 100644 generator/states-newstyle-opt-extended-headers.c
diff --git a/generator/API.ml b/generator/API.ml
index 36033817..d1849710 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -953,23 +953,24 @@ "set_request_meta_context", {
(all C<nbd_connect_*> calls when L<nbd_set_opt_mode(3)> is false,
or L<nbd_opt_go(3)> and L<nbd_opt_info(3)> when option mode is
enabled) will also try to issue NBD_OPT_SET_META_CONTEXT when
-the server supports structured replies and any contexts were
-registered by L<nbd_add_meta_context(3)>. The default setting
-is true; however the extra step of negotiating meta contexts is
-not always desirable: performing both info and go on the same
-export works without needing to re-negotiate contexts on the
-second call; integration testing of other servers may benefit
-from manual invocation of L<nbd_opt_set_meta_context(3)> at
-other times in the negotiation sequence; and even when using just
-L<nbd_opt_info(3)>, it can be faster to collect the server's
+the server supports structured replies or extended headers and
+any contexts were registered by L<nbd_add_meta_context(3)>. The
+default setting is true; however the extra step of negotiating
+meta contexts is not always desirable: performing both info and
+go on the same export works without needing to re-negotiate
+contexts on the second call; integration testing of other servers
+may benefit from manual invocation of L<nbd_opt_set_meta_context(3)>
+at other times in the negotiation sequence; and even when using
+just L<nbd_opt_info(3)>, it can be faster to collect the server's
results by relying on the callback function passed to
L<nbd_opt_list_meta_context(3)> than a series of post-process
calls to L<nbd_can_meta_context(3)>.
Note that this control has no effect if the server does not
-negotiate structured replies, or if the client did not request
-any contexts via L<nbd_add_meta_context(3)>. Setting this
-control to false may cause L<nbd_block_status(3)> to fail.";
+negotiate structured replies or extended headers, or if the
+client did not request any contexts via L<nbd_add_meta_context(3)>.
+Setting this control to false may cause L<nbd_block_status(3)>
+to fail.";
see_also = [Link "set_opt_mode"; Link "opt_go"; Link
"opt_info";
Link "opt_list_meta_context"; Link
"opt_set_meta_context";
Link "get_structured_replies_negotiated";
@@ -1404,11 +1405,11 @@ "opt_info", {
If successful, functions like L<nbd_is_read_only(3)> and
L<nbd_get_size(3)> will report details about that export. If
L<nbd_set_request_meta_context(3)> is set (the default) and
-structured replies were negotiated, it is also valid to use
-L<nbd_can_meta_context(3)> after this call. However, it may be
-more efficient to clear that setting and manually utilize
-L<nbd_opt_list_meta_context(3)> with its callback approach, for
-learning which contexts an export supports. In general, if
+structured replies or extended headers were negotiated, it is also
+valid to use L<nbd_can_meta_context(3)> after this call. However,
+it may be more efficient to clear that setting and manually
+utilize L<nbd_opt_list_meta_context(3)> with its callback approach,
+for learning which contexts an export supports. In general, if
L<nbd_opt_go(3)> is called next, that call will likely succeed
with the details remaining the same, although this is not
guaranteed by all servers.
@@ -1538,12 +1539,12 @@ "opt_set_meta_context", {
recent L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.
This can only be used if L<nbd_set_opt_mode(3)> enabled option
mode. Normally, this function is redundant, as L<nbd_opt_go(3)>
-automatically does the same task if structured replies have
-already been negotiated. But manual control over meta context
-requests can be useful for fine-grained testing of how a server
-handles unusual negotiation sequences. Often, use of this
-function is coupled with L<nbd_set_request_meta_context(3)> to
-bypass the automatic context request normally performed by
+automatically does the same task if structured replies or extended
+headers have already been negotiated. But manual control over
+meta context requests can be useful for fine-grained testing of
+how a server handles unusual negotiation sequences. Often, use
+of this function is coupled with L<nbd_set_request_meta_context(3)>
+to bypass the automatic context request normally performed by
L<nbd_opt_go(3)>.
The NBD protocol allows a client to decide how many queries to ask
@@ -1597,12 +1598,13 @@ "opt_set_meta_context_queries", {
or L<nbd_connect_uri(3)>. This can only be used if
L<nbd_set_opt_mode(3)> enabled option mode. Normally, this
function is redundant, as L<nbd_opt_go(3)> automatically does
-the same task if structured replies have already been
-negotiated. But manual control over meta context requests can
-be useful for fine-grained testing of how a server handles
-unusual negotiation sequences. Often, use of this function is
-coupled with L<nbd_set_request_meta_context(3)> to bypass the
-automatic context request normally performed by L<nbd_opt_go(3)>.
+the same task if structured replies or extended headers have
+already been negotiated. But manual control over meta context
+requests can be useful for fine-grained testing of how a server
+handles unusual negotiation sequences. Often, use of this
+function is coupled with L<nbd_set_request_meta_context(3)> to
+bypass the automatic context request normally performed by
+L<nbd_opt_go(3)>.
The NBD protocol allows a client to decide how many queries to ask
the server. This function takes an explicit list of queries; to
@@ -3281,13 +3283,13 @@ "aio_opt_set_meta_context", {
recent L<nbd_set_export_name(3)> or L<nbd_connect_uri(3)>.
This can only be used if L<nbd_set_opt_mode(3)> enabled option
mode. Normally, this function is redundant, as L<nbd_opt_go(3)>
-automatically does the same task if structured replies have
-already been negotiated. But manual control over meta context
-requests can be useful for fine-grained testing of how a server
-handles unusual negotiation sequences. Often, use of this
-function is coupled with L<nbd_set_request_meta_context(3)> to
-bypass the automatic context request normally performed by
-L<nbd_opt_go(3)>.
+automatically does the same task if structured replies or
+extended headers have already been negotiated. But manual
+control over meta context requests can be useful for fine-grained
+testing of how a server handles unusual negotiation sequences.
+Often, use of this function is coupled with
+L<nbd_set_request_meta_context(3)> to bypass the automatic
+context request normally performed by L<nbd_opt_go(3)>.
To determine when the request completes, wait for
L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
@@ -3314,12 +3316,13 @@ "aio_opt_set_meta_context_queries", {
or L<nbd_connect_uri(3)>. This can only be used
if L<nbd_set_opt_mode(3)> enabled option mode. Normally, this
function is redundant, as L<nbd_opt_go(3)> automatically does
-the same task if structured replies have already been
-negotiated. But manual control over meta context requests can
-be useful for fine-grained testing of how a server handles
-unusual negotiation sequences. Often, use of this function is
-coupled with L<nbd_set_request_meta_context(3)> to bypass the
-automatic context request normally performed by L<nbd_opt_go(3)>.
+the same task if structured replies or extended headers have
+already been negotiated. But manual control over meta context
+requests can be useful for fine-grained testing of how a server
+handles unusual negotiation sequences. Often, use of this
+function is coupled with L<nbd_set_request_meta_context(3)> to
+bypass the automatic context request normally performed by
+L<nbd_opt_go(3)>.
To determine when the request completes, wait for
L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
diff --git a/generator/state_machine.ml b/generator/state_machine.ml
index 7a5bc59b..2d912ef8 100644
--- a/generator/state_machine.ml
+++ b/generator/state_machine.ml
@@ -297,6 +297,7 @@ and
* NEGOTIATING after OPT_STRUCTURED_REPLY or any failed OPT_GO.
*)
Group ("OPT_STARTTLS", newstyle_opt_starttls_state_machine);
+ Group ("OPT_EXTENDED_HEADERS",
newstyle_opt_extended_headers_state_machine);
Group ("OPT_STRUCTURED_REPLY",
newstyle_opt_structured_reply_state_machine);
Group ("OPT_META_CONTEXT",
newstyle_opt_meta_context_state_machine);
Group ("OPT_GO", newstyle_opt_go_state_machine);
@@ -441,6 +442,46 @@ and
};
]
+(* Fixed newstyle NBD_OPT_EXTENDED_HEADERS option.
+ * Implementation: generator/states-newstyle-opt-extended-headers.c
+ *)
+and newstyle_opt_extended_headers_state_machine = [
+ State {
+ default_state with
+ name = "START";
+ comment = "Try to negotiate newstyle NBD_OPT_EXTENDED_HEADERS";
+ external_events = [];
+ };
+
+ State {
+ default_state with
+ name = "SEND";
+ comment = "Send newstyle NBD_OPT_EXTENDED_HEADERS negotiation
request";
+ external_events = [ NotifyWrite, "" ];
+ };
+
+ State {
+ default_state with
+ name = "RECV_REPLY";
+ comment = "Receive newstyle NBD_OPT_EXTENDED_HEADERS option
reply";
+ external_events = [ NotifyRead, "" ];
+ };
+
+ State {
+ default_state with
+ name = "RECV_REPLY_PAYLOAD";
+ comment = "Receive any newstyle NBD_OPT_EXTENDED_HEADERS reply
payload";
+ external_events = [ NotifyRead, "" ];
+ };
+
+ State {
+ default_state with
+ name = "CHECK_REPLY";
+ comment = "Check newstyle NBD_OPT_EXTENDED_HEADERS option reply";
+ external_events = [];
+ };
+]
+
(* Fixed newstyle NBD_OPT_STRUCTURED_REPLY option.
* Implementation: generator/states-newstyle-opt-structured-reply.c
*)
diff --git a/generator/states-newstyle-opt-extended-headers.c
b/generator/states-newstyle-opt-extended-headers.c
new file mode 100644
index 00000000..1ec25e97
--- /dev/null
+++ b/generator/states-newstyle-opt-extended-headers.c
@@ -0,0 +1,94 @@
+/* nbd client library in userspace: state machine
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* State machine for negotiating NBD_OPT_EXTENDED_HEADERS. */
+
+STATE_MACHINE {
+ NEWSTYLE.OPT_EXTENDED_HEADERS.START:
+ assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
+ assert (h->opt_current != NBD_OPT_EXTENDED_HEADERS);
+ assert (CALLBACK_IS_NULL (h->opt_cb.completion));
+ if (!h->request_eh || !h->request_sr) {
+ SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ return 0;
+ }
+
+ h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
+ h->sbuf.option.option = htobe32 (NBD_OPT_EXTENDED_HEADERS);
+ h->sbuf.option.optlen = htobe32 (0);
+ h->chunks_sent++;
+ h->wbuf = &h->sbuf;
+ h->wlen = sizeof h->sbuf.option;
+ SET_NEXT_STATE (%SEND);
+ return 0;
+
+ NEWSTYLE.OPT_EXTENDED_HEADERS.SEND:
+ switch (send_from_wbuf (h)) {
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
+ case 0:
+ h->rbuf = &h->sbuf;
+ h->rlen = sizeof h->sbuf.or.option_reply;
+ SET_NEXT_STATE (%RECV_REPLY);
+ }
+ return 0;
+
+ NEWSTYLE.OPT_EXTENDED_HEADERS.RECV_REPLY:
+ switch (recv_into_rbuf (h)) {
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
+ case 0:
+ if (prepare_for_reply_payload (h, NBD_OPT_EXTENDED_HEADERS) == -1) {
+ SET_NEXT_STATE (%.DEAD);
+ return 0;
+ }
+ SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
+ }
+ return 0;
+
+ NEWSTYLE.OPT_EXTENDED_HEADERS.RECV_REPLY_PAYLOAD:
+ switch (recv_into_rbuf (h)) {
+ case -1: SET_NEXT_STATE (%.DEAD); return 0;
+ case 0: SET_NEXT_STATE (%CHECK_REPLY);
+ }
+ return 0;
+
+ NEWSTYLE.OPT_EXTENDED_HEADERS.CHECK_REPLY:
+ uint32_t reply;
+
+ reply = be32toh (h->sbuf.or.option_reply.reply);
+ switch (reply) {
+ case NBD_REP_ACK:
+ debug (h, "negotiated extended headers on this connection");
+ h->extended_headers = true;
+ /* Extended headers trump structured replies, so skip ahead. */
+ h->structured_replies = true;
+ break;
+ default:
+ if (handle_reply_error (h) == -1) {
+ SET_NEXT_STATE (%.DEAD);
+ return 0;
+ }
+
+ debug (h, "extended headers are not supported by this server");
+ break;
+ }
+
+ /* Next option. */
+ SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ return 0;
+
+} /* END STATE MACHINE */
diff --git a/generator/states-newstyle-opt-starttls.c
b/generator/states-newstyle-opt-starttls.c
index e497548c..1e2997a3 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -26,7 +26,7 @@ NEWSTYLE.OPT_STARTTLS.START:
else {
/* If TLS was not requested we skip this option and go to the next one. */
if (h->tls == LIBNBD_TLS_DISABLE) {
- SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START);
return 0;
}
assert (CALLBACK_IS_NULL (h->opt_cb.completion));
@@ -128,7 +128,7 @@ NEWSTYLE.OPT_STARTTLS.CHECK_REPLY:
SET_NEXT_STATE (%.NEGOTIATING);
else {
debug (h, "continuing with unencrypted connection");
- SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START);
}
return 0;
}
@@ -185,7 +185,7 @@ NEWSTYLE.OPT_STARTTLS.TLS_HANDSHAKE_DONE:
if (h->opt_current == NBD_OPT_STARTTLS)
SET_NEXT_STATE (%.NEGOTIATING);
else
- SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START);
return 0;
} /* END STATE MACHINE */
diff --git a/generator/Makefile.am b/generator/Makefile.am
index c3d53b26..c8477842 100644
--- a/generator/Makefile.am
+++ b/generator/Makefile.am
@@ -30,6 +30,7 @@ states_code = \
states-issue-command.c \
states-magic.c \
states-newstyle-opt-export-name.c \
+ states-newstyle-opt-extended-headers.c \
states-newstyle-opt-list.c \
states-newstyle-opt-go.c \
states-newstyle-opt-meta-context.c \
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 21/25] api: Add nbd_[aio_]opt_extended_headers()
Very similar to the recent addition of nbd_opt_structured_reply,
giving us fine-grained control over an extended headers request.
Because nbdkit does not yet support extended headers, testsuite
coverage is limited to interop testing with qemu-nbd. It shows that
extended headers imply structured replies, regardless of which order
the two are manually negotiated in.
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: no change
---
generator/API.ml | 79 +++++++--
generator/states-newstyle.c | 3 +
.../states-newstyle-opt-extended-headers.c | 30 +++-
lib/opt.c | 44 +++++
interop/Makefile.am | 6 +
interop/opt-extended-headers.c | 153 ++++++++++++++++++
interop/opt-extended-headers.sh | 29 ++++
.gitignore | 1 +
8 files changed, 329 insertions(+), 16 deletions(-)
create mode 100644 interop/opt-extended-headers.c
create mode 100755 interop/opt-extended-headers.sh
diff --git a/generator/API.ml b/generator/API.ml
index d1849710..a02a8a2d 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -825,6 +825,7 @@ "set_request_extended_headers", {
if L<nbd_set_request_structured_replies(3)> is also set to false,
since the use of extended headers implies structured replies.";
see_also = [Link "get_request_extended_headers";
+ Link "opt_extended_headers";
Link "set_handshake_flags"; Link
"set_strict_mode";
Link "get_extended_headers_negotiated";
Link "zero"; Link "trim"; Link
"cache";
@@ -856,7 +857,9 @@ "get_extended_headers_negotiated", {
shortdesc = "see if extended headers are in use";
longdesc = "\
After connecting you may call this to find out if the connection is
-using extended headers.
+using extended headers. Note that this setting is sticky; this
+can return true even after a second L<nbd_opt_extended_headers(3)>
+returns false because the server detected a duplicate request.
When extended headers are not in use, commands are limited to a
32-bit length, even when the libnbd API uses a 64-bit parameter
@@ -938,7 +941,7 @@ "get_structured_replies_negotiated", {
attempted.";
see_also = [Link "set_request_structured_replies";
Link "get_request_structured_replies";
- Link "opt_structured_reply";
+ Link "opt_structured_reply"; Link
"opt_extended_headers";
Link "get_protocol";
Link "get_extended_headers_negotiated"];
};
@@ -1211,12 +1214,13 @@ "set_opt_mode", {
newstyle server. This setting has no effect when connecting to an
oldstyle server.
-Note that libnbd defaults to attempting C<NBD_OPT_STARTTLS> and
-C<NBD_OPT_STRUCTURED_REPLY> before letting you control remaining
-negotiation steps; if you need control over these steps as well,
-first set L<nbd_set_tls(3)> to C<LIBNBD_TLS_DISABLE> and
-L<nbd_set_request_structured_replies(3)> to false before starting
-the connection attempt.
+Note that libnbd defaults to attempting C<NBD_OPT_STARTTLS>,
+C<NBD_OPT_EXTENDED_HEADERS>, and C<NBD_OPT_STRUCTURED_REPLY>
+before letting you control remaining negotiation steps; if you
+need control over these steps as well, first set L<nbd_set_tls(3)>
+to C<LIBNBD_TLS_DISABLE>, and
L<nbd_set_request_extended_headers(3)>
+or L<nbd_set_request_structured_replies(3)> to false, before
+starting the connection attempt.
When option mode is enabled, you have fine-grained control over which
options are negotiated, compared to the default of the server
@@ -1324,6 +1328,35 @@ "opt_starttls", {
Link "supports_tls"]
};
+ "opt_extended_headers", {
+ default_call with
+ args = []; ret = RBool;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server to enable extended headers";
+ longdesc = "\
+Request that the server use extended headers, by sending
+C<NBD_OPT_EXTENDED_HEADERS>. This can only be used if
+L<nbd_set_opt_mode(3)> enabled option mode; furthermore, libnbd
+defaults to automatically requesting this unless you use
+L<nbd_set_request_extended_headers(3)> or
+L<nbd_set_request_structured_replies(3)> prior to connecting.
+This function is mainly useful for integration testing of corner
+cases in server handling.
+
+This function returns true if the server replies with success,
+false if the server replies with an error, and fails only if
+the server does not reply (such as for a loss of connection).
+Note that some servers fail a second request as redundant;
+libnbd assumes that once one request has succeeded, then
+extended headers are supported (as visible by
+L<nbd_get_extended_headers_negotiated(3)>) regardless if
+later calls to this function return false. If this function
+returns true, the use of structured replies is implied.";
+ see_also = [Link "set_opt_mode"; Link
"aio_opt_extended_headers";
+ Link "opt_go"; Link
"set_request_extended_headers";
+ Link "set_request_structured_replies"]
+ };
+
"opt_structured_reply", {
default_call with
args = []; ret = RBool;
@@ -1345,7 +1378,9 @@ "opt_structured_reply", {
libnbd assumes that once one request has succeeded, then
structured replies are supported (as visible by
L<nbd_get_structured_replies_negotiated(3)>) regardless if
-later calls to this function return false.";
+later calls to this function return false. Similarly, a
+server may fail this request if extended headers are already
+negotiated, since extended headers take priority.";
see_also = [Link "set_opt_mode"; Link
"aio_opt_structured_reply";
Link "opt_go"; Link
"set_request_structured_replies"]
};
@@ -3146,6 +3181,30 @@ "aio_opt_starttls", {
see_also = [Link "set_opt_mode"; Link "opt_starttls"];
};
+ "aio_opt_extended_headers", {
+ default_call with
+ args = [];
+ optargs = [ OClosure completion_closure ];
+ ret = RErr;
+ permitted_states = [ Negotiating ];
+ shortdesc = "request the server to enable extended headers";
+ longdesc = "\
+Request that the server use extended headers, by sending
+C<NBD_OPT_EXTENDED_HEADERS>. This behaves like the synchronous
+counterpart L<nbd_opt_extended_headers(3)>, except that it does
+not wait for the server's response.
+
+To determine when the request completes, wait for
+L<nbd_aio_is_connecting(3)> to return false. Or supply the optional
+C<completion_callback> which will be invoked as described in
+L<libnbd(3)/Completion callbacks>, except that it is automatically
+retired regardless of return value. Note that detecting whether the
+server returns an error (as is done by the return value of the
+synchronous counterpart) is only possible with a completion
+callback.";
+ see_also = [Link "set_opt_mode"; Link
"opt_extended_headers"];
+ };
+
"aio_opt_structured_reply", {
default_call with
args = [];
@@ -4122,6 +4181,8 @@ let first_version
"set_request_extended_headers", (1, 18);
"get_request_extended_headers", (1, 18);
"get_extended_headers_negotiated", (1, 18);
+ "opt_extended_headers", (1, 18);
+ "aio_opt_extended_headers", (1, 18);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index ad5bbf72..45893a8b 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -146,6 +146,9 @@ NEWSTYLE.START:
case NBD_OPT_STRUCTURED_REPLY:
SET_NEXT_STATE (%OPT_STRUCTURED_REPLY.START);
return 0;
+ case NBD_OPT_EXTENDED_HEADERS:
+ SET_NEXT_STATE (%OPT_EXTENDED_HEADERS.START);
+ return 0;
case NBD_OPT_STARTTLS:
SET_NEXT_STATE (%OPT_STARTTLS.START);
return 0;
diff --git a/generator/states-newstyle-opt-extended-headers.c
b/generator/states-newstyle-opt-extended-headers.c
index 1ec25e97..5017a629 100644
--- a/generator/states-newstyle-opt-extended-headers.c
+++ b/generator/states-newstyle-opt-extended-headers.c
@@ -21,11 +21,14 @@
STATE_MACHINE {
NEWSTYLE.OPT_EXTENDED_HEADERS.START:
assert (h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE);
- assert (h->opt_current != NBD_OPT_EXTENDED_HEADERS);
- assert (CALLBACK_IS_NULL (h->opt_cb.completion));
- if (!h->request_eh || !h->request_sr) {
- SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
- return 0;
+ if (h->opt_current == NBD_OPT_EXTENDED_HEADERS)
+ assert (h->opt_mode);
+ else {
+ assert (CALLBACK_IS_NULL (h->opt_cb.completion));
+ if (!h->request_eh || !h->request_sr) {
+ SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ return 0;
+ }
}
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
@@ -68,6 +71,7 @@ NEWSTYLE.OPT_EXTENDED_HEADERS.RECV_REPLY_PAYLOAD:
NEWSTYLE.OPT_EXTENDED_HEADERS.CHECK_REPLY:
uint32_t reply;
+ int err = ENOTSUP;
reply = be32toh (h->sbuf.or.option_reply.reply);
switch (reply) {
@@ -76,19 +80,31 @@ NEWSTYLE.OPT_EXTENDED_HEADERS.CHECK_REPLY:
h->extended_headers = true;
/* Extended headers trump structured replies, so skip ahead. */
h->structured_replies = true;
+ err = 0;
break;
+ case NBD_REP_ERR_INVALID:
+ err = EINVAL;
+ /* fallthrough */
default:
if (handle_reply_error (h) == -1) {
SET_NEXT_STATE (%.DEAD);
return 0;
}
- debug (h, "extended headers are not supported by this server");
+ if (h->extended_headers)
+ debug (h, "extended headers already negotiated");
+ else
+ debug (h, "extended headers are not supported by this server");
break;
}
/* Next option. */
- SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ if (h->opt_current == NBD_OPT_EXTENDED_HEADERS)
+ SET_NEXT_STATE (%.NEGOTIATING);
+ else
+ SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START);
+ CALL_CALLBACK (h->opt_cb.completion, &err);
+ nbd_internal_free_option (h);
return 0;
} /* END STATE MACHINE */
diff --git a/lib/opt.c b/lib/opt.c
index 1446eef3..dd0bed96 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -164,6 +164,31 @@ nbd_unlocked_opt_starttls (struct nbd_handle *h)
return r;
}
+/* Issue NBD_OPT_EXTENDED_HEADERS and wait for the reply. */
+int
+nbd_unlocked_opt_extended_headers (struct nbd_handle *h)
+{
+ int err;
+ nbd_completion_callback c = { .callback = go_complete, .user_data = &err
};
+ int r = nbd_unlocked_aio_opt_extended_headers (h, &c);
+
+ if (r == -1)
+ return r;
+
+ r = wait_for_option (h);
+ if (r == 0) {
+ if (nbd_internal_is_state_negotiating (get_next_state (h)))
+ r = err == 0;
+ else {
+ assert (nbd_internal_is_state_dead (get_next_state (h)));
+ set_error (err,
+ "failed to get response to opt_extended_headers
request");
+ r = -1;
+ }
+ }
+ return r;
+}
+
/* Issue NBD_OPT_STRUCTURED_REPLY and wait for the reply. */
int
nbd_unlocked_opt_structured_reply (struct nbd_handle *h)
@@ -388,6 +413,25 @@ nbd_unlocked_aio_opt_starttls (struct nbd_handle *h,
#endif
}
+/* Issue NBD_OPT_EXTENDED_HEADERS without waiting. */
+int
+nbd_unlocked_aio_opt_extended_headers (struct nbd_handle *h,
+ nbd_completion_callback *complete)
+{
+ if ((h->gflags & LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE) == 0) {
+ set_error (ENOTSUP, "server is not using fixed newstyle
protocol");
+ return -1;
+ }
+
+ h->opt_current = NBD_OPT_EXTENDED_HEADERS;
+ h->opt_cb.completion = *complete;
+ SET_CALLBACK_TO_NULL (*complete);
+
+ if (nbd_internal_run (h, cmd_issue) == -1)
+ debug (h, "option queued, ignoring state machine failure");
+ return 0;
+}
+
/* Issue NBD_OPT_STRUCTURED_REPLY without waiting. */
int
nbd_unlocked_aio_opt_structured_reply (struct nbd_handle *h,
diff --git a/interop/Makefile.am b/interop/Makefile.am
index ec8ea0b2..3f81df0c 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -25,6 +25,7 @@ EXTRA_DIST = \
list-exports-test-dir/disk1 \
list-exports-test-dir/disk2 \
structured-read.sh \
+ opt-extended-headers.sh \
$(NULL)
TESTS_ENVIRONMENT = \
@@ -134,6 +135,7 @@ check_PROGRAMS += \
socket-activation-qemu-nbd \
dirty-bitmap \
structured-read \
+ opt-extended-headers \
$(NULL)
TESTS += \
interop-qemu-nbd \
@@ -144,6 +146,7 @@ TESTS += \
dirty-bitmap.sh \
structured-read.sh \
interop-qemu-block-size.sh \
+ opt-extended-headers.sh \
$(NULL)
interop_qemu_nbd_SOURCES = \
@@ -235,6 +238,9 @@ dirty_bitmap_LDADD = $(top_builddir)/lib/libnbd.la
structured_read_SOURCES = structured-read.c
structured_read_LDADD = $(top_builddir)/lib/libnbd.la
+opt_extended_headers_SOURCES = opt-extended-headers.c
+opt_extended_headers_LDADD = $(top_builddir)/lib/libnbd.la
+
endif HAVE_QEMU_NBD
#----------------------------------------------------------------------
diff --git a/interop/opt-extended-headers.c b/interop/opt-extended-headers.c
new file mode 100644
index 00000000..f50cd78f
--- /dev/null
+++ b/interop/opt-extended-headers.c
@@ -0,0 +1,153 @@
+/* NBD client library in userspace
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Demonstrate low-level use of nbd_opt_extended_headers(). */
+
+#include <config.h>
+
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/stat.h>
+
+#include <libnbd.h>
+
+#define check(got, exp) do_check (#got, got, exp)
+
+static void
+do_check (const char *act, int64_t got, int64_t exp)
+{
+ fprintf (stderr, "trying %s\n", act);
+ if (got == -1)
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ else
+ fprintf (stderr, "succeeded, result %" PRId64 "\n",
got);
+ if (got != exp) {
+ fprintf (stderr, "got %" PRId64 ", but expected %"
PRId64 "\n", got, exp);
+ exit (EXIT_FAILURE);
+ }
+}
+
+static int
+cb (void *data, const char *metacontext, uint64_t offset,
+ nbd_extent *entries, size_t nr_entries, int *error)
+{
+ /* If we got here, extents worked, implying at least structured replies */
+ bool *seen = data;
+
+ *seen = true;
+ return 0;
+}
+
+struct nbd_handle *
+prep (bool sr, bool eh, char **cmd)
+{
+ struct nbd_handle *nbd;
+
+ nbd = nbd_create ();
+ if (nbd == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ /* Connect to the server in opt mode, disable client-side failsafes so
+ * that we are testing server response even when client breaks protocol.
+ */
+ check (nbd_set_opt_mode (nbd, true), 0);
+ check (nbd_set_strict_mode (nbd, 0), 0);
+ check (nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION), 0);
+ check (nbd_set_request_structured_replies (nbd, sr), 0);
+ check (nbd_set_request_extended_headers (nbd, eh), 0);
+ check (nbd_connect_systemd_socket_activation (nbd, cmd), 0);
+
+ return nbd;
+}
+
+void
+cleanup (struct nbd_handle *nbd, bool extents_exp)
+{
+ bool extents = false;
+
+ check (nbd_opt_go (nbd), 0);
+ check (nbd_can_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION),
+ extents_exp);
+ check (nbd_block_status_64 (nbd, 512, 0,
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data =
&extents },
+ 0), extents_exp ? 0 : -1);
+ check (extents, extents_exp);
+ nbd_close (nbd);
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int64_t bytes_sent;
+
+ if (argc < 2) {
+ fprintf (stderr, "%s qemu-nbd [args ...]\n", argv[0]);
+ exit (EXIT_FAILURE);
+ }
+
+ /* Default setup tries eh first, and skips sr request when eh works... */
+ nbd = prep (true, true, &argv[1]);
+ bytes_sent = nbd_stats_bytes_sent (nbd);
+ check (nbd_get_extended_headers_negotiated (nbd), true);
+ check (nbd_get_structured_replies_negotiated (nbd), true);
+ /* Duplicate eh request is no-op as redundant, but does not change state */
+ check (nbd_opt_extended_headers (nbd), false);
+ /* Trying sr after eh is no-op as redundant, but does not change state */
+ check (nbd_opt_structured_reply (nbd), false);
+ check (nbd_get_extended_headers_negotiated (nbd), true);
+ check (nbd_get_structured_replies_negotiated (nbd), true);
+ cleanup (nbd, true);
+
+ /* ...which should result in the same amount of initial negotiation
+ * traffic as explicitly requesting just structured replies, albeit
+ * with different results on what got negotiated.
+ */
+ nbd = prep (true, false, &argv[1]);
+ check (nbd_stats_bytes_sent (nbd), bytes_sent);
+ check (nbd_get_extended_headers_negotiated (nbd), false);
+ check (nbd_get_structured_replies_negotiated (nbd), true);
+ cleanup (nbd, true);
+
+ /* request_eh is ignored if request_sr is false. */
+ nbd = prep (false, true, &argv[1]);
+ check (nbd_get_extended_headers_negotiated (nbd), false);
+ check (nbd_get_structured_replies_negotiated (nbd), false);
+ cleanup (nbd, false);
+
+ /* Swap order, requesting structured replies before extended headers */
+ nbd = prep (false, false, &argv[1]);
+ check (nbd_get_extended_headers_negotiated (nbd), false);
+ check (nbd_get_structured_replies_negotiated (nbd), false);
+ check (nbd_opt_structured_reply (nbd), true);
+ check (nbd_get_extended_headers_negotiated (nbd), false);
+ check (nbd_get_structured_replies_negotiated (nbd), true);
+ check (nbd_opt_extended_headers (nbd), true);
+ check (nbd_get_extended_headers_negotiated (nbd), true);
+ check (nbd_get_structured_replies_negotiated (nbd), true);
+ cleanup (nbd, true);
+
+ exit (EXIT_SUCCESS);
+}
diff --git a/interop/opt-extended-headers.sh b/interop/opt-extended-headers.sh
new file mode 100755
index 00000000..41322f36
--- /dev/null
+++ b/interop/opt-extended-headers.sh
@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright Red Hat
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# Test low-level nbd_opt_extended_headers() details with qemu-nbd
+
+source ../tests/functions.sh
+set -e
+set -x
+
+requires qemu-nbd --version
+requires nbdinfo --can extended-headers -- [ qemu-nbd -r -f raw "$0"
]
+
+# Run the test.
+$VG ./opt-extended-headers qemu-nbd -r -f raw "$0"
diff --git a/.gitignore b/.gitignore
index e5304f16..981b3bda 100644
--- a/.gitignore
+++ b/.gitignore
@@ -119,6 +119,7 @@ Makefile.in
/interop/list-exports-nbdkit
/interop/list-exports-qemu-nbd
/interop/nbd-server-tls.conf
+/interop/opt-extended-headers
/interop/requires.c
/interop/socket-activation-nbdkit
/interop/socket-activation-qemu-nbd
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 22/25] interop: Add test of 64-bit block status
Prove that we can round-trip a block status request larger than 4G
through a new-enough qemu-nbd. Also serves as a unit test of our shim
for converting internal 64-bit representation back to the older 32-bit
nbd_block_status callback interface.
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: s/large-status/block-status-64/ [Rich]
---
interop/Makefile.am | 6 ++
interop/block-status-64.c | 186 +++++++++++++++++++++++++++++++++++++
interop/block-status-64.sh | 49 ++++++++++
.gitignore | 1 +
4 files changed, 242 insertions(+)
create mode 100644 interop/block-status-64.c
create mode 100755 interop/block-status-64.sh
diff --git a/interop/Makefile.am b/interop/Makefile.am
index 3f81df0c..3947ce3d 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -21,6 +21,7 @@ EXTRA_DIST = \
dirty-bitmap.sh \
interop-qemu-storage-daemon.sh \
interop-qemu-block-size.sh \
+ block-status-64.sh \
list-exports-nbd-config \
list-exports-test-dir/disk1 \
list-exports-test-dir/disk2 \
@@ -134,6 +135,7 @@ check_PROGRAMS += \
list-exports-qemu-nbd \
socket-activation-qemu-nbd \
dirty-bitmap \
+ block-status-64 \
structured-read \
opt-extended-headers \
$(NULL)
@@ -144,6 +146,7 @@ TESTS += \
list-exports-qemu-nbd \
socket-activation-qemu-nbd \
dirty-bitmap.sh \
+ block-status-64.sh \
structured-read.sh \
interop-qemu-block-size.sh \
opt-extended-headers.sh \
@@ -235,6 +238,9 @@ socket_activation_qemu_nbd_LDADD =
$(top_builddir)/lib/libnbd.la
dirty_bitmap_SOURCES = dirty-bitmap.c
dirty_bitmap_LDADD = $(top_builddir)/lib/libnbd.la
+block_status_64_SOURCES = block-status-64.c
+block_status_64_LDADD = $(top_builddir)/lib/libnbd.la
+
structured_read_SOURCES = structured-read.c
structured_read_LDADD = $(top_builddir)/lib/libnbd.la
diff --git a/interop/block-status-64.c b/interop/block-status-64.c
new file mode 100644
index 00000000..36415653
--- /dev/null
+++ b/interop/block-status-64.c
@@ -0,0 +1,186 @@
+/* NBD client library in userspace
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Test 64-bit block status with qemu. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <assert.h>
+#include <stdbool.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+static const char *bitmap;
+
+struct data {
+ bool req_one; /* input: true if req_one was passed to request */
+ int count; /* input: count of expected remaining calls */
+ bool seen_base; /* output: true if base:allocation encountered */
+ bool seen_dirty; /* output: true if qemu:dirty-bitmap encountered */
+};
+
+static int
+cb32 (void *opaque, const char *metacontext, uint64_t offset,
+ uint32_t *entries, size_t len, int *error)
+{
+ struct data *data = opaque;
+
+ assert (offset == 0);
+ assert (data->count-- > 0);
+
+ if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
+ assert (!data->seen_base);
+ data->seen_base = true;
+
+ /* Data block offset 0 size 64k, remainder is hole */
+ assert (len == 4);
+ assert (entries[0] == 65536);
+ assert (entries[1] == 0);
+ /* libnbd had to truncate qemu's >4G answer */
+ assert (entries[2] == 4227858432);
+ assert (entries[3] == (LIBNBD_STATE_HOLE|LIBNBD_STATE_ZERO));
+ }
+ else if (strcmp (metacontext, bitmap) == 0) {
+ assert (!data->seen_dirty);
+ data->seen_dirty = true;
+
+ /* Dirty block at offset 5G-64k, remainder is clean */
+ /* libnbd had to truncate qemu's >4G answer */
+ assert (len == 2);
+ assert (entries[0] == 4227858432);
+ assert (entries[1] == 0);
+ }
+ else {
+ fprintf (stderr, "unexpected context %s\n", metacontext);
+ exit (EXIT_FAILURE);
+ }
+ return 0;
+}
+
+static int
+cb64 (void *opaque, const char *metacontext, uint64_t offset,
+ nbd_extent *entries, size_t len, int *error)
+{
+ struct data *data = opaque;
+
+ assert (offset == 0);
+ assert (data->count-- > 0);
+
+ if (strcmp (metacontext, LIBNBD_CONTEXT_BASE_ALLOCATION) == 0) {
+ assert (!data->seen_base);
+ data->seen_base = true;
+
+ /* Data block offset 0 size 64k, remainder is hole */
+ assert (len == 2);
+ assert (entries[0].length == 65536);
+ assert (entries[0].flags == 0);
+ assert (entries[1].length == 5368643584ULL);
+ assert (entries[1].flags == (LIBNBD_STATE_HOLE|LIBNBD_STATE_ZERO));
+ }
+ else if (strcmp (metacontext, bitmap) == 0) {
+ assert (!data->seen_dirty);
+ data->seen_dirty = true;
+
+ /* Dirty block at offset 5G-64k, remainder is clean */
+ assert (len == 2);
+ assert (entries[0].length == 5368643584ULL);
+ assert (entries[0].flags == 0);
+ assert (entries[1].length == 65536);
+ assert (entries[1].flags == 1);
+ }
+ else {
+ fprintf (stderr, "unexpected context %s\n", metacontext);
+ exit (EXIT_FAILURE);
+ }
+ return 0;
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int64_t exportsize;
+ struct data data;
+
+ if (argc < 3) {
+ fprintf (stderr, "%s bitmap qemu-nbd [args ...]\n", argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ bitmap = argv[1];
+
+ nbd = nbd_create ();
+ if (nbd == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_add_meta_context (nbd, LIBNBD_CONTEXT_BASE_ALLOCATION);
+ nbd_add_meta_context (nbd, bitmap);
+
+ if (nbd_connect_systemd_socket_activation (nbd, &argv[2]) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ exportsize = nbd_get_size (nbd);
+ if (exportsize == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ if (nbd_get_extended_headers_negotiated (nbd) != 1) {
+ fprintf (stderr, "skipping: qemu-nbd lacks extended headers\n");
+ exit (77);
+ }
+
+ /* Prove that we can round-trip a >4G block status request */
+ data = (struct data) { .count = 2, };
+ if (nbd_block_status_64 (nbd, exportsize, 0,
+ (nbd_extent64_callback) { .callback = cb64,
+ .user_data = &data },
+ 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ assert (data.seen_base && data.seen_dirty);
+
+ /* Check libnbd's handling of a >4G response through older interface
*/
+ data = (struct data) { .count = 2, };
+ if (nbd_block_status (nbd, exportsize, 0,
+ (nbd_extent_callback) { .callback = cb32,
+ .user_data = &data },
+ 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ assert (data.seen_base && data.seen_dirty);
+
+ if (nbd_shutdown (nbd, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_close (nbd);
+
+ exit (EXIT_SUCCESS);
+}
diff --git a/interop/block-status-64.sh b/interop/block-status-64.sh
new file mode 100755
index 00000000..46810dc3
--- /dev/null
+++ b/interop/block-status-64.sh
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright Red Hat
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# Test reading qemu dirty-bitmap.
+
+source ../tests/functions.sh
+set -e
+set -x
+
+requires qemu-img bitmap --help
+requires qemu-nbd --version
+
+# This test uses the qemu-nbd -B option.
+if ! qemu-nbd --help | grep -sq -- -B; then
+ echo "$0: skipping because qemu-nbd does not support the -B
option"
+ exit 77
+fi
+
+files="large-status.qcow2"
+rm -f $files
+cleanup_fn rm -f $files
+
+# Create mostly-sparse file with intentionally different data vs. dirty areas
+# (64k data, 5G-64k hole,zero; 5G-64k clean, 64k dirty)
+qemu-img create -f qcow2 large-status.qcow2 5G
+qemu-img bitmap --add --enable -f qcow2 large-status.qcow2 bitmap0
+qemu-io -f qcow2 -c "w -z $((5*1024*1024*1024 - 64*1024)) 64k" \
+ large-status.qcow2
+qemu-img bitmap --disable -f qcow2 large-status.qcow2 bitmap0
+qemu-io -f qcow2 -c 'w 0 64k' large-status.qcow2
+
+# Run the test.
+$VG ./large-status qemu:dirty-bitmap:bitmap0 \
+ qemu-nbd -f qcow2 -B bitmap0 large-status.qcow2
diff --git a/.gitignore b/.gitignore
index 981b3bda..c0761b25 100644
--- a/.gitignore
+++ b/.gitignore
@@ -102,6 +102,7 @@ Makefile.in
/info/nbdinfo
/info/nbdinfo.1
/install-sh
+/interop/block-status-64
/interop/dirty-bitmap
/interop/interop-nbd-server
/interop/interop-nbd-server-tls
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 23/25] api: Add nbd_can_block_status_payload()
In the recent NBD protocol extensions to add 64-bit commands [1], an
additional option was added to allow NBD_CMD_BLOCK_STATUS to pass a
client payload instructing the server to filter its answers in nbd.git
commit e6f3b94a (mainly useful when the client requests more than one
meta context with NBD_OPT_SET_META_CONTEXT). This patch lays the
groundwork by exposing servers that advertise this capability,
although libnbd does not yet actually utilize it until the next patch.
At the time this patch was written, qemu-nbd was also patched to
provide such support; hence, an interop/ test shows the API in action.
[1] https://github.com/NetworkBlockDevice/nbd/blob/extension-ext-header/doc/
Signed-off-by: Eric Blake <eblake at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: rebase to earlier changes, s/can/has/ [Rich]
---
info/nbdinfo.pod | 8 ++
lib/nbd-protocol.h | 7 ++
generator/API.ml | 18 +++++
lib/flags.c | 12 +++
examples/server-flags.c | 7 +-
interop/Makefile.am | 6 ++
interop/block-status-payload.c | 126 ++++++++++++++++++++++++++++++++
interop/block-status-payload.sh | 68 +++++++++++++++++
.gitignore | 1 +
info/can.c | 5 ++
info/show.c | 9 ++-
11 files changed, 265 insertions(+), 2 deletions(-)
create mode 100644 interop/block-status-payload.c
create mode 100755 interop/block-status-payload.sh
diff --git a/info/nbdinfo.pod b/info/nbdinfo.pod
index 2bdd573f..9b1962da 100644
--- a/info/nbdinfo.pod
+++ b/info/nbdinfo.pod
@@ -171,6 +171,11 @@ for supporting block status commands).
Test if server supports extended headers (a prerequisite for
supporting 64-bit commands; implies structured replies as well).
+=item nbdinfo --has block-status-payload URI
+
+Test if server has support for passing a client payload to limit the
+response to a block status command.
+
=item nbdinfo --is rotational URI
Test if the server export is backed by something which behaves like a
@@ -373,6 +378,8 @@ When using I<--list>, the default is
I<--no-content> (since
downloading from each export is expensive). To enable content probing
use I<--list --content>.
+=item B<--has block-status-payload>
+
=item B<--has extended-headers>
=item B<--has structured-reply>
@@ -385,6 +392,7 @@ indicate an error querying the flag).
For further information see the L<NBD
protocol|https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md>
and the following libnbd functions:
+L<nbd_can_block_status_payload(3)>,
L<nbd_get_extended_headers_negotiated(3)>,
L<nbd_get_structured_replies_negotiated(3)>.
diff --git a/lib/nbd-protocol.h b/lib/nbd-protocol.h
index b5a28ae4..e5288a17 100644
--- a/lib/nbd-protocol.h
+++ b/lib/nbd-protocol.h
@@ -113,6 +113,7 @@ struct nbd_fixed_new_option_reply {
#define NBD_FLAG_CAN_MULTI_CONN (1U << 8)
#define NBD_FLAG_SEND_CACHE (1U << 10)
#define NBD_FLAG_SEND_FAST_ZERO (1U << 11)
+#define NBD_FLAG_BLOCK_STATUS_PAYLOAD (1U << 12)
/* NBD options (new style handshake only). */
#define NBD_OPT_EXPORT_NAME 1
@@ -204,6 +205,12 @@ struct nbd_request_ext {
uint64_t count; /* Request effect or payload length. */
} NBD_ATTRIBUTE_PACKED;
+/* Extended request payload for NBD_CMD_BLOCK_STATUS, when supported. */
+struct nbd_block_status_payload {
+ uint64_t length; /* Effective length of client request */
+ /* followed by array of uint32_t ids */
+} NBD_ATTRIBUTE_PACKED;
+
/* Simple reply (server -> client). */
struct nbd_simple_reply {
uint32_t magic; /* NBD_SIMPLE_REPLY_MAGIC. */
diff --git a/generator/API.ml b/generator/API.ml
index a02a8a2d..6228a4e5 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -2327,6 +2327,23 @@ "can_fast_zero", {
example = Some "examples/server-flags.c";
};
+ "can_block_status_payload", {
+ default_call with
+ args = []; ret = RBool;
+ permitted_states = [ Negotiating; Connected; Closed ];
+ shortdesc = "does the server support the block status payload
flag?";
+ longdesc = "\
+Returns true if the server supports the use of the
+C<LIBNBD_CMD_FLAG_PAYLOAD_LEN> flag to allow filtering of the
+block status command. Returns
+false if the server does not. Note that this will never return
+true if L<nbd_get_extended_headers_negotiated(3)> is false."
+^ non_blocking_test_call_description;
+ see_also = [SectionLink "Flag calls"; Link "opt_info";
+ Link "get_extended_headers_negotiated"];
+ example = Some "examples/server-flags.c";
+ };
+
"can_df", {
default_call with
args = []; ret = RBool;
@@ -4183,6 +4200,7 @@ let first_version
"get_extended_headers_negotiated", (1, 18);
"opt_extended_headers", (1, 18);
"aio_opt_extended_headers", (1, 18);
+ "can_block_status_payload", (1, 18);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/lib/flags.c b/lib/flags.c
index be880acf..7e6ddedd 100644
--- a/lib/flags.c
+++ b/lib/flags.c
@@ -66,6 +66,12 @@ nbd_internal_set_size_and_flags (struct nbd_handle *h,
eflags &= ~NBD_FLAG_SEND_DF;
}
+ if (eflags & NBD_FLAG_BLOCK_STATUS_PAYLOAD &&
!h->extended_headers) {
+ debug (h, "server lacks extended headers, ignoring claim "
+ "of block status payload");
+ eflags &= ~NBD_FLAG_BLOCK_STATUS_PAYLOAD;
+ }
+
if (eflags & NBD_FLAG_SEND_FAST_ZERO &&
!(eflags & NBD_FLAG_SEND_WRITE_ZEROES)) {
debug (h, "server lacks write zeroes, ignoring claim of fast
zero");
@@ -213,6 +219,12 @@ nbd_unlocked_can_cache (struct nbd_handle *h)
return get_flag (h, NBD_FLAG_SEND_CACHE);
}
+int
+nbd_unlocked_can_block_status_payload (struct nbd_handle *h)
+{
+ return get_flag (h, NBD_FLAG_BLOCK_STATUS_PAYLOAD);
+}
+
int
nbd_unlocked_can_meta_context (struct nbd_handle *h, const char *name)
{
diff --git a/examples/server-flags.c b/examples/server-flags.c
index d156aced..f53b86ed 100644
--- a/examples/server-flags.c
+++ b/examples/server-flags.c
@@ -78,8 +78,13 @@ main (int argc, char *argv[])
PRINT_FLAG (nbd_can_multi_conn);
PRINT_FLAG (nbd_can_trim);
PRINT_FLAG (nbd_can_zero);
-#if LIBNBD_HAVE_NBD_CAN_FAST_ZERO /* Added in 1.2 */
+#if LIBNBD_HAVE_NBD_CAN_FAST_ZERO
+ /* Added in 1.2 */
PRINT_FLAG (nbd_can_fast_zero);
+#endif
+#if LIBNBD_HAVE_NBD_CAN_BLOCK_STATUS_PAYLOAD
+ /* Added in 1.18 */
+ PRINT_FLAG (nbd_can_block_status_payload);
#endif
PRINT_FLAG (nbd_is_read_only);
PRINT_FLAG (nbd_is_rotational);
diff --git a/interop/Makefile.am b/interop/Makefile.am
index 3947ce3d..d6485adf 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -27,6 +27,7 @@ EXTRA_DIST = \
list-exports-test-dir/disk2 \
structured-read.sh \
opt-extended-headers.sh \
+ block-status-payload.sh \
$(NULL)
TESTS_ENVIRONMENT = \
@@ -138,6 +139,7 @@ check_PROGRAMS += \
block-status-64 \
structured-read \
opt-extended-headers \
+ block-status-payload \
$(NULL)
TESTS += \
interop-qemu-nbd \
@@ -150,6 +152,7 @@ TESTS += \
structured-read.sh \
interop-qemu-block-size.sh \
opt-extended-headers.sh \
+ block-status-payload.sh \
$(NULL)
interop_qemu_nbd_SOURCES = \
@@ -247,6 +250,9 @@ structured_read_LDADD = $(top_builddir)/lib/libnbd.la
opt_extended_headers_SOURCES = opt-extended-headers.c
opt_extended_headers_LDADD = $(top_builddir)/lib/libnbd.la
+block_status_payload_SOURCES = block-status-payload.c
+block_status_payload_LDADD = $(top_builddir)/lib/libnbd.la
+
endif HAVE_QEMU_NBD
#----------------------------------------------------------------------
diff --git a/interop/block-status-payload.c b/interop/block-status-payload.c
new file mode 100644
index 00000000..9603dfe5
--- /dev/null
+++ b/interop/block-status-payload.c
@@ -0,0 +1,126 @@
+/* NBD client library in userspace
+ * Copyright Red Hat
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/* Test interaction with qemu using block status payload filtering. */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <assert.h>
+#include <stdbool.h>
+#include <errno.h>
+
+#include <libnbd.h>
+
+#include "array-size.h"
+
+static const char *contexts[] = {
+ "base:allocation",
+ "qemu:allocation-depth",
+ "qemu:dirty-bitmap:bitmap0",
+ "qemu:dirty-bitmap:bitmap1",
+};
+
+static int
+cb (void *opaque, const char *metacontext, uint64_t offset,
+ nbd_extent *entries, size_t len, int *error)
+{
+ /* Adjust seen according to which context was visited */
+ unsigned int *seen = opaque;
+ size_t i;
+
+ for (i = 0; i < ARRAY_SIZE (contexts); i++)
+ if (strcmp (contexts[i], metacontext) == 0)
+ break;
+ *seen |= 1 << i;
+ return 0;
+}
+
+int
+main (int argc, char *argv[])
+{
+ struct nbd_handle *nbd;
+ int64_t exportsize;
+ unsigned int seen;
+ size_t i;
+ int r;
+
+ if (argc < 2) {
+ fprintf (stderr, "%s qemu-nbd [args ...]\n", argv[0]);
+ exit (EXIT_FAILURE);
+ }
+
+ nbd = nbd_create ();
+ if (nbd == NULL) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ assert (ARRAY_SIZE (contexts) == 4);
+ for (i = 0; i < ARRAY_SIZE (contexts); i++) {
+ if (nbd_add_meta_context (nbd, contexts[i]) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ }
+
+ if (nbd_connect_systemd_socket_activation (nbd, &argv[1]) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ r = nbd_can_block_status_payload (nbd);
+ if (r == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ if (r != 1) {
+ fprintf (stderr, "expecting block status payload support from
qemu\n");
+ exit (EXIT_FAILURE);
+ }
+
+ exportsize = nbd_get_size (nbd);
+ if (exportsize == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ /* An unfiltered call should see all four contexts */
+ seen = 0;
+ if (nbd_block_status_64 (nbd, exportsize, 0,
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data = &seen },
+ 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ assert (seen == 0xf);
+
+ /* FIXME: Test filtered calls once the API is added */
+ if (nbd_shutdown (nbd, 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+
+ nbd_close (nbd);
+
+ exit (EXIT_SUCCESS);
+}
diff --git a/interop/block-status-payload.sh b/interop/block-status-payload.sh
new file mode 100755
index 00000000..a12cfc8a
--- /dev/null
+++ b/interop/block-status-payload.sh
@@ -0,0 +1,68 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright Red Hat
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# Test use of block status payload for server filtering
+
+source ../tests/functions.sh
+set -e
+set -x
+
+requires qemu-img bitmap --help
+# This test uses the qemu-nbd -A and -B options.
+requires qemu-nbd -A -BA --version
+
+file="block-status-payload.qcow2"
+rm -f $file
+cleanup_fn rm -f $file
+
+# Create sparse file with two bitmaps.
+qemu-img create -f qcow2 $file 1M
+qemu-img bitmap --add --enable -f qcow2 $file bitmap0
+qemu-img bitmap --add --enable -f qcow2 $file bitmap1
+
+# Unconditional part of test: qemu should not advertise block status payload
+# support if extended headers are not in use
+nbdsh -c '
+h.set_request_extended_headers(False)
+h.add_meta_context("base:allocation")
+h.add_meta_context("qemu:allocation-depth")
+h.add_meta_context("qemu:dirty-bitmap:bitmap0")
+h.add_meta_context("qemu:dirty-bitmap:bitmap1")
+h.set_opt_mode(True)
+args = ["qemu-nbd", "-f", "qcow2",
"-A", "-B", "bitmap0", "-B",
"bitmap1",
+ "'"$file"'"]
+h.connect_systemd_socket_activation(args)
+assert h.aio_is_negotiating() is True
+assert h.get_extended_headers_negotiated() is False
+# Flag not available until info or go
+try:
+ h.can_block_status_payload()
+ assert False
+except nbd.Error:
+ pass
+h.opt_info()
+assert h.can_block_status_payload() is False
+assert h.can_meta_context("base:allocation") is True
+h.opt_abort()
+'
+
+# Conditional part of test: if qemu is new enough to support extended
+# headers, we assume it can also support block status payload.
+requires nbdinfo --can extended-headers -- [ qemu-nbd -r -f qcow2
"$file" ]
+$VG ./block-status-payload \
+ qemu-nbd -f qcow2 -A -B bitmap0 -B bitmap1 $file
diff --git a/.gitignore b/.gitignore
index c0761b25..cc45d61b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -103,6 +103,7 @@ Makefile.in
/info/nbdinfo.1
/install-sh
/interop/block-status-64
+/interop/block-status-payload
/interop/dirty-bitmap
/interop/interop-nbd-server
/interop/interop-nbd-server-tls
diff --git a/info/can.c b/info/can.c
index 8547ef04..6bb36fb1 100644
--- a/info/can.c
+++ b/info/can.c
@@ -72,6 +72,11 @@ do_can (void)
else if (strcasecmp (can, "rotational") == 0)
feature = nbd_is_rotational (nbd);
+ else if (strcasecmp (can, "block status payload") == 0 ||
+ strcasecmp (can, "block-status-payload") == 0 ||
+ strcasecmp (can, "block_status_payload") == 0)
+ feature = nbd_can_block_status_payload (nbd);
+
else if (strcasecmp (can, "cache") == 0)
feature = nbd_can_cache (nbd);
diff --git a/info/show.c b/info/show.c
index 920bbb0a..8914f927 100644
--- a/info/show.c
+++ b/info/show.c
@@ -54,7 +54,7 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
char *uri = NULL;
int is_rotational, is_read_only;
int can_cache, can_df, can_fast_zero, can_flush, can_fua,
- can_multi_conn, can_trim, can_zero;
+ can_multi_conn, can_trim, can_zero, can_block_status_payload;
int64_t block_minimum, block_preferred, block_maximum;
string_vector contexts = empty_vector;
bool show_context = false;
@@ -120,6 +120,7 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
can_multi_conn = nbd_can_multi_conn (nbd);
can_trim = nbd_can_trim (nbd);
can_zero = nbd_can_zero (nbd);
+ can_block_status_payload = nbd_can_block_status_payload (nbd);
block_minimum = nbd_get_block_size (nbd, LIBNBD_SIZE_MINIMUM);
block_preferred = nbd_get_block_size (nbd, LIBNBD_SIZE_PREFERRED);
block_maximum = nbd_get_block_size (nbd, LIBNBD_SIZE_MAXIMUM);
@@ -161,6 +162,8 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
if (is_read_only >= 0)
fprintf (fp, "\t%s: %s\n", "is_read_only",
is_read_only ? "true" : "false");
+ if (can_block_status_payload >= 0)
+ show_boolean ("can_block_status_payload",
can_block_status_payload);
if (can_cache >= 0)
show_boolean ("can_cache", can_cache);
if (can_df >= 0)
@@ -230,6 +233,10 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
if (is_read_only >= 0)
fprintf (fp, "\t\"%s\": %s,\n",
"is_read_only", is_read_only ? "true" :
"false");
+ if (can_block_status_payload >= 0)
+ fprintf (fp, "\t\"%s\": %s,\n",
+ "can_block_status_payload",
+ can_block_status_payload ? "true" :
"false");
if (can_cache >= 0)
fprintf (fp, "\t\"%s\": %s,\n",
"can_cache", can_cache ? "true" :
"false");
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 24/25] api: Add nbd_[aio_]block_status_filter()
As part of extending NBD to support 64-bit lengths, the protocol also
added an option for servers to allow clients to request filtered
responses to NBD_CMD_BLOCK_STATUS when more than one meta-context is
negotiated (see NBD commit e6f3b94a). At the same time as this patch,
qemu-nbd was taught to support and advertise this feature as a server,
but does not utilize it as a client (qemu doesn't yet need to connect
to multiple contexts at once). Thus, addding generic client support
and enhancing the interop/ test in libnbd is needed to prove that the
feature is viable and worth standardizing.
Signed-off-by: Eric Blake <eblake at redhat.com>
Acked-by: Richard W.M. Jones <rjones at redhat.com>
---
v4: rebase to earlier changes
---
lib/internal.h | 5 +-
generator/API.ml | 71 ++++++++++++++++--
generator/states-issue-command.c | 4 +-
lib/aio.c | 4 +
lib/rw.c | 124 ++++++++++++++++++++++++++++++-
interop/block-status-payload.c | 117 ++++++++++++++++++++++++++++-
interop/block-status-payload.sh | 14 +++-
info/info-can.sh | 3 +
8 files changed, 332 insertions(+), 10 deletions(-)
diff --git a/lib/internal.h b/lib/internal.h
index efbb5abb..64579bc0 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -73,6 +73,8 @@ struct meta_context {
};
DEFINE_VECTOR_TYPE (meta_vector, struct meta_context);
+DEFINE_VECTOR_TYPE(uint32_vector, uint32_t);
+
struct export {
char *name;
char *description;
@@ -399,7 +401,8 @@ struct command {
uint64_t cookie;
uint64_t offset;
uint64_t count;
- void *data; /* Buffer for read/write */
+ void *data; /* Buffer for read/write, uint32_vector* for status payload */
+ uint32_vector *ids; /* For block status with payload */
struct command_cb cb;
bool initialized; /* For read, true if getting a hole may skip memset */
uint32_t data_seen; /* For read, cumulative size of data chunks seen */
diff --git a/generator/API.ml b/generator/API.ml
index 6228a4e5..2e8a33bd 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -2335,12 +2335,13 @@ "can_block_status_payload", {
longdesc = "\
Returns true if the server supports the use of the
C<LIBNBD_CMD_FLAG_PAYLOAD_LEN> flag to allow filtering of the
-block status command. Returns
+block status command (see L<nbd_block_status_filter(3)>). Returns
false if the server does not. Note that this will never return
true if L<nbd_get_extended_headers_negotiated(3)> is false."
^ non_blocking_test_call_description;
see_also = [SectionLink "Flag calls"; Link "opt_info";
- Link "get_extended_headers_negotiated"];
+ Link "get_extended_headers_negotiated";
+ Link "block_status_filter"];
example = Some "examples/server-flags.c";
};
@@ -2409,6 +2410,10 @@ "can_meta_context", {
meta contexts were requested but there is a missing or failed
attempt at NBD_OPT_SET_META_CONTEXT during option negotiation.
+If the server supports block status filtering (see
+L<nbd_can_block_status_payload(3)>, this function must return
+true for any filter name passed to L<nbd_block_status_filter(3)>.
+
The single parameter is the name of the metadata context,
for example C<LIBNBD_CONTEXT_BASE_ALLOCATION>.
B<E<lt>libnbd.hE<gt>> includes defined constants for
well-known
@@ -2941,9 +2946,12 @@ "block_status_64", {
information about blocks beginning from the specified
offset to be returned. The C<count> parameter is a hint: the
server may choose to return less status, or the final block
-may extend beyond the requested range. If multiple contexts
+may extend beyond the requested range. When multiple contexts
are supported, the number of blocks and cumulative length
-of those blocks need not be identical between contexts.
+of those blocks need not be identical between contexts; this
+command generally returns the status of all negotiated contexts,
+while some servers also support a filtered request (see
+L<nbd_can_block_status_payload(3)>, L<nbd_block_status_filter(3)>).
Note that not all servers can support a C<count> of 4GiB or larger;
L<nbd_get_extended_headers_negotiated(3)> indicates which servers
@@ -2993,11 +3001,38 @@ "block_status_64", {
does not exceed C<count> bytes; however, libnbd does not
validate that the server obeyed the flag."
^ strict_call_description;
- see_also = [Link "block_status";
+ see_also = [Link "block_status"; Link
"block_status_filter";
Link "add_meta_context"; Link
"can_meta_context";
Link "aio_block_status_64"; Link
"set_strict_mode"];
};
+ "block_status_filter", {
+ default_call with
+ args = [ UInt64 "count"; UInt64 "offset"; StringList
"contexts";
+ Closure extent64_closure ];
+ optargs = [ OFlags ("flags", cmd_flags, Some
["REQ_ONE"; "PAYLOAD_LEN"]) ];
+ ret = RErr;
+ permitted_states = [ Connected ];
+ shortdesc = "send filtered block status command, with 64-bit
callback";
+ longdesc = "\
+Issue a filtered block status command to the NBD server. If
+supported by the server (see L<nbd_can_block_status_payload(3)>),
+this causes metadata context information about blocks beginning
+from the specified offset to be returned, and with the result
+limited to just the contexts specified in C<filter>. Note that
+all strings in C<filter> must be supported by
+L<nbd_can_meta_context(3)>.
+
+All other parameters to this function have the same semantics
+as in L<nbd_block_status_64(3)>; except that for convenience,
+the C<flags> parameter may additionally contain or omit
+C<LIBNBD_CMD_FLAG_PAYLOAD_LEN>."
+^ strict_call_description;
+ see_also = [Link "block_status_64";
+ Link "can_block_status_payload"; Link
"can_meta_context";
+ Link "aio_block_status_filter"; Link
"set_strict_mode"];
+ };
+
"poll", {
default_call with
args = [ Int "timeout" ]; ret = RInt;
@@ -3667,6 +3702,30 @@ "aio_block_status_64", {
Link "set_strict_mode"];
};
+ "aio_block_status_filter", {
+ default_call with
+ args = [ UInt64 "count"; UInt64 "offset"; StringList
"contexts";
+ Closure extent64_closure ];
+ optargs = [ OClosure completion_closure;
+ OFlags ("flags", cmd_flags, Some
["REQ_ONE"; "PAYLOAD_LEN"]) ];
+ ret = RCookie;
+ permitted_states = [ Connected ];
+ shortdesc = "send filtered block status command to the NBD
server";
+ longdesc = "\
+Send a filtered block status command to the NBD server.
+
+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_filter(3)>."
+^ strict_call_description;
+ see_also = [SectionLink "Issuing asynchronous commands";
+ Link "aio_block_status_64"; Link
"block_status_filter";
+ Link "can_meta_context"; Link
"can_block_status_payload";
+ Link "set_strict_mode"];
+ };
+
"aio_get_fd", {
default_call with
args = []; ret = RFd;
@@ -4201,6 +4260,8 @@ let first_version "opt_extended_headers", (1,
18);
"aio_opt_extended_headers", (1, 18);
"can_block_status_payload", (1, 18);
+ "block_status_filter", (1, 18);
+ "aio_block_status_filter", (1, 18);
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
diff --git a/generator/states-issue-command.c b/generator/states-issue-command.c
index 59b05835..d825fbb7 100644
--- a/generator/states-issue-command.c
+++ b/generator/states-issue-command.c
@@ -84,7 +84,9 @@ ISSUE_COMMAND.PREPARE_WRITE_PAYLOAD:
assert (h->cmds_to_issue != NULL);
cmd = h->cmds_to_issue;
assert (cmd->cookie == be64toh (h->req.compact.cookie));
- if (cmd->type == NBD_CMD_WRITE) {
+ if (cmd->type == NBD_CMD_WRITE ||
+ (h->extended_headers && cmd->type == NBD_CMD_BLOCK_STATUS
&&
+ cmd->flags & NBD_CMD_FLAG_PAYLOAD_LEN)) {
h->wbuf = cmd->data;
h->wlen = cmd->count;
if (cmd->next && cmd->count < 64 * 1024)
diff --git a/lib/aio.c b/lib/aio.c
index cf9421ec..30505a36 100644
--- a/lib/aio.c
+++ b/lib/aio.c
@@ -37,6 +37,10 @@ nbd_internal_retire_and_free_command (struct command *cmd)
FREE_CALLBACK (cmd->cb.fn.extent64);
else
FREE_CALLBACK (cmd->cb.fn.extent32);
+ if (cmd->ids) {
+ uint32_vector_reset (cmd->ids);
+ free (cmd->ids);
+ }
}
if (cmd->type == NBD_CMD_READ)
FREE_CALLBACK (cmd->cb.fn.chunk);
diff --git a/lib/rw.c b/lib/rw.c
index 8e8c024c..87bcb1b6 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -187,6 +187,26 @@ nbd_unlocked_block_status_64 (struct nbd_handle *h,
return wait_for_command (h, cookie);
}
+/* Issue a filtered block status command and wait for the reply. */
+int
+nbd_unlocked_block_status_filter (struct nbd_handle *h,
+ uint64_t count, uint64_t offset,
+ char **filter,
+ nbd_extent64_callback *extent64,
+ uint32_t flags)
+{
+ int64_t cookie;
+ nbd_completion_callback c = NBD_NULL_COMPLETION;
+
+ cookie = nbd_unlocked_aio_block_status_filter (h, count, offset, filter,
+ extent64, &c, flags);
+ if (cookie == -1)
+ return -1;
+
+ assert (CALLBACK_IS_NULL (*extent64));
+ 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,
@@ -195,6 +215,7 @@ nbd_internal_command_common (struct nbd_handle *h,
void *data, struct command_cb *cb)
{
struct command *cmd;
+ uint32_vector *ids = NULL;
if (h->disconnect_request) {
set_error (EINVAL, "cannot request more commands after
NBD_CMD_DISC");
@@ -242,10 +263,23 @@ nbd_internal_command_common (struct nbd_handle *h,
}
break;
+ case NBD_CMD_BLOCK_STATUS:
+ if (data) {
+ ids = data;
+ count = ids->len * sizeof (uint32_t);
+ data = ids->ptr;
+ if (count > MAX_REQUEST_SIZE ||
+ (h->strict & LIBNBD_STRICT_PAYLOAD && count >
h->payload_maximum)) {
+ set_error (ERANGE, "filter set too large");
+ goto err;
+ }
+ break;
+ }
+ /* fallthrough */
+ default:
/* Other commands are limited by the 32 bit field in the command
* structure on the wire, unless extended headers were negotiated.
*/
- default:
if (!h->extended_headers && count > UINT32_MAX) {
set_error (ERANGE, "request too large: maximum request size is
%" PRIu32,
UINT32_MAX);
@@ -265,6 +299,7 @@ nbd_internal_command_common (struct nbd_handle *h,
cmd->offset = offset;
cmd->count = count;
cmd->data = data;
+ cmd->ids = ids;
if (cb)
cmd->cb = *cb;
@@ -314,6 +349,10 @@ nbd_internal_command_common (struct nbd_handle *h,
FREE_CALLBACK (cb->fn.extent32);
else
FREE_CALLBACK (cb->fn.extent64);
+ if (ids) {
+ uint32_vector_reset (ids);
+ free (ids);
+ }
}
if (type == NBD_CMD_READ)
FREE_CALLBACK (cb->fn.chunk);
@@ -561,3 +600,86 @@ nbd_unlocked_aio_block_status_64 (struct nbd_handle *h,
return nbd_internal_command_common (h, flags, NBD_CMD_BLOCK_STATUS, offset,
count, EINVAL, NULL, &cb);
}
+
+int64_t
+nbd_unlocked_aio_block_status_filter (struct nbd_handle *h,
+ uint64_t count, uint64_t offset,
+ char **filter,
+ nbd_extent64_callback *extent64,
+ nbd_completion_callback *completion,
+ uint32_t flags)
+{
+ struct command_cb cb = { .fn.extent64 = *extent64, .wide = true,
+ .completion = *completion };
+ uint32_vector *ids;
+ char *name;
+ size_t i;
+
+ /* Because this affects wire format, it is more convenient to manage
+ * PAYLOAD_LEN by what was negotiated than to require the user to
+ * have to set it correctly.
+ */
+ if (!h->extended_headers) {
+ set_error (ENOTSUP, "server does not support extended headers");
+ return -1;
+ }
+ flags |= LIBNBD_CMD_FLAG_PAYLOAD_LEN;
+
+ if (h->strict & LIBNBD_STRICT_COMMANDS) {
+ if (nbd_unlocked_can_block_status_payload (h) != 1) {
+ set_error (EINVAL,
+ "server does not support the block status payload
flag");
+ return -1;
+ }
+
+ if (!h->meta_valid || h->meta_contexts.len == 0) {
+ 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;
+ }
+ }
+
+ ids = calloc (1, sizeof *ids);
+ if (ids == NULL) {
+ set_error (errno, "calloc");
+ return -1;
+ }
+ if (uint32_vector_append (ids, htobe32 (count >> 32)) == -1 ||
+ uint32_vector_append (ids, htobe32 (count)) == -1) {
+ set_error (errno, "realloc");
+ goto fail;
+ }
+
+ /* O(n^2) search - hopefully filter and negotiated contexts are both small */
+ for ( ; (name = *filter) != NULL; filter++) {
+ if (!h->meta_valid) {
+ set_error (EINVAL, "context %s not negotiated", name);
+ goto fail;
+ }
+ for (i = 0; i < h->meta_contexts.len; i++) {
+ struct meta_context *meta = &h->meta_contexts.ptr[i];
+ if (strcmp (name, meta->name) == 0) {
+ if (uint32_vector_append (ids, htobe32 (meta->context_id)) == -1) {
+ set_error (errno, "realloc");
+ goto fail;
+ }
+ break;
+ }
+ }
+ if (i == h->meta_contexts.len) {
+ set_error (EINVAL, "context %s not negotiated", name);
+ goto fail;
+ }
+ }
+
+ SET_CALLBACK_TO_NULL (*extent64);
+ SET_CALLBACK_TO_NULL (*completion);
+ return nbd_internal_command_common (h, flags, NBD_CMD_BLOCK_STATUS, offset,
+ count, EINVAL, ids, &cb);
+
+ fail:
+ uint32_vector_reset (ids);
+ free (ids);
+ return -1;
+}
diff --git a/interop/block-status-payload.c b/interop/block-status-payload.c
index 9603dfe5..704b25aa 100644
--- a/interop/block-status-payload.c
+++ b/interop/block-status-payload.c
@@ -54,11 +54,26 @@ cb (void *opaque, const char *metacontext, uint64_t offset,
return 0;
}
+static char **
+list (unsigned int use)
+{
+ static const char *array[ARRAY_SIZE (contexts) + 1];
+ size_t i, j;
+
+ assert (use < 1 << ARRAY_SIZE (contexts));
+ for (i = j = 0; i < ARRAY_SIZE (contexts); i++)
+ if (use & (1 << i))
+ array[j++] = contexts[i];
+ array[j] = NULL;
+ return (char **) array;
+}
+
int
main (int argc, char *argv[])
{
struct nbd_handle *nbd;
int64_t exportsize;
+ uint64_t bytes_sent;
unsigned int seen;
size_t i;
int r;
@@ -114,7 +129,107 @@ main (int argc, char *argv[])
}
assert (seen == 0xf);
- /* FIXME: Test filtered calls once the API is added */
+ /* Filtering with all contexts listed, same effect as unfilitered call */
+ seen = 0;
+ if (nbd_block_status_filter (nbd, exportsize, 0, list (0xf),
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data = &seen
},
+ 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ assert (seen == 0xf);
+
+ /* Filtering with just two out of four contexts; test optional flag */
+ seen = 0;
+ if (nbd_block_status_filter (nbd, exportsize, 0, list (0x5),
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data = &seen
},
+ LIBNBD_CMD_FLAG_PAYLOAD_LEN) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ assert (seen == 0x5);
+
+ /* Filtering with one context, near end of file (to make sure the
+ * payload length isn't confused with the effect length)
+ */
+ seen = 0;
+ if (nbd_block_status_filter (nbd, 1, exportsize - 1, list (0x2),
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data = &seen
},
+ 0) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ assert (seen == 0x2);
+
+ /* Filtering with no contexts - pointless, so qemu rejects it */
+ bytes_sent = nbd_stats_bytes_sent (nbd);
+ seen = 0;
+ if (nbd_block_status_filter (nbd, exportsize, 0, list (0x0),
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data = &seen
},
+ 0) != -1) {
+ fprintf (stderr, "expecting block status failure\n");
+ exit (EXIT_FAILURE);
+ }
+ assert (seen == 0x0);
+ if (nbd_get_errno () != EINVAL) {
+ fprintf (stderr, "expecting EINVAL after block status
failure\n");
+ exit (EXIT_FAILURE);
+ }
+ if (nbd_stats_bytes_sent (nbd) <= bytes_sent) {
+ fprintf (stderr, "expecting server-side rejection of bad
request\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Giving unknown string triggers EINVAL from libnbd */
+ bytes_sent = nbd_stats_bytes_sent (nbd);
+ seen = 0;
+ {
+ const char *bogus[] = { "qemu:dirty-bitmap:bitmap2", NULL };
+ if (nbd_block_status_filter (nbd, exportsize, 0, (char **) bogus,
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data =
&seen },
+ 0) != -1) {
+ fprintf (stderr, "expecting block status failure\n");
+ exit (EXIT_FAILURE);
+ }
+ }
+ if (nbd_get_errno () != EINVAL) {
+ fprintf (stderr, "expecting EINVAL after block status
failure\n");
+ exit (EXIT_FAILURE);
+ }
+ assert (seen == 0x0);
+ if (nbd_stats_bytes_sent (nbd) != bytes_sent) {
+ fprintf (stderr, "expecting client-side rejection of bad
request\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Giving same string twice triggers EINVAL from qemu */
+ seen = 0;
+ {
+ const char *dupes[] = { "base:allocation",
"base:allocation", NULL };
+ if (nbd_block_status_filter (nbd, exportsize, 0, (char **) dupes,
+ (nbd_extent64_callback) { .callback = cb,
+ .user_data =
&seen },
+ 0) != -1) {
+ fprintf (stderr, "expecting block status failure\n");
+ exit (EXIT_FAILURE);
+ }
+ }
+ if (nbd_get_errno () != EINVAL) {
+ fprintf (stderr, "expecting EINVAL after block status
failure\n");
+ exit (EXIT_FAILURE);
+ }
+ assert (seen == 0x0);
+ if (nbd_stats_bytes_sent (nbd) <= bytes_sent) {
+ fprintf (stderr, "expecting server-side rejection of bad
request\n");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Done */
if (nbd_shutdown (nbd, 0) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
diff --git a/interop/block-status-payload.sh b/interop/block-status-payload.sh
index a12cfc8a..0e6681b6 100755
--- a/interop/block-status-payload.sh
+++ b/interop/block-status-payload.sh
@@ -49,6 +49,7 @@ args = ["qemu-nbd", "-f",
"qcow2", "-A", "-B", "bitmap0",
"-B", "bitmap1",
h.connect_systemd_socket_activation(args)
assert h.aio_is_negotiating() is True
assert h.get_extended_headers_negotiated() is False
+
# Flag not available until info or go
try:
h.can_block_status_payload()
@@ -58,7 +59,18 @@ except nbd.Error:
h.opt_info()
assert h.can_block_status_payload() is False
assert h.can_meta_context("base:allocation") is True
-h.opt_abort()
+
+# Filter request not allowed if not advertised
+def f():
+ assert False
+h.opt_go()
+assert h.can_block_status_payload() is False
+try:
+ h.block_status_filter(0, 512, ["base:allocation"], f)
+ assert False
+except nbd.Error:
+ pass
+h.shutdown()
'
# Conditional part of test: if qemu is new enough to support extended
diff --git a/info/info-can.sh b/info/info-can.sh
index 5d9f2e89..391d2c12 100755
--- a/info/info-can.sh
+++ b/info/info-can.sh
@@ -38,6 +38,9 @@ requires bash -c "nbdkit sh --dump-plugin | grep
has_can_cache=1"
# and oldstyle never, but that feels like depending a bit too much on
# the implementation.
+# --has block-status-payload is not supported by nbdkit yet. Testing
+# is done during interop with new-enough qemu.
+
# --has structured-reply is not a per-export setting, but rather
# something set on the server as a whole.
--
2.41.0
Eric Blake
2023-Aug-03 01:50 UTC
[Libguestfs] [libnbd PATCH v4 25/25] api: Add LIBNBD_STRICT_AUTO_FLAG control to nbd_set_strict
We've previously documented that nbd_set_strict() can add new bits,
defaulting to on to provide client-side safety, but which can be
overridden when performing specific integration tests against a
server. Prior to the introduction of libnbd support for extended
headers, a user could manually disable STRICT_FLAGS and
STRICT_COMMANDS to test the response of an older server receiving an
unexpected NBD_CMD_FLAG_PAYLOAD_LEN; but for convenience sake, we
prefer our default for that flag to now track extended headers and
ignore what the user passes in, which removes the ability we had for
that integration test. Adding a new strictness knob lets the user be
in charge of that bit's contents once again. The caveat remains that
disabling the strictness bits makes it possible for a client to
violate the NBD protocol which may have undefined results (the
connection may drop, libnbd may hang, ...), so most clients will never
call nbd_set_strict() in the first place.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
v4: new patch
---
generator/API.ml | 32 +++++++++++++++++++++++++-------
lib/rw.c | 46 +++++++++++++++++++++++++++++-----------------
2 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/generator/API.ml b/generator/API.ml
index 2e8a33bd..4d1021b4 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -234,6 +234,7 @@ let strict_flags "ZERO_SIZE", 1 lsl 3;
"ALIGN", 1 lsl 4;
"PAYLOAD", 1 lsl 5;
+ "AUTO_FLAG", 1 lsl 6;
]
}
let allow_transport_flags = {
@@ -1138,7 +1139,8 @@ "set_strict_mode", {
Flags that are known by libnbd as associated with a given command
(such as C<LIBNBD_CMD_FLAG_DF> for L<nbd_pread_structured(3)> gated
by L<nbd_can_df(3)>) are controlled by C<LIBNBD_STRICT_COMMANDS>
-instead.
+instead; and C<LIBNBD_CMD_FLAG_PAYLOAD_LEN> is managed automatically
+by libnbd unless C<LIBNBD_STRICT_AUTO_FLAG> is disabled.
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
@@ -1175,6 +1177,19 @@ "set_strict_mode", {
requests larger than 32MiB are liable to cause some servers to
disconnect.
+=item C<LIBNBD_STRICT_AUTO_FLAG> = 0x40
+
+If set, commands that accept the C<LIBNBD_CMD_FLAG_PAYLOAD_LEN>
+flag (such as L<nbd_pwrite(3)> and C<nbd_block_status_filter(3)>)
+ignore the presence or absence of that flag from the caller,
+instead sending the value over the wire that matches the
+server's expectations based on whether extended headers were
+negotiated when the connection was made. If clear, the caller
+takes on the responsibility for whether the payload length
+flag is set or clear during the affected command, which can
+be useful during integration testing but is more likely to
+lead to undefined behavior.
+
=back
For convenience, the constant C<LIBNBD_STRICT_MASK> is available to
@@ -2682,10 +2697,11 @@ "pwrite", {
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)>). For convenience, libnbd ignores the presence
-or absence of the flag C<LIBNBD_CMD_FLAG_PAYLOAD_LEN> in C<flags>,
-while correctly using the flag over the wire according to whether
-extended headers were negotiated."
+L<nbd_can_fua(3)>). For convenience, unless
C<nbd_set_strict_flags(3)>
+was used to disable C<LIBNBD_STRICT_AUTO_FLAG>, libnbd ignores the
+presence or absence of the flag C<LIBNBD_CMD_FLAG_PAYLOAD_LEN>
+in C<flags>, while correctly using the flag over the wire
+according to whether extended headers were negotiated."
^ strict_call_description;
see_also = [Link "can_fua"; Link "is_read_only";
Link "aio_pwrite"; Link "get_block_size";
@@ -3025,8 +3041,10 @@ "block_status_filter", {
All other parameters to this function have the same semantics
as in L<nbd_block_status_64(3)>; except that for convenience,
-the C<flags> parameter may additionally contain or omit
-C<LIBNBD_CMD_FLAG_PAYLOAD_LEN>."
+unless <nbd_set_strict_flags(3)> was used to disable
+C<LIBNBD_STRICT_AUTO_FLAG>, libnbd ignores the presence or
+absence of the flag C<LIBNBD_CMD_FLAG_PAYLOAD_LEN>
+in C<flags>, while correctly using the flag over the wire."
^ strict_call_description;
see_also = [Link "block_status_64";
Link "can_block_status_payload"; Link
"can_meta_context";
diff --git a/lib/rw.c b/lib/rw.c
index 87bcb1b6..2a849d21 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -406,6 +406,15 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void
*buf,
{
struct command_cb cb = { .completion = *completion };
+ if (h->strict & LIBNBD_STRICT_AUTO_FLAG) {
+ /* It is more convenient to manage PAYLOAD_LEN by what was negotiated
+ * than to require the user to have to set it correctly.
+ */
+ if (h->extended_headers)
+ flags |= LIBNBD_CMD_FLAG_PAYLOAD_LEN;
+ else
+ flags &= ~LIBNBD_CMD_FLAG_PAYLOAD_LEN;
+ }
if (h->strict & LIBNBD_STRICT_COMMANDS) {
if (nbd_unlocked_is_read_only (h) == 1) {
set_error (EPERM, "server does not support write operations");
@@ -417,16 +426,12 @@ nbd_unlocked_aio_pwrite (struct nbd_handle *h, const void
*buf,
set_error (EINVAL, "server does not support the FUA flag");
return -1;
}
+
+ if (!!(flags & LIBNBD_CMD_FLAG_PAYLOAD_LEN) != h->extended_headers)
{
+ set_error (EINVAL, "incorrect setting for PAYLOAD_LEN flag");
+ return -1;
+ }
}
- /* It is more convenient to manage PAYLOAD_LEN by what was negotiated
- * than to require the user to have to set it correctly.
- * TODO: Add new h->strict bit to allow intentional protocol violation
- * for interoperability testing.
- */
- if (h->extended_headers)
- flags |= LIBNBD_CMD_FLAG_PAYLOAD_LEN;
- else
- flags &= ~LIBNBD_CMD_FLAG_PAYLOAD_LEN;
SET_CALLBACK_TO_NULL (*completion);
return nbd_internal_command_common (h, flags, NBD_CMD_WRITE, offset, count,
@@ -615,15 +620,17 @@ nbd_unlocked_aio_block_status_filter (struct nbd_handle
*h,
char *name;
size_t i;
- /* Because this affects wire format, it is more convenient to manage
- * PAYLOAD_LEN by what was negotiated than to require the user to
- * have to set it correctly.
- */
- if (!h->extended_headers) {
- set_error (ENOTSUP, "server does not support extended headers");
- return -1;
+ if (h->strict & LIBNBD_STRICT_AUTO_FLAG) {
+ /* Because this affects wire format, it is more convenient to manage
+ * PAYLOAD_LEN by what was negotiated than to require the user to
+ * have to set it correctly.
+ */
+ if (!h->extended_headers) {
+ set_error (ENOTSUP, "server does not support extended
headers");
+ return -1;
+ }
+ flags |= LIBNBD_CMD_FLAG_PAYLOAD_LEN;
}
- flags |= LIBNBD_CMD_FLAG_PAYLOAD_LEN;
if (h->strict & LIBNBD_STRICT_COMMANDS) {
if (nbd_unlocked_can_block_status_payload (h) != 1) {
@@ -638,6 +645,11 @@ nbd_unlocked_aio_block_status_filter (struct nbd_handle *h,
"connecting or the server does not support it");
return -1;
}
+
+ if ((flags & LIBNBD_CMD_FLAG_PAYLOAD_LEN) == 0) {
+ set_error (EINVAL, "incorrect setting for PAYLOAD_LEN flag");
+ return -1;
+ }
}
ids = calloc (1, sizeof *ids);
--
2.41.0
Eric Blake
2023-Aug-11 18:18 UTC
[Libguestfs] [libnbd PATCH v4 02.5/25] generator: Support Extent64 arg in Rust code
See the earlier commit "Add Extent64 arg type" for rationale in
supporting a new generator arg type. This patch adds the Rust
bindings for use of Extent64.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
Tage, could you please review this patch? I'm about to commit my
64-bit extension patches, but had to rebase them on top of your Rust
patches that landed in the meantime. I'm new enough to Rust that I'm
not sure if I'm doing everything in the optimum manner, so I welcome
any insights you may offer.
This email is one of the two main patches I'm worried about, I'll
reply again with a link to a git repo and the diff to the generated
code after this patch plus minor tweaks to 9 and 10/25 are in place to
actually put this code to use (uncommenting extent64_callback and
actually defining the nbd_block_status_64 API is what makes the code
paths in this patch worth anything); plus the relevant changes to
patch to 11/25 that copies the language bindings unit tests I did for
all the other bindings to also work in Rust.
My thinking in this patch: the public libnbd crate should expose a
full-fledged Rust type NbdExport, while the low-level libnbd-sys code
needs an ffi interface that mirrors C's 'struct nbd_extent' from the
generated <libnbd.h>. Since my unit test passed (soon to be posted in
the next email), I assume I got things right, but I could be easily
doing something stupid that 'unsafe' let me blindly paper over.
---
generator/Rust.ml | 8 ++++++--
generator/RustSys.ml | 15 +++++++++++----
rust/src/types.rs | 6 ++++++
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/generator/Rust.ml b/generator/Rust.ml
index b9c938e8..3babc1a5 100644
--- a/generator/Rust.ml
+++ b/generator/Rust.ml
@@ -108,6 +108,7 @@ let
| Path n
| Fd n
| Enum (n, _)
+ | Extent64 n
| Flags (n, _)
| SockAddrAndLen (n, _)
| BytesIn (n, _)
@@ -116,7 +117,6 @@ let
| BytesPersistOut (n, _)
| Closure { cbname = n } ->
n
- | Extent64 _ -> assert false (* only used in extent64_closure *)
(* Get the name of a rust optional argument. *)
let rust_optarg_name : optarg -> string = function
@@ -151,7 +151,7 @@ let
| BytesPersistIn _ -> "&'static [u8]"
| BytesPersistOut _ -> "&'static mut [u8]"
| Closure { cbargs } -> "impl " ^ rust_closure_trait cbargs
- | Extent64 _ -> assert false (* only used in extent64_closure *)
+ | Extent64 _ -> "NbdExtent"
(* Get the Rust closure trait for a callback, That is `Fn*(...) -> ...)`. *)
and rust_closure_trait ?(lifetime = Some "'static") cbargs :
string @@ -232,6 +232,7 @@ let
| CBString _ -> [ "*const c_char" ]
| CBBytesIn _ -> [ "*const c_void"; "usize" ]
| CBArrayAndLen (UInt32 _, _) -> [ "*mut u32"; "usize"
]
+ | CBArrayAndLen (Extent64 _, _) -> [ "*mut nbd_extent";
"usize" ]
| CBArrayAndLen _ ->
failwith
"generator/Rust.ml: in ffi_cbarg_types: Unsupported type of array
\
@@ -377,6 +378,8 @@ let
ffi_len_name
| CBArrayAndLen (UInt32 _, _), [ ffi_arr_name; ffi_len_name ] ->
pr "slice::from_raw_parts(%s, %s)" ffi_arr_name ffi_len_name
+ | CBArrayAndLen (Extent64 _, _), [ ffi_arr_name; ffi_len_name ] ->
+ pr "slice::from_raw_parts(%s as *const NbdExtent, %s)"
ffi_arr_name ffi_len_name
| CBArrayAndLen _, [ _; _ ] ->
failwith
"generator/Rust.ml: in ffi_cbargs_to_rust: Unsupported type of
array \
@@ -553,6 +556,7 @@ let
pr "use std::path::PathBuf;\n";
pr "use std::ptr;\n";
pr "use std::slice;\n";
+ (* pr "use libnbd_sys::nbd_extent;\n"; uncomment for
extent64_callback *)
pr "\n"
let generate_rust_bindings () diff --git a/generator/RustSys.ml
b/generator/RustSys.ml
index be01ab8d..1025c699 100644
--- a/generator/RustSys.ml
+++ b/generator/RustSys.ml
@@ -39,7 +39,7 @@ let
| BytesIn (n1, n2) | BytesPersistIn (n1, n2) -> [ "*const
c_void"; "usize" ]
| BytesOut (n1, n2) | BytesPersistOut (n1, n2) -> [ "*mut
c_void"; "usize" ]
| Closure { cbname } -> [ sprintf "nbd_%s_callback" cbname ]
- | Extent64 _ -> assert false (* only used in extent64_closure *)
+ | Extent64 (_) -> [ "nbd_extent" ]
(** The type of an optional argument. *)
let optarg_type : optarg -> string = function
@@ -133,13 +133,20 @@ let
|> String.concat ", ")
(ret_type call.ret)
-(** Print a definition of the "nbd_handle" type. *)
-let print_nbd_handle () +(** Print a definition of "nbd_handle" and
"nbd_extent" types. *)
+let print_types () pr "#[repr(C)]\n";
pr "#[derive(Debug, Clone, Copy)]\n";
pr "pub struct nbd_handle {\n";
pr " _unused: [u8; 0],\n";
pr "}\n";
+ pr "\n";
+ pr "#[repr(C)]\n";
+ pr "#[derive(Debug, Clone, Copy)]\n";
+ pr "pub struct nbd_extent {\n";
+ pr " length: u64,\n";
+ pr " flags: u64,\n";
+ pr "}\n";
pr "\n"
(** Print some more "extern definitions". *)
@@ -161,7 +168,7 @@ let
generate_header CStyle ~copyright:"Tage Johansson";
pr "\n";
print_imports ();
- print_nbd_handle ();
+ print_types ();
print_more_defs ();
all_closures |> List.iter print_closure_struct;
pr "extern \"C\" {\n";
diff --git a/rust/src/types.rs b/rust/src/types.rs
index eb2df060..6bce2888 100644
--- a/rust/src/types.rs
+++ b/rust/src/types.rs
@@ -16,3 +16,9 @@
// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
pub struct Cookie(pub(crate) u64);
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct NbdExtent {
+ pub length: u64,
+ pub flags: u64,
+}
--
2.41.0
Eric Blake
2023-Aug-11 18:27 UTC
[Libguestfs] [libnbd PATCH v4 11.5/25] api: Add tests for [aio_]nbd_block_status_64
Add unit test coverage to prove that the new API works in each of C,
Python, OCaml, Go, and Rust bindings.
Signed-off-by: Eric Blake <eblake at redhat.com>
---
The bulk of this patch is unchanged from
https://listman.redhat.com/archives/libguestfs/2023-August/032238.html
so I've elided those files from the listing below.
However, the Rust portion is new, because I had to rebase on top of
Tage's work that landed in the meantime. 'make check' passed for me
with Rust in the build, so I assume I got it right, but I would
appreciate the double-check.
The patches can also be checked out from
https://repo.or.cz/libnbd/ericb.git/ (warning, I may do
non-fast-forward pushes there as I continue rebase work), if that
makes it easier to check out my work.
---
python/t/465-block-status-64.py | 56 ++++++++++
ocaml/tests/Makefile.am | 1 +
ocaml/tests/test_465_block_status_64.ml | 58 ++++++++++
rust/Makefile.am | 1 +
rust/tests/test_465_block_status_64.rs | 127 ++++++++++++++++++++++
tests/meta-base-allocation.c | 104 +++++++++++++++---
golang/Makefile.am | 1 +
golang/libnbd_465_block_status_64_test.go | 119 ++++++++++++++++++++
...
diff --git a/rust/Makefile.am b/rust/Makefile.am
index 7098c9ad..8ec54700 100644
--- a/rust/Makefile.am
+++ b/rust/Makefile.am
@@ -58,6 +58,7 @@ source_files = \
tests/test_405_pread_structured.rs \
tests/test_410_pwrite.rs \
tests/test_460_block_status.rs \
+ tests/test_465_block_status_64.rs \
tests/test_620_stats.rs \
tests/test_log/mod.rs \
run-tests.sh.in \
diff --git a/rust/tests/test_465_block_status_64.rs
b/rust/tests/test_465_block_status_64.rs
new file mode 100644
index 00000000..347ee9b5
--- /dev/null
+++ b/rust/tests/test_465_block_status_64.rs
@@ -0,3 +1,130 @@
+// libnbd Rust test case
+// Copyright Tage Johansson
+// Copyright Red Hat
+//
+// This library is free software; you can redistribute it and/or
+// modify it under the terms of the GNU Lesser General Public
+// License as published by the Free Software Foundation; either
+// version 2 of the License, or (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#![deny(warnings)]
+
+use libnbd::types::NbdExtent;
+use std::env;
+use std::path::Path;
+use std::sync::Arc;
+use std::sync::Mutex;
+
+fn block_status_get_entries(
+ nbd: &libnbd::Handle,
+ count: u64,
+ offset: u64,
+ flags: Option<libnbd::CmdFlag>,
+) -> Vec<NbdExtent> {
+ let entries = Arc::new(Mutex::new(None));
+ let entries_clone = entries.clone();
+ nbd.block_status_64(
+ count,
+ offset,
+ move |metacontext, _, entries, err| {
+ assert_eq!(*err, 0);
+ if metacontext == libnbd::CONTEXT_BASE_ALLOCATION {
+ *entries_clone.lock().unwrap() = Some(entries.to_vec());
+ }
+ 0
+ },
+ flags,
+ )
+ .unwrap();
+ Arc::into_inner(entries)
+ .unwrap()
+ .into_inner()
+ .unwrap()
+ .unwrap()
+}
+
+#[test]
+fn test_block_status() {
+ let srcdir = env::var("srcdir").unwrap();
+ let srcdir = Path::new(&srcdir);
+ let script_path =
srcdir.join("../tests/meta-base-allocation.sh");
+ let script_path = script_path.to_str().unwrap();
+ let nbd = libnbd::Handle::new().unwrap();
+ nbd.add_meta_context(libnbd::CONTEXT_BASE_ALLOCATION)
+ .unwrap();
+ nbd.connect_command(&[
+ "nbdkit",
+ "-s",
+ "--exit-with-parent",
+ "-v",
+ "sh",
+ script_path,
+ ])
+ .unwrap();
+
+ assert_eq!(
+ block_status_get_entries(&nbd, 65536, 0, None).as_slice(),
+ &[
+ NbdExtent {
+ length: 8192,
+ flags: 0
+ },
+ NbdExtent {
+ length: 8192,
+ flags: 1
+ },
+ NbdExtent {
+ length: 16384,
+ flags: 3
+ },
+ NbdExtent {
+ length: 16384,
+ flags: 2
+ },
+ NbdExtent {
+ length: 16384,
+ flags: 0
+ },
+ ]
+ );
+
+ assert_eq!(
+ block_status_get_entries(&nbd, 1024, 32256, None).as_slice(),
+ &[
+ NbdExtent {
+ length: 512,
+ flags: 3
+ },
+ NbdExtent {
+ length: 16384,
+ flags: 2
+ }
+ ]
+ );
+
+ assert_eq!(
+ block_status_get_entries(
+ &nbd,
+ 1024,
+ 32256,
+ Some(libnbd::CmdFlag::REQ_ONE)
+ )
+ .as_slice(),
+ &[NbdExtent {
+ length: 512,
+ flags: 3
+ }]
+ );
+}
...
--
2.41.0
Possibly Parallel Threads
- [libnbd PATCH v2 06/13] api: Add nbd_opt_abort and nbd_aio_opt_abort
- [libnbd PATCH] API: Add nbd_set_opt_mode to expose NEGOTIATING state
- [libnbd PATCH 4/4] states: Add NBD_OPT_EXPORT_NAME handling
- [PATCH libnbd PROPOSAL] Add APIs for listing exports from an NBD server.
- [libnbd PATCH 5/7] states: Factor out NBD_REP payload prep