Re: [Libguestfs] [libnbd PATCH v3 14/19] CONNECT_COMMAND.START: plug child process leak on error

2023-03-23 Thread Eric Blake
of the socket pair (sv[0]). So once that function > returns successfully, we can't close sv[0] even on the error path -- we > close the "higher level" socket, and that invalidates sv[0] at once. Track > this ownership transfer with a new boolean flag therefore. > > Sign

Re: [Libguestfs] [libnbd PATCH v3 16/19] socket activation: generalize environment construction

2023-03-23 Thread Eric Blake
; with the new > "capturing" interface. > > Signed-off-by: Laszlo Ersek > Reviewed-by: Richard W.M. Jones > --- Reviewed-by: Eric Blake > @@ -153,13 +237,18 @@ CONNECT_SA.START: >if (nbd_internal_execvpe_init (&execvpe_ctx, h->argv.ptr[0], h->argv.

Re: [Libguestfs] [libnbd PATCH v3 18/19] generator: Add APIs to get/set the socket activation socket name

2023-03-23 Thread Eric Blake
see_also = [Link "connect_systemd_socket_activation"; > + Link "get_socket_activation_name"]; > + }; > + > + "get_socket_activation_name", { > +default_call with > +args = []; ret = RString; > +shortdesc = "get

Re: [Libguestfs] [libnbd PATCH v3 19/19] socket activation: set LISTEN_FDNAMES

2023-03-23 Thread Eric Blake
he fourth parameter as an indication of an unset request (vs. "" when you want it set to the empty string)? At which point, you would drop the 'if (h->sact_name != NULL)', and just blindly use SACT_VAR_PUSH(, h->sact_name,). That has ripple effects earlier in the series to s

Re: [Libguestfs] [libnbd PATCH v3 19/19] socket activation: set LISTEN_FDNAMES

2023-03-24 Thread Eric Blake
eport it there (done by adding in cc here). They did not specify LISTEN_FDNAMES at the time LISTEN_PID was first documented, so the likelihood of libnbd not being the only application that happens to leak inherited LISTEN_FDNAMES through to the child process is non-zero, where this sort of bug wi

Re: [Libguestfs] [libnbd PATCH v3 19/19] socket activation: set LISTEN_FDNAMES

2023-03-24 Thread Eric Blake
will be seen by the server'. Agreed that case (4) (supplying an explicit "unkown") is the best path forward, and with your analysis that we are not only adding a feature, but fixing a latent bug which existed ever since systemd a

Re: [Libguestfs] [PATCH 1/1] nbd/server: push pending frames after sending reply

2023-03-24 Thread Eric Blake
/* It wasn't -EIO, so, according to nbd_co_receive_request() > * semantics, we should return the error to the client. */ > @@ -2692,6 +2694,7 @@ static coroutine_fn void nbd_trip(void *opaque) > goto disconnect; > } > > +qio_channel_set_co

Re: [Libguestfs] [PATCH 1/1] nbd/server: push pending frames after sending reply

2023-03-24 Thread Eric Blake
On Fri, Mar 24, 2023 at 02:41:20PM -0500, Eric Blake wrote: > On Fri, Mar 24, 2023 at 11:47:20AM +0100, Florian Westphal wrote: > > qemu-nbd doesn't set TCP_NODELAY on the tcp socket. Replying to myself, WHY aren't we setting TCP_NODELAY on the socket? > > And su

Re: [Libguestfs] [libnbd PATCH v5 3/4] generator: Add APIs to get/set the socket activation socket name

2023-03-27 Thread Eric Blake
t we are going to pass "unknown" if "socket_name" is set > to the empty string > > <http://mid.mail-archive.com/oqwjnjvq4phqr76yum6zo5erfrm3tvmyewr5nxru3oxklobpgp@4plkku7opujw>.] > > Signed-off-by: Laszlo Ersek > Reviewed-by: Eric Blake > --- > &

Re: [Libguestfs] libnbd | Failed pipeline for master | 2db30279

2023-03-28 Thread Eric Blake
ve a race window in that test, perhaps a non-unique socket name? At any rate, it is transient - which is good for getting green status, but annoying for reproducing to be able to get to root cause and prevent future failures. -- Eric Blake, Principal Software Engineer Red Hat, Inc.

Re: [Libguestfs] [nbdkit PATCH 1/2] common/utils: document empty_vector compound literal assignment

2023-03-28 Thread Eric Blake
t; names = (string_vector)empty_vector; > > where "(string_vector)empty_vector" is a compound literal. > > Link: 20230221183810.vjilfbmj3woqivlj@redhat.com">http://mid.mail-archive.com/20230221183810.vjilfbmj3woqivlj@redhat.com > Suggested-by: Eric Blake >

Re: [Libguestfs] [nbdkit PATCH 2/2] plugins/rust: restrict predicates-{tree, core} to {1.0.7, 1.0.5}

2023-03-28 Thread Eric Blake
ard, but being cavalier about it is not doing your users a service. > > Signed-off-by: Laszlo Ersek > --- > plugins/rust/Cargo.toml | 2 ++ > 1 file changed, 2 insertions(+) I'm not a Rust expert, so I won't give R-b, but since it fixes the build, I can at least offer: A

Re: [Libguestfs] [PATCH nbdkit] tests: Add a test of floppy plugin + size parameter

2023-04-13 Thread Eric Blake
py-size.sh | 61 +++ > 2 files changed, 69 insertions(+), 2 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org _

[Libguestfs] [PATCH v3 0/6] NBD 64-bit extensions (spec only)

2023-04-13 Thread Eric Blake
ize' 003/6:[0237] [FC] 'spec: Add NBD_OPT_EXTENDED_HEADERS' 004/6:[] [-C] 'spec: Allow 64-bit block status results' 005/6:[] [-C] 'spec: Introduce NBD_FLAG_BLOCK_STATUS_PAYLOAD' 006/6:[0062] [FC] 'RFC: spec: Introduce NBD_REPLY_TYPE_OFFSET_HOLE_EXT' Er

[Libguestfs] [PATCH v3 1/6] spec: Recommend cap on NBD_REPLY_TYPE_BLOCK_STATUS length

2023-04-13 Thread Eric Blake
s, where a client's request is no longer bounded to 4G and could thereby produce even more than 8M extents for the corner case when every 512 bytes is a new extent, if it were not for this recommendation. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- v2: Add wording abou

[Libguestfs] [PATCH v3 5/6] spec: Introduce NBD_FLAG_BLOCK_STATUS_PAYLOAD

2023-04-13 Thread Eric Blake
express dynamic interest in varying subsets of those contexts over the life of the connection, for less wasted effort in responding to NBD_CMD_BLOCK_STATUS. This works by having the command payload supply an effect length and a list of ids the client is currently interested in. Signed-off-by: Eric

[Libguestfs] [PATCH v3 6/6] RFC: spec: Introduce NBD_REPLY_TYPE_OFFSET_HOLE_EXT

2023-04-13 Thread Eric Blake
Rather than requiring all servers and clients to have a 32-bit limit on maximum NBD_CMD_READ/WRITE sizes, we can choose to standardize support for a 64-bit single I/O transaction now. NBD_REPLY_TYPE_OFFSET_DATA can already handle a large reply, but NBD_REPLY_TYPE_OFFSET_HOLE needs a 64-bit counterp

[Libguestfs] [PATCH v3 2/6] spec: Change maximum block size to maximum payload size

2023-04-13 Thread Eric Blake
ter term, it means that a server may (but not must) accept a read request larger than the maximum block size if it can use structured replies to keep each chunk of the response under the maximum payload limits. Signed-off-by: Eric Blake --- v3: Completely drop 'maximum block length&#x

[Libguestfs] [PATCH v3 4/6] spec: Allow 64-bit block status results

2023-04-13 Thread Eric Blake
There are some potential extension metadata contexts that would benefit from a 64-bit status value. For example, Zoned Block Devices (see https://zonedstorage.io/docs/linux/zbd-api) may want to return the relative offset of where the next write will occur within the zone, where a zone may be large

[Libguestfs] [PATCH v3 3/6] spec: Add NBD_OPT_EXTENDED_HEADERS

2023-04-13 Thread Eric Blake
to whether extended headers were negotiated. This spec addition was done in parallel with proof of concept implementations in qemu (server and client), libnbd (client), and nbdkit (server). Signed-off-by: Eric Blake --- v3: prohibit NBD_OPT_EXPORT_NAME, clarify more regarding NBD_REP_ERR_EXT_H

Re: [Libguestfs] [PATCH libnbd v2] README: Document additional packages

2023-04-17 Thread Eric Blake
s internals, so I can't say anything > definitive here; I guess I'd propose putting autoconf and automake under > [2], and libtool under [1]. Rich, Eric? Libtool should also be in [2]; if it has shifted into [1], that's a bug that should reported to the u

Re: [Libguestfs] [PATCH v3 2/6] spec: Change maximum block size to maximum payload size

2023-04-18 Thread Eric Blake
On Tue, Apr 18, 2023 at 11:26:40AM +0200, Wouter Verhelst wrote: > On Thu, Apr 13, 2023 at 05:02:37PM -0500, Eric Blake wrote: > > Commit 9f30fedb improved the spec to allow non-payload requests like > > NBD_CMD_TRIM that exceed any advertised maximum block size. Take this > >

Re: [Libguestfs] [PATCH v3 3/6] spec: Add NBD_OPT_EXTENDED_HEADERS

2023-04-18 Thread Eric Blake
On Tue, Apr 18, 2023 at 02:04:31PM +0200, Wouter Verhelst wrote: > On Thu, Apr 13, 2023 at 05:02:38PM -0500, Eric Blake wrote: > > Add a new negotiation feature where the client and server agree to use > > larger packet headers on every packet sent during transmission phase. &

Re: [Libguestfs] [PATCH v3 6/6] RFC: spec: Introduce NBD_REPLY_TYPE_OFFSET_HOLE_EXT

2023-04-18 Thread Eric Blake
On Tue, Apr 18, 2023 at 02:33:41PM +0200, Wouter Verhelst wrote: > On Thu, Apr 13, 2023 at 05:02:41PM -0500, Eric Blake wrote: > > Rather than requiring all servers and clients to have a 32-bit limit > > on maximum NBD_CMD_READ/WRITE sizes, we can choose to standardize > >

Re: [Libguestfs] [libnbd PATCH 05/18] generator/states: wrap source code at 80 characters

2023-04-18 Thread Eric Blake
an oldstyle or fixed " > + "newstyle NBD server"); The term 'fixed newstyle' has enough relevance on its own in the protocol that I might wrap one word earlier to keep that phrasing together; but I'm not opposed to your patch going in as-is. -

Re: [Libguestfs] [libnbd PATCH 11/18] tests/closure-lifetimes: wrap source code at 80 characters

2023-04-19 Thread Eric Blake
27; as 'done thrice' is less frequently heard). Another alternative is 'repeated'. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 08/18] lib/internal.h: wrap source code at 80 characters

2023-04-19 Thread Eric Blake
_NONNULL (1, 2); Looks reasonable to me. emacs may have a bit of a difficulty in auto-indenting this back to the same place, but it is infrequent enough that manual override is still viable. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 16/18] tests/requires: wrap source code at 80 characters

2023-04-19 Thread Eric Blake
e seeing this message). > +"then\n" > +"exit 1\n" > +"else\n" > +"exit 0\n" > +"fi\n", For that matter, 'if ...; then exit 1; else exit 0; fi' can be compressed to '! ...'. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 00/18] wrap hand-written source code at 80 characters

2023-04-19 Thread Eric Blake
er generated lines for faster coding of the generator is a tolerable tradeoff in my book. Overall, the series looked okay to me at a first read through; I did spot some things on individual patches where I made comments, but they are of the nature where I'm also okay with you adding

Re: [Libguestfs] [libnbd PATCH v2] tests/requires: wrap source code at 80 characters

2023-04-19 Thread Eric Blake
> fit in 80 columns. > > At Eric's suggestion, also: > > - replace the non-portable control operator "|&" with the portable > redirection operator "2>&1" and the portable control operator "|"; > > - replace the "if ...;

Re: [Libguestfs] [libnbd PATCH 16/18] tests/requires: wrap source code at 80 characters

2023-04-19 Thread Eric Blake
compressed to '! ...'. > > > > Yes, I noticed that too, but I assumed there was a finer point I was > missing. :/ autoconf still avoids it because of portability to Solaris /bin/sh (not /bin/xpg4/sh), which lacked !. But that's ancient history these days; ALL mod

Re: [Libguestfs] [libnbd PATCH 16/18] tests/requires: wrap source code at 80 characters

2023-04-19 Thread Eric Blake
t that system() does not use 'set -e' automatically. Then again, since shell snippes run in Makefile ARE supposed to use implicit set -e, it never hurts to be vigilant about context when looking at shell snippets embedded in a larger file. -- Eric Bla

[Libguestfs] [PATCH] spec: Document extended headers extension branch

2023-04-24 Thread Eric Blake
while, structured replies and block status have been incorporated into mainline for some time now (see commit 6e896bca in 2018, v3.17); and while block status is still not implemented in the kernel module or in nbd-server, we did just recently implement structured replies in nbd-server. Signed-off-by:

Re: [Libguestfs] [PATCH v3 6/6] RFC: spec: Introduce NBD_REPLY_TYPE_OFFSET_HOLE_EXT

2023-04-24 Thread Eric Blake
cumented at https://github.com/NetworkBlockDevice/nbd/blob/extension-ext-header/doc/proto.md. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.

Re: [Libguestfs] [libnbd PATCH 11/23] generator/C: print_trace_enter: break "enter" to a common new line

2023-04-25 Thread Eric Blake
handle *h) > > > >pthread_mutex_lock (&h->lock); > >if_debug (h) { > > -debug (h, "enter:"); > > +debug (h, > > + "enter:"); Looks a little funny to not wrap here; but it's generated code, and the context a

Re: [Libguestfs] [libnbd PATCH 22/23] generator/C: print_wrapper: use helper variable for permitted state check

2023-04-26 Thread Eric Blake
OCaml involved (I may have missed something), but the generated code does look nicer after this series. Feel free to add Reviewed-by: Eric Blake throughout the series when pushing. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvi

Re: [Libguestfs] [PATCH nbdkit 2/2] allocators: sparse: Split the highly contended mutex

2023-04-27 Thread Eric Blake
ght than bare mutex, which explains the slowdown on the read-heavy workload even while improving the write-heavy workload. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org _

Re: [Libguestfs] [libnbd PATCH v2 0/2] continue wrapping generated C code harder

2023-05-02 Thread Eric Blake
next to the top > of the function. Series: Reviewed-by: Eric Blake > > Thanks! > Laszlo > > Laszlo Ersek (2): > generator/C: print_wrapper: use helper variable for permitted state > check > generator/C: lib/api.c: indent arg list 2 spaces relative to function &g

Re: [Libguestfs] [PATCH nbdkit] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-08 Thread Eric Blake
>/* Check all debug flags were used, and free them. */ >free_debug_flags (); > > +#ifndef WIN32 > + /* Dump the environment if asked. This is the earliest we can do it > + * because it uses a debug flag. > + */ > + if (nbdkit_debug_environ && verbose) > +

Re: [Libguestfs] [PATCH libnbd] tests: Add a test of nbd_{set, get}_socket_activation_name

2023-05-08 Thread Eric Blake
ot;, > +NULL > + }; > + if (nbd_connect_systemd_socket_activation (nbd, cmd) == -1) { > +fprintf (stderr, "%s\n", nbd_get_error ()); > +exit (EXIT_FAILURE); > + } Should we run this twice, once with a set name (as you did here, to prove LISTEN_FDNAMES=hell

Re: [Libguestfs] [PATCH libnbd] tests: Add a test of nbd_{set, get}_socket_activation_name

2023-05-08 Thread Eric Blake
d into the usual 8-bit position. It's not required by POSIX, but you can generally get away with relying on 'rc==0' implies 'WIFEXITED(rc) && WEXITSTATUS(rc)==0' on all platforms. > > Also, should we invoke this "grep" with "-sq" as well (at

Re: [Libguestfs] [PATCH nbdkit] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-08 Thread Eric Blake
mically run by code that the compiler didn't see). This is all way outside of the bounds of what POSIX says, so at best we are just relying on "does it work for how we are using it" - but since we are only using it for debugging, where it is doing what we want, I'm not too wor

Re: [Libguestfs] [PATCH nbdkit v3 2/6] server: debug: Don't emit ANSI colour codes on Windows

2023-05-09 Thread Eric Blake
vfprintf (fp, fs, args); > > +#ifndef WIN32 >if (!in_server && tty) ansi_force_restore (fp); > +#endif and this one can also be unconditional. But what you have works too, so I'm fine either way. -- Eric Blake, Principal Software Engineer Red Hat, Inc.

Re: [Libguestfs] [PATCH nbdkit v3 3/6] common/utils: Add C string quoting function

2023-05-09 Thread Eric Blake
that for this patch. But if we need to add it in the future, it will be easier to rejigger the loop as (abbreviated, but you get the idea): for (i = 0; c = str[i], c != '\0'; ++i) { switch (c) { case '\\': fputc ('\\'); fpu

Re: [Libguestfs] [PATCH nbdkit v3 4/6] server: debug: Return earlier if not verbose

2023-05-09 Thread Eric Blake
6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Good thing we don't insist on C89 declaration before statement. This is one case where a late declaration makes sense. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 V

Re: [Libguestfs] [PATCH nbdkit v3 5/6] server: debug: Fix error handling

2023-05-09 Thread Eric Blake
-- > 1 file changed, 16 insertions(+), 12 deletions(-) Reviewed-by: Eric Blake > + fail: > + /* Try to emit what we can. */ > + errno = err; > + vfprintf (stderr, fs, args); > + fprintf (stderr, "\n"); >errno = err; It might make more sense if the first

Re: [Libguestfs] [PATCH nbdkit v3 6/6] server: debug: Escape debug strings

2023-05-09 Thread Eric Blake
si_force_restore (fp_outer); > #endif > - fprintf (fp, "\n"); > - if (close_memstream (fp) == -1) > + fprintf (fp_outer, "\n"); > + if (close_memstream (fp_outer) == -1) Affects multiple patches: close_memstream() (on Linux) fails with EOF, which happens

Re: [Libguestfs] [libnbd PATCH 2/6] generator/utils: add "pr_wrap_c_comment"

2023-05-10 Thread Eric Blake
d the transition Otherwise, well-commented, and looks like it meets the intent. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailin

Re: [Libguestfs] [libnbd PATCH 3/6] state_machine_generator: wrap state comments in lib/states.{h, c}

2023-05-10 Thread Eric Blake
rce the wrap one word earlier in this particular instance without adding even more complexity. So I'm okay with how it ended up. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 0/6] finish wrapping generated C code harder

2023-05-10 Thread Eric Blake
te_machine_generator: wrap enter_*() calls and prototypes > state_machine_generator: rename, and break up the init. of, > "next_state" For those without explicit comments, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [libnbd PATCH 3/6] state_machine_generator: wrap state comments in lib/states.{h, c}

2023-05-11 Thread Eric Blake
On Thu, May 11, 2023 at 08:45:25AM +0200, Laszlo Ersek wrote: > On 5/10/23 17:14, Eric Blake wrote: > > On Wed, May 10, 2023 at 01:48:11PM +0200, Laszlo Ersek wrote: > >> Wrap those comments in "lib/states.h" and "lib/states.c" that describe the > >>

[Libguestfs] [PATCH v3 02/14] nbd/client: Add safety check on chunk payload length

2023-05-15 Thread Eric Blake
length, we should drop the connection right then rather than assuming the layer on top will be careful. This becomes more important when we permit 64-bit lengths which are even more likely to have the potential for attempted denial of service abuse. Signed-off-by: Eric Blake --- nbd/client.c

[Libguestfs] [PATCH v3 05/14] nbd: Add types for extended headers

2023-05-15 Thread Eric Blake
patch does not change the status quo that neither the client nor server use a packed-struct representation for the request header. Signed-off-by: Eric Blake --- docs/interop/nbd.txt | 1 + include/block/nbd.h | 74 nbd/common.c | 10

[Libguestfs] [PATCH v3 03/14] nbd/server: Prepare for alternate-size headers

2023-05-15 Thread Eric Blake
-by: Eric Blake --- include/block/nbd.h | 8 +++--- nbd/server.c| 64 - 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index a4c98169c39..f1d838d24f5 100644 --- a/include/block/nbd.h

[Libguestfs] [PATCH v3 06/14] nbd/server: Refactor handling of request payload

2023-05-15 Thread Eric Blake
: Eric Blake --- nbd/server.c | 55 +--- nbd/trace-events | 1 + 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index cf38a104d9a..5812a773ace 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -2316,6 +2316,8

[Libguestfs] [PATCH v3 04/14] nbd: Prepare for 64-bit request effect lengths

2023-05-15 Thread Eric Blake
hether the client successfully negotiated extended headers with the server, allowing the nbd driver to pass larger requests along where possible; although in this patch the enum never surpasses structured replies, for no semantic change yet. Signed-off-by: Eric Blake --- include/block/nbd.

[Libguestfs] [PATCH v3 01/14] nbd/client: Use smarter assert

2023-05-15 Thread Eric Blake
t;, v4.2.0) Reported-by: Dr. David Alan Gilbert Signed-off-by: Eric Blake --- Looking through older branches, I came across this one that was never applied at the time, but which also had a useful review comment from Vladimir that invalidates the R-b it had back then. v2 was here: https://lists.

[Libguestfs] [PATCH v3 07/14] nbd/server: Refactor to pass full request around

2023-05-15 Thread Eric Blake
the buffer is also available). This is a refactoring patch to pass the full request around the reply stack, rather than just the handle, so that later patches can then access request->from when extended headers are active. But for this patch, there are no semantic changes. Signed-off-b

[Libguestfs] [PATCH v3 00/14] qemu patches for 64-bit NBD extensions

2023-05-15 Thread Eric Blake
size, which in turn required rearranging server patches a bit - other changes that I noticed while testing with parallel changes being added to libnbd (link to those patches to follow in the next week or so) Eric Blake (14): nbd/client: Use smarter assert nbd/client: Add safety check on

[Libguestfs] [PATCH v3 10/14] nbd/client: Initial support for extended headers

2023-05-15 Thread Eric Blake
insist that the server is compliant. The only caller to nbd_receive_reply() always passed NULL for errp; since we are changing the signature anyways, I decided to sink the decision to ignore errors one layer lower. Signed-off-by: Eric Blake --- include/block/nbd.h | 2 +- block/nbd.c

[Libguestfs] [PATCH v3 08/14] nbd/server: Support 64-bit block status

2023-05-15 Thread Eric Blake
ol does not allow sending a larger request without extended headers). This patch adds some assertions that ensure we continue to avoid overflowing 32 bits for a narrow client, while fully utilizing 64-bits all the way through when the client understands that. Signed-off-by:

[Libguestfs] [PATCH v3 09/14] nbd/server: Initial support for extended headers

2023-05-15 Thread Eric Blake
x27;s length is a hint, and the previous patch took care of implementing the required NBD_REPLY_TYPE_BLOCK_STATUS_EXT. Signed-off-by: Eric Blake --- nbd/nbd-internal.h | 5 +- nbd/server.c | 130 +++-- 2 files changed, 106 insertions(+), 29 dele

[Libguestfs] [PATCH v3 13/14] nbd/server: Prepare for per-request filtering of BLOCK_STATUS

2023-05-15 Thread Eric Blake
return on a per-command basis (for now, identical to the full set of negotiated contexts). Signed-off-by: Eric Blake --- include/block/nbd.h | 15 ++ nbd/server.c| 108 +++- 2 files changed, 72 insertions(+), 51 deletions(-) diff --git a/include

[Libguestfs] [PATCH v3 11/14] nbd/client: Accept 64-bit block status chunks

2023-05-15 Thread Eric Blake
checking whether more than the two least-significant bits are set. Signed-off-by: Eric Blake --- block/nbd.c| 39 --- block/trace-events | 1 + 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index d6caea4

[Libguestfs] [PATCH v3 12/14] nbd/client: Request extended headers during negotiation

2023-05-15 Thread Eric Blake
essential to the information collected during 'qemu-nbd --list', but probing for it gives us one more piece of information in that output. Update the iotests affected by the new line of output. Signed-off-by: Eric Blake --- block/nbd.c | 5 +--

