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

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.