Re: [Libguestfs] [PATCH v3 6/6] RFC: spec: Introduce NBD_REPLY_TYPE_OFFSET_HOLE_EXT
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(a)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