[Libguestfs] [PATCH v3 14/14] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS

2023-05-15 Thread Eric Blake
server is advertising the capability, and to the testsuite to reflect the addition to that output. Signed-off-by: Eric Blake --- docs/interop/nbd.txt | 2 +- include/block/nbd.h | 32 -- nbd/server.c

Re: [Libguestfs] [PATCH v3 00/14] qemu patches for 64-bit NBD extensions

2023-05-15 Thread Eric Blake
Adding qemu-block for the cover letter (not sure how I missed that the first time). On Mon, May 15, 2023 at 02:53:29PM -0500, Eric Blake wrote: > > v2 was here: > https://lists.gnu.org/archive/html/qemu-devel/2022-11/msg02340.html > > Since then: > - upstream NBD has acce

Re: [Libguestfs] [PATCH nbdkit 3/5] common/include: Add next_power_of_2 function

2023-05-16 Thread Eric Blake
00) == 0x1); > + assert (next_power_of_2 (UINT64_C ( 0x)) == 0x1); > + assert (next_power_of_2 (UINT64_C (0x1)) == 0x1); > + assert (next_power_of_2 (UINT64_C (0x20001)) == 0x4); > + assert (next_power_of_2 (UINT64_C (0x6ffff)) == 0x

Re: [Libguestfs] [PATCH nbdkit 4/5] New plugin: ones

2023-05-16 Thread Eric Blake
pre-initialized to a repetition of a given byte. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit 5/5] New filter: evil

2023-05-16 Thread Eric Blake
:2000]) > + > +buf1 = h.pread(1, 999) > +assert(buf1 == buf[999:10999]) Should we also assert that there is at least one stuck bit in those two ranges, and/or pick a different or larger range to improve the chances of that being the case? > +++ b/tests/test-evil-stuck-wires.

Re: [Libguestfs] [PATCH nbdkit 5/5] New filter: evil

2023-05-16 Thread Eric Blake
seed to deterministically know where the stuck bits will be, but it has to be separate from tests where we want the pseudo-random behavior to differ between runs but still demonstrate that our approximations are getting around the ratio of set bits that we expected. -- Eric Bl

Re: [Libguestfs] [PATCH nbdkit 1/5] Add new public nbdkit_parse_probability function

2023-05-16 Thread Eric Blake
es me feel very uncomfortable. It's more than just scanf() lacking > proper internal error handling; the real complications come when we > perform operations on the floating point values. Our pre-existing error filter is already doing percentage calculations via floating point, s

Re: [Libguestfs] [PATCH nbdkit v2 1/6] Add new public nbdkit_parse_probability function

2023-05-17 Thread Eric Blake
ide note: if you really want a trip, read the 2023 SIGBOVIK article on "GradIEEEnt half decent" about 16-bit floating point values being exploited for their non-linear rounding properties as a way to create non-monotonic functions that can in turn form the basis of a Turing complete system c

Re: [Libguestfs] [PATCH nbdkit v2 2/6] error: Use new nbdkit_parse_probability

2023-05-17 Thread Eric Blake
on success, then then left half of this conditional would now be dead code. Otherwise, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs

Re: [Libguestfs] [PATCH nbdkit v2 3/6] common/include: Add next_power_of_2 function

2023-05-17 Thread Eric Blake
On Wed, May 17, 2023 at 11:06:56AM +0100, Richard W.M. Jones wrote: > It takes a 64 bit integer and finds the next power of 2, > eg. next_power_of_2 (510) => 512 (2^9) > > Taken from https://jameshfisher.com/2018/03/30/round-up-power-2/ > with some fixes. > > Thanks: Er

Re: [Libguestfs] [PATCH nbdkit v2 4/6] common/include: Make log_2_bits work on 64 bit ints

2023-05-17 Thread Eric Blake
es where is_power_of_2() returns true. We should probably take this opportunity to either document that it is not just 0, but all non-power-of-2 values that have undefined behavior; or else to test some values with more than one bit set. --

Re: [Libguestfs] [PATCH nbdkit v2 4/6] common/include: Make log_2_bits work on 64 bit ints

2023-05-17 Thread Eric Blake
spowerof2.c | 6 ++ > 2 files changed, 4 insertions(+), 6 deletions(-) Should we also fix is_power_of_2() to work on 64-bit values on all platforms? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu

Re: [Libguestfs] [PATCH nbdkit v2 5/6] New plugin: ones

2023-05-17 Thread Eric Blake
; + > +Instead of C it is more efficient to use > +L. Good catch. On v1, I suggested nbdkit-zero-plugin, but re-reading the difference between those two, I see you got the one I meant. (For the casual reader: zero-plugin has size zero, while the null plugin has non-zero size that reads as zero)

Re: [Libguestfs] [PATCH nbdkit v2 6/6] New filter: evil

2023-05-17 Thread Eric Blake
; + > +static uint8_t > +corrupt_one_bit (uint8_t byte, unsigned bit, > + uint64_t rand, enum corruption_type ct) Another use of 'rand' that might be confusing given the function. > +{ > + const unsigned mask = 1 << bit; > + > + switch (ct) { > + case FLIP: > +byte ^= mask; > +break; > + case STUCK: > +rand &= 0x; > +if (evil_stuck_probability * 0x1 > rand) { > + if (rand & 1) /* stuck high or low? */ > +byte |= mask; > + else > +byte &= ~mask; > +} > + } > + return byte; > +} And I ran out of review time today, before finishing reading the rest of your patch. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit v2 5/6] New plugin: ones

2023-05-18 Thread Eric Blake
is plugin, errno is preserved properly along error return > + * paths from failed system calls. > + */ > + .errno_is_preserved = 1, > +}; ...we probably also want to supply .can_fast_zero with a function that returns true. -- Eric Blake, Principal Software Engineer Red Hat,

Re: [Libguestfs] [PATCH nbdkit v2 6/6] New filter: evil

2023-05-18 Thread Eric Blake
d commit what you've cleaned up based on these reviews, and we can do any further cleanups as followups rather than needing to see a full v3 posting. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

[Libguestfs] [libnbd PATCH] maint: pick consistent spacing style for casts

2023-05-23 Thread Eric Blake
e is whitespace only. [1] https://listman.redhat.com/archives/libguestfs/2023-February/030771.html Signed-off-by: Eric Blake --- configure.ac | 6 ++-- common/utils/vector.h| 4 +-- generator/OCaml.ml | 4 +-- g

Re: [Libguestfs] [libnbd PATCH] maint: pick consistent spacing style for casts

2023-05-24 Thread Eric Blake
7;ve checked that the compiler does not complain if we remove casts that only serve to explicitly document that we are removing const when passing a string literal to a place that wants a 'char *' but will otherwise not modify the contents. -- Eric Blake, Principal Software Engi

[Libguestfs] [libnbd PATCH] maint: Drop useless casts

2023-05-24 Thread Eric Blake
eded casts lets the remaining casts stand out for why they are necessary: casting aways const, casts between pointer aliases (such as struct sockaddr*, char* vs unsigned char*, ...), and scalars passed through vargs in printf-like functions. Signed-off-by: Eric Blake --- generator/Python.ml

Re: [Libguestfs] [libnbd PATCH] maint: pick consistent spacing style for casts

2023-05-24 Thread Eric Blake
On Wed, May 24, 2023 at 07:39:20AM +0200, Laszlo Ersek wrote: > On 5/24/23 04:12, Eric Blake wrote: > > We are inconsistent on whether a cast operator should be separated > > from its argument by a space. As a unary prefix operator, casts bind > > with relatively tight p

Re: [Libguestfs] [libnbd PATCH] maint: Drop useless casts

