On Wed, Aug 02, 2023 at 08:50:43PM -0500, Eric Blake wrote: > 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 <ebl...@redhat.com> > Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> > --- > > v4: rebase to earlier changes, s/can/has/ [Rich] > ---
This one has been giving me some grief in my integration testing. It turns out that my qemu patches chose the following logic: if requesting NBD_OPT_INFO, advertise the block_status_payload feature unconditionally (regardless of whether NBD_OPT_SET_META_CONTEXT has been called); but when requesting NBD_OPT_GO, only advertise the block_status_payload feature when it is actually usable (that is, when the caller negotiated at least one metacontext). The upshot of that: 'nbdinfo -- [ qemu-nbd ... ] | grep payload' shows can_block_status_payload:true (because that was determined by NBD_OPT_INFO), but 'nbd --can block-status-payload -- [ qemu-nbd ... ]' shows false (because we skip straight to NBD_OPT_GO without requesting any meta contexts). I'll have to add in another patch to nbdinfo to prefer opt mode by default. > +++ 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)>. I'm second-guessing this naming. While '--can extended-headers' does not match any API name, and '--has extended-headers' reads nicely and corresponds to the fact that the condition is a property of the server as a whole (regardless of which export you connect to), '--can block-status-payload' better matches the API call as well as the fact that it is a per-export decision (in the qemu case, the feature is not present if you did not specifically negotiate meta-contexts for the export in question, at least for NBD_OPT_GO). Both spellings will still work, but I'll probably change this patch to stick to the --can naming from v3 (while leaving the --has naming for extended-headers) as the documented canonical spelling. > +++ b/interop/block-status-payload.sh ... > + > +# 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 This part is where I hit the integration failures. Just because qemu supports extended headers does not mean it supports block status payload (at least, not if you bisect my qemu patch series at the right point in time), even if it is true for a released version of qemu. But as I just found out, testing for --can block-status-payload is trickier unless I first teach nbdinfo to prefer NBD_OPT_INFO probing, even when not producing full output. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
