Re: [PATCH v3 6/6] RFC: spec: Introduce NBD_REPLY_TYPE_OFFSET_HOLE_EXT

[Eric Blake]

On Tue, Apr 18, 2023 at 02:33:41PM +0200, Wouter Verhelst wrote:
> On Thu, Apr 13, 2023 at 05:02:41PM -0500, Eric Blake wrote:
> > Rather than requiring all servers and clients to have a 32-bit limit
> > on maximum NBD_CMD_READ/WRITE sizes, we can choose to standardize
> > support for a 64-bit single I/O transaction now.
> > NBD_REPLY_TYPE_OFFSET_DATA can already handle a large reply, but
> > NBD_REPLY_TYPE_OFFSET_HOLE needs a 64-bit counterpart.
> > 
> > By standardizing this, all clients must be prepared to support both
> > types of hole type replies, even though most server implementations of
> > extended replies are likely to only send one hole type.
> 
> I think it's probably a better idea to push this patch to a separate
> "extension-*" branch, and link to that from proto.md on master. Those
> are documented as "we standardized this, but no first implementor exists
> yet".
> 
> If someone actually comes up with a reason for 64-bit transactions, we
> can then see if the spec matches the need and merge it to master.

Okay, based on that, I'll merge this in a new branch, at which point
we WILL have something definitive to point to as a way to unjam the
patches to libnbd and qemu (they were waiting in part on having a spec
close enough to final).  I think patches 1 and 2 are ready for
mainstream, and only 3 and later need the extension branch.  I'm also
very reluctant to check patch 6 into the extension branch for now
(having it just be in the mail archives is good enough), since I have
not yet played with making qemu or libnbd support payloads larger than
64M, and am not sure if it ever makes sense to try to do an
NBD_CMD_READ or NBD_CMD_WRITE with more than 4G of material at once.
For that matter, ssize_t constraints to send() and recv() may make it
impractical to ever allow a maximum payload larger than 2G.

> 
> Otherwise this feels too much like a solution in search of a problem to
> me.

Rich has already followed up with some demonstrations of where larger
effect lengths can matter (on commands where effect length is
orthogonal to payload length).

> 
> With that said, for the things I didn't reply to, you can add:
> 
> Reviewed-By: Wouter Verhelst <w@uter.be>

I've replied to your other reviews with a couple of ideas to squash
in.  The insistence on only a 64-bit block status reply when extended
headers are in effect will have the most ripple effect on my
qemu/libnbd patches, but I don't think it is insurmountable, and agree
that insisting on extended headers mandating 64-bit responses is a bit
simpler than a client that has to handle both 32- and 64-bit responses
(a client may still need the complexity of handling both types in
order to talk to servers without extended headers, but that is a
different sort of complexity and can be phased out over time if lots
of servers decide to move towards 64-bit headers).

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