Re: [RFC v2] nbd/proto: introduce extended request and 64bit commands
17.03.2020 19:18, Wouter Verhelst wrote:
Hi Vladimir,
Sorry for the delay; I got married late last month (yay!), so obviously
I was a little preoccupied ;-)
Congratulations!! Be happy!
On Fri, Feb 28, 2020 at 01:22:43PM +0300, Vladimir Sementsov-Ogievskiy wrote:
ping
06.02.2020 18:15, Vladimir Sementsov-Ogievskiy wrote:
[...]
+The extended request message, sent by the client, looks as follows:
+
+C: 32 bits, 0x23876289, magic (`NBD_EXTENDED_REQUEST_MAGIC`)
+C: 16 bits, flags
+C: 16 bits, type
+C: 64 bits, handle
+C: 32 bits, length of payload (unsigned)
+C: *length* bytes of payload data (if *length* is nonzero)
[...]
A client MUST NOT send a trim request unless `NBD_FLAG_SEND_TRIM`
was set in the transmission flags field.
+ `NBD_CMD_TRIM` supports extended requests, with the following
+ 16-bytes payload:
+
+ 64 bits: offset
+ 64 bits: length
+
* `NBD_CMD_CACHE` (5)
A cache request. The client is informing the server that it plans
@@ -2095,6 +2127,12 @@ The following request types exist:
including one or more sectors beyond the size of the device. It SHOULD
return `NBD_EPERM` if it receives a write zeroes request on a read-only export.
+ `NBD_CMD_WRITE_ZEROES` supports extended requests, with the following
+ 16-bytes payload:
+
+ 64 bits: offset
+ 64 bits: length
+
* `NBD_CMD_BLOCK_STATUS` (7)
A block status query request. Length and offset define the range
@@ -2154,6 +2192,12 @@ The following request types exist:
`NBD_EINVAL` if it receives a `NBD_CMD_BLOCK_STATUS` request including
one or more sectors beyond the size of the device.
+ `NBD_CMD_BLOCK_STATUS` supports extended requests, with the following
+ 16-bytes payload:
+
+ 64 bits: offset
+ 64 bits: length
I can't actually think of any command that would not require offset and
length fields; and given my quote, it would appear neither can you.
Hmm. What about RESIZE? May be, some kind of REOPEN..
Given that, wouldn't it make more sense to have the offset and length
fields be part of the extended request message, and to keep the payload
empty for messages that don't actually send any data along? That would
make the handling for such messages easier to do, too. In other words,
make the extended request message have a magic, flags, type, handle,
payload length, offset, and "affected length" field, and leave payload
for actual data.
But why not to keep it more portable? What is a benefit? I see the drawback:
if we define it with offset/length, than, when we'll want to implement a new
command without them, it will be incompatible extension..
If somehow we do end up with a message that does not need the offset or
length fields, we can then always mark those as reserved and make the
server ignore them.
We may just invent a name for offset/lenght extended requests, something like
"standard extended request", and instead of
>>> + `***` supports extended requests, with the following
>>> + 16-bytes payload:
>>> +
>>> + 64 bits: offset
>>> + 64 bits: length
several times we'll have just
>>> + `***` supports standarad extended requests
And we'll need additional paragraph, defining standard extended request, including
its layout (64 bits offset and 64 bits length).
...
Or what you mean by "reserved"? Just treat commands with no offset/length as unknown
commands? But this adds nothing to the spec actually.
So, do you mean, document that all extended commands have offset and length, but note
that "in future, new commands may be added without these fields" ?
--
Best regards,
Vladimir
Reply to: