[Libguestfs] [nbdkit PATCH v2] main: Add option to disable SR advertisement

[Richard W.M. Jones]

On Tue, Aug 20, 2019 at 02:32:08PM -0500, Eric Blake wrote:
> On 8/20/19 2:28 PM, Eric Blake wrote:
> > When we added support for .extents, we had nbdkit unconditionally
> > support structured replies if the client requests them, and the
> > plugin's .can_extents has no impact on what the server advertises.
> > However, while the plugin API doesn't care whether the client
> > requested SR, there are still integration situations where not
> > advertising SR can be useful (such as comparison on what a client does
> > with no block status vs. a block status that always reports
> > allocated).  We already have the command line options -o/-n for
> > tweaking core server functionality; add --no-sr to the mix.
> > 
> > In particular, doing this found that 'qemu-nbd --list' from qemu 4.2
> > is rather picky: it hangs up on a server that replies with
> > NBD_REP_ERR_POLICY, rather than silently proceeding without SR support
> > (at least libnbd is more tolerant).
> > 
> > Signed-off-by: Eric Blake <eblake at redhat.com>
> > ---
> > 
> > Looks much different as a command line option instead of a hack to
> > the noextents filter, but I like the result a lot better.
> > 
> > I'm open to bike-shed opinions on the option name, or even if we want
> > to emulate a tri-state option --protocol=[old|no-sr|new] (with -o and
> > -n remaining synonyms for 2 of the 3 options for back-compat).  I
> > purposefully did not burn a short-option letter on the new option, as
> > it is unlikely to be commonly needed.
> > 
> >  docs/nbdkit-plugin.pod               |  5 +++-
> >  docs/nbdkit-protocol.pod             | 21 ++++++++++++----
> >  docs/nbdkit.pod                      | 12 ++++++++--
> >  docs/synopsis.txt                    |  2 +-
> >  server/internal.h                    |  1 +
> >  server/options.h                     |  4 +++-
> >  server/main.c                        |  5 ++++
> >  server/protocol-handshake-newstyle.c | 14 +++++++++++
> >  tests/test-eflags.sh                 | 36 ++++++++++++++++++++++++++++
> >  9 files changed, 91 insertions(+), 9 deletions(-)
> 
> Oh, and I missed squashing this in:
> 
> diff --git i/filters/noextents/nbdkit-noextents-filter.pod
> w/filters/noextents/nbdkit-noextents-filter.pod
> index 47223928..9cc586ff 100644
> --- i/filters/noextents/nbdkit-noextents-filter.pod
> +++ w/filters/noextents/nbdkit-noextents-filter.pod
> @@ -11,7 +11,11 @@ nbdkit-noextents-filter - disable extents in the
> underlying plugin
>  “Extents” are a feature of the NBD protocol / nbdkit which allow the
>  client to detect sparse regions of the underlying disk.
>  C<nbdkit-noextents-filter> disables this so that the plugin appears to
> -be fully allocated.
> +be fully allocated, at least to a client that requests structured
> +replies.  It is also possible to use the I<--no-sr> option to nbdkit
> +to prevent the client from using structured replies, which among its
> +other side effects will prevent the client from querying extents at
> +all.

Yes, the two patches squashed look good, ACK.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html