18.03.2020 8:33, Wouter Verhelst wrote:
On Tue, Mar 17, 2020 at 09:10:32PM +0300, Vladimir Sementsov-Ogievskiy wrote:
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..
RESIZE would need a length. It would probably not need an offset, that
is true.
A REOPEN command could use both if we wanted to support a partial
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?
The benefit is that you can read the whole command in one go, without
having to process the payload length and do a second read.
Reasonable
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..
No, we can just ignore the values there.
So, offset will always be 0 for RESIZE?
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).
That still requires the second read.
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" ?
No, I just mean that if we do end up defining a command that doesn't
require either of those two fields, we can just add to the spec that
"the length and offset fields are reserved, and should be set to zero
for this command".
OK, understand. Reasonable thing, agreed. I'll resend.