Re: [Libguestfs] [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.