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

[Eric Blake]

On Thu, Feb 17, 2022 at 09:52:52AM +0000, Richard W.M. Jones wrote:
> 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?

Perhaps on the nbd passthrough plugin.  After all, if strict is false,
the plugin wants to use the libnbd API to request that we do NOT
request strict block size request from the destination, because we
know the client will not be necessarily be honoring strict requests.

> 
> 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?

You do have a point there - besides the nbd passthrough plugin, it's
hard to see how other plugins may care about whether the client
intends to obey the advertised values (either we can support
out-of-spec requests as a bonus, like qemu-nbd can, or we can't).

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

If for no other reason, letting us emulate the fact that qemu-nbd
currently advertises different numbers based on the client.
Interoperability testing is easier when we can reproduce what other
servers do.

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