Eric,
> close, but needs a v4
Should have guessed :-)
>> +++ b/doc/proto.md
>> @@ -217,6 +217,33 @@ handle as was sent by the client in the corresponding request. In
>> this way, the client can correlate which request is receiving a
>> response.
>>
>> +#### Ordering of messages and writes
>> +
>> +The server MAY process commands out of order, and MAY reply out of
>> +order, save that:
>
> maybe s/save/except/
ok
>> +
>> +* All write commands (that includes both `NBD_CMD_WRITE` and
>> + `NBD_CMD_TRIM`) that the server completes (i.e. replies to)
>
> should we also list NBD_CMD_WRITE_ZEROES?
My mistake
>
>> + prior to processing to a `NBD_CMD_FLUSH` MUST be written to non-volatile
>> + storage prior to replying to that `NBD_CMD_FLUSH`. This
>> + paragraph only applies if `NBD_FLAG_SEND_FLUSH` is set within
>> + the transmission flags, as otherwise `NBD_CMD_FLUSH` will never
>> + be sent by the client to the server.
>> +
>> +* A server MUST NOT reply to a command that has `NBD_CMD_FLAG_FUA` set
>> + in its command flags until the data (if any) written by that command
>> + is persisted to non-volatile storage. This only applies if
>> + `NBD_FLAG_SEND_FLUSH` is set within the transmission flags, as otherwise
>
> s/FLUSH/FUA/
Yes
>> + `NBD_CMD_FLAG_FUA` will not be set on any commands sent to the server
>> + by the client.
>> +
>> +`NBD_CMD_FLUSH` is modelled on the Linux kernel empty bio with
>> +`REQ_FLUSH` set. `NBD_CMD_FLAG_FUA` is modelled on the Linux
>> +kernel bio with `REQ_FUA` set. In case of ambiguity in this
>> +specification, the
>> +[kernel documentation](https://www.kernel.org/doc/Documentation/block/writeback_cache_control.txt)
>> +may be useful.
>> +
>> #### Request message
>>
>> The request message, sent by the client, looks as follows:
>> @@ -483,10 +510,20 @@ affects a particular command. Clients MUST NOT set a command flag bit
>> that is not documented for the particular command; and whether a flag is
>> valid may depend on negotiation during the handshake phase.
>>
>> -- bit 0, `NBD_CMD_FLAG_FUA`; valid during `NBD_CMD_WRITE` and
>> - `NBD_CMD_WRITE_ZEROES` commands. SHOULD be set to 1 if the client requires
>> - "Force Unit Access" mode of operation. MUST NOT be set unless transmission
>> - flags included `NBD_FLAG_SEND_FUA`.
>> +- bit 0, `NBD_CMD_FLAG_FUA`; This flag is valid for all commands provided
>
> s/commands/commands,/
Yes
>
>> + `NBD_FLAG_SEND_FUA` has been negotiated, in which case the server MUST
>> + accept all commands with this bit set (even by ignoring the bit). The
>> + client SHOULD NOT set this bit unless the command has the potential of
>> + writing data (current commands are `NBD_CMD_WRITE`, `NBD_CMD_WRITE_ZEROES`
>> + and `NBD_CMD_TRIM`); existing clients are known to set this bit on
>
> maybe: s/existing/but existing/
>
>> + other commands; subject to that, provided `NBD_FLAG_SEND_FUA` is
>> + negotiated, the client MAY set this bit as it wishes. If the server
>
> Sounds redundant. I'd strike 'subject to that, ... as it wishes'.
I was trying to say that in a stream of commands it can set it as
it wishes. I've made it clearer and referred to the section on
ordering.
>
>> + receives a command with `NBD_CMD_FLAG_FUA` set it MUST NOT send its
>> + reply to that command until all write operations (if any) associated with
>> + that command command have been completed and persisted to non-volatile
>> + storage. If the command does not in fact write data (for instance on an
>> + `NBD_CMD_TRIM` which does is ignored), the server MAY ignore this bit
>
> s/does //
Reworded differently.
>> + being set on such a command.
>> - bit 1, `NBD_CMD_NO_HOLE`; defined by the experimental `WRITE_ZEROES`
>> extension; see below.
>> - bit 2, `NBD_CMD_FLAG_DF`; defined by the experimental `STRUCTURED_REPLY`
>> @@ -535,12 +572,6 @@ The following request types exist:
>> message. The server MAY send the reply message before the data has
>> reached permanent storage.
>>
>> - If the `NBD_FLAG_SEND_FUA` flag ("Force Unit Access") was set in the
>> - transmission flags field, the client MAY set the flag `NBD_CMD_FLAG_FUA` in
>> - the command flags field. If this flag was set, the server MUST NOT send
>> - the reply until it has ensured that the newly-written data has reached
>> - permanent storage.
>> -
>
> You should drop this paragraph from both NBD_CMD_WRITE and
> NBD_CMD_WRITE_ZEROES, now that the flag is globally described (here, you
> only dropped it from one of the two).
Thanks, missed that.
--
Alex Bligh
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail