Libguestfs - Feb 2022 - [PATCH nbdkit 5/6] tests: Add a simple test for block size constraints

On Wed, Feb 16, 2022 at 04:20:40PM +0000, Richard W.M. Jones
wrote:
> ---
>  tests/Makefile.am                    |  4 +++
>  tests/test-block-size-constraints.sh | 50 ++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
ACK.  But food for thought:
> +requires_plugin eval
> +requires nbdsh -c 'print(h.get_block_size)'
> +
> +# Create an nbdkit eval plugin which presents block size constraints.
> +# Check the advertised block size constraints can be read.
> +nbdkit -U - eval \
> +       block_size="echo 64K 128K 32M" \
> +       get_size="echo 0" \
> +       --run 'nbdsh \
> +           -u "$uri" \
> +           -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 *
1024" \
> +           -c "assert h.get_block_size(nbd.SIZE_PREFERRED) == 128 *
1024" \
> +           -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 * 1024
* 1024" \
The NBD protocol says that if a client requests block size, it should
abide by the constraints (even though we have proven that not all
clients do); but if a client does NOT request block size, the server
can still advertise sizes but must be prepared for clients to ignore
it.  When I first tweaked libnbd to unconditionally ask for block size
as a client, I did not consider the subtle difference this presents,
but at least qemu-nbd has code where it will advertise different sizes
based on whether the client requested size vs. did not.  So we
probably need to add a knob to libnbd to control whether it will
request sizes, at which point this test in nbdkit will need to ensure
the libnbd knob is in the right position (I haven't yet decided
whether it makes more sense to request block sizes by default with the
knob allowing a client to not request, or to not request by default
such that the explicit request means you are writing a client that
intends to comply).

But now that I've written that, it also means that if the plugin
provides .block_size, we may want to advertise those values even to
clients that did not request the info.  Furthermore, we may need a
boolean that we pass _in_ to .block_size that says whether we are
grabbing the information in a non-binding manner (the client didn't
request size), or in a strict manner (the client requested size, and
promises to obey it).  That is, a signature of

  int (*block_size) (struct context *, bool strict,
                     uint32_t *minimum, uint32_t *preferred, uint32_t *maximum);

where strict corresponds to whether the client requested NBD_INFO_BLOCK_SIZE.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Wed, Feb 16, 2022 at 12:36:16PM -0600, Eric Blake
wrote:
> On Wed, Feb 16, 2022 at 04:20:40PM +0000, Richard W.M. Jones wrote:
> > ---
> >  tests/Makefile.am                    |  4 +++
> >  tests/test-block-size-constraints.sh | 50
++++++++++++++++++++++++++++
> >  2 files changed, 54 insertions(+)
> 
> ACK.  But food for thought:
> 
> > +requires_plugin eval
> > +requires nbdsh -c 'print(h.get_block_size)'
> > +
> > +# Create an nbdkit eval plugin which presents block size constraints.
> > +# Check the advertised block size constraints can be read.
> > +nbdkit -U - eval \
> > +       block_size="echo 64K 128K 32M" \
> > +       get_size="echo 0" \
> > +       --run 'nbdsh \
> > +           -u "$uri" \
> > +           -c "assert h.get_block_size(nbd.SIZE_MINIMUM) == 64 *
1024" \
> > +           -c "assert h.get_block_size(nbd.SIZE_PREFERRED) ==
128 * 1024" \
> > +           -c "assert h.get_block_size(nbd.SIZE_MAXIMUM) == 32 *
1024 * 1024" \
> 
> The NBD protocol says that if a client requests block size, it should
> abide by the constraints (even though we have proven that not all
> clients do); but if a client does NOT request block size, the server
> can still advertise sizes but must be prepared for clients to ignore
> it.  When I first tweaked libnbd to unconditionally ask for block size
> as a client, I did not consider the subtle difference this presents,
> but at least qemu-nbd has code where it will advertise different sizes
> based on whether the client requested size vs. did not.  So we
> probably need to add a knob to libnbd to control whether it will
> request sizes, at which point this test in nbdkit will need to ensure
> the libnbd knob is in the right position (I haven't yet decided
> whether it makes more sense to request block sizes by default with the
> knob allowing a client to not request, or to not request by default
> such that the explicit request means you are writing a client that
> intends to comply).
> 
> But now that I've written that, it also means that if the plugin
> provides .block_size, we may want to advertise those values even to
> clients that did not request the info.  Furthermore, we may need a
> boolean that we pass _in_ to .block_size that says whether we are
> grabbing the information in a non-binding manner (the client didn't
> request size), or in a strict manner (the client requested size, and
> promises to obey it).  That is, a signature of
> 
>   int (*block_size) (struct context *, bool strict,
>                      uint32_t *minimum, uint32_t *preferred, uint32_t
*maximum);
> 
> where strict corresponds to whether the client requested
NBD_INFO_BLOCK_SIZE.
This sounds like a very subtle distinction.  Is there a case where
it's actually useful?

Let's say we have a device that only supports whole sector reads and
writes (like VDDK).  If the client doesn't request block sizes, what
would we advertise for the "non-strict" case, given that if the client
sends < 512 byte we will fail anyway?

I'm trying to devise an example where a plugin would want to do this.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org