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