2023-05-24 Thread Eric Blake
On Wed, May 24, 2023 at 08:18:32PM +0100, Richard W.M. Jones wrote: > On Wed, May 24, 2023 at 10:00:03AM -0500, Eric Blake wrote: > > diff --git a/copy/main.c b/copy/main.c > > index 391c0c4f..9449440e 100644 > > --- a/copy/main.c > > +++ b/copy/main.c > > @@ -371

[Libguestfs] [libnbd PATCH v3 15/22] info: Update nbdinfo --map to use 64-bit block status

2023-05-25 Thread Eric Blake
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 ---

[Libguestfs] [libnbd PATCH v3 20/22] interop: Add test of 64-bit block status

2023-05-25 Thread Eric Blake
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 --- interop/Makefile.am

[Libguestfs] [libnbd PATCH v3 19/22] api: Add nbd_[aio_]opt_extended_headers()

2023-05-25 Thread Eric Blake
replies, regardless of which order the two are manually negotiated in. Signed-off-by: Eric Blake --- generator/API.ml | 79 +++-- .../states-newstyle-opt-extended-headers.c| 30 +++- generator/states-newstyle.c | 3 + lib/opt.c

[Libguestfs] [libnbd PATCH v3 12/22] copy: Update nbdcopy to use 64-bit block status

2023-05-25 Thread Eric Blake
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 --- copy/nbd-ops.c | 22 +++

[Libguestfs] [libnbd PATCH v3 13/22] dump: Update nbddump to use 64-bit block status

2023-05-25 Thread Eric Blake
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 --- dump/dump.c | 27 ++- 1 file changed, 14 in

[Libguestfs] [libnbd PATCH v3 17/22] ocaml: Add example for 64-bit extents

2023-05-25 Thread Eric Blake
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 --- ocaml/examples/Makefile.am | 1 + ocaml/examples/extents64.ml

[Libguestfs] [libnbd PATCH v3 11/22] api: Add several functions for controlling extended headers

2023-05-25 Thread Eric Blake
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 --- lib/internal.h|

[Libguestfs] [libnbd PATCH v3 03/22] protocol: Add definitions for extended headers

2023-05-25 Thread Eric Blake
for a later patch). [1] https://github.com/NetworkBlockDevice/nbd/blob/extension-ext-header/doc/proto.md Signed-off-by: Eric Blake --- lib/nbd-protocol.h | 66 -- 1 file changed, 52 insertions(+), 14 deletions(-) diff --git a/lib/nbd-protocol.h b/lib

[Libguestfs] [libnbd PATCH v3 00/22] NBD 64-bit extensions (libnbd portion)

2023-05-25 Thread Eric Blake
' 021/22:[0011] [FC] 'api: Add nbd_can_block_status_payload()' 022/22:[0004] [FC] 'api: Add nbd_[aio_]block_status_filter()' The changes are mostly fallout from rebasing on top of style cleanups that Laszlo has been working on, but also deal with some changes with the spec

[Libguestfs] [libnbd PATCH v3 08/22] block_status: Track 64-bit extents internally

2023-05-25 Thread Eric Blake
the new 64-bit interfaces as more and more servers start supporting extended headers. One of the trickier aspects of this patch is auditing that both the user's extent and our malloc'd shim get cleaned up once on all possible paths, so that there is neither a leak nor a double free.

[Libguestfs] [libnbd PATCH v3 22/22] api: Add nbd_[aio_]block_status_filter()

2023-05-25 Thread Eric Blake
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 s

[Libguestfs] [libnbd PATCH v3 02/22] internal: Refactor layout of replies in sbuf

2023-05-25 Thread Eric Blake
many systems prefer that any struct with a uint64_t provide 8-byte alignment to its containing union. The commit is largely mechanical, and there should be no semantic change. Signed-off-by: Eric Blake --- lib/internal.h | 12 ++-- generator/states-reply-simple.c | 4

[Libguestfs] [libnbd PATCH v3 09/22] block_status: Accept 64-bit extents during block status

2023-05-25 Thread Eric Blake
the two names makes it easier to reason about when values are still in network byte order from the server or native endian for local processing. Signed-off-by: Eric Blake --- lib/internal.h | 3 + generator/states-reply-structured.c | 94 +++--

[Libguestfs] [libnbd PATCH v3 07/22] generator: Add struct nbd_extent in prep for 64-bit extents

2023-05-25 Thread Eric Blake
zonedstorage.io/docs/linux/zbd-api Signed-off-by: Eric Blake --- generator/API.mli | 1 + generator/API.ml| 12 +++- generator/C.ml | 24 +--- generator/GoLang.ml | 24 generator/OCaml.ml | 24 +--- gene

<    1   2   3   4   5   6   7   8   9   10   >