Re: [Nbd] [PATCHv4] docs/proto.md: Clarify SHOULD / MUST / MAY etc

[Wouter Verhelst <w@...112...>]

Hi Alex,

On Wed, Apr 06, 2016 at 10:48:27PM +0100, Alex Bligh wrote:
[...]
> +- bit 0, `NBD_FLAG_HAS_FLAGS`: MUST always be 1
> +- bit 1, `NBD_FLAG_READ_ONLY`: The server MAY set this flag to indicate
> +  to the client that the export is read-only (exports might be read-only
> +  in a manner undetectable to the server, for instance because of
> +  permissions). If this flag is set, the server MUST error subsequent
> +  write operations to the export.
> +- bit 2, `NBD_FLAG_SEND_FLUSH`: the server MAY set this flag to indicate
> +  that it supports the `NBD_CMD_FLUSH` command. The server MUST NOT set this
> +  flag if it does not support the `NBD_CMD_FLUSH` command. The client MUST
> +  NOT send `NBD_CMD_FLUSH` commands if this flag is not set.
> +- bit 3, `NBD_FLAG_SEND_FUA`: the server MAY set this flag to indicate
> +  that it supports the `NBD_CMD_FLAG_FUA` flag. The server MUST NOT set
> +  this flag if it does not support the `NBD_CMD_FLAG_FUA` flag. The client
> +  MUST NOT set the `NBD_CMD_FLAG_FUA` flag if this flag is not set.
> +- bit 4, `NBD_FLAG_ROTATIONAL`: the server MAY set this flag to 1 to
> +  inform the client that the export has the characteristics of a rotational
> +  medium, and the client MAY schedule I/O accesses in a manner corresponding
> +  to the setting of this flag.
> +- bit 5, `NBD_FLAG_SEND_TRIM`: the server MAY set this flag to indicate
> +  that it supports the `NBD_CMD_TRIM` command. The server MUST NOT set this
> +  flag if it does not support the `NBD_CMD_TRIM` command. The client MUST
> +  NOT send `NBD_CMD_FLUSH` commands if this flag is not set.
> +- bit 6, `NBD_FLAG_SEND_WRITE_ZEROES`: defined by the experimental

This is fairly repetitive. What about something like:

---
Many of these flags allow the server to expose to the client which
features it understands. In each case, the server MAY set the flag for
features it understands; the client MUST NOT use a feature unless the
corresponding flag was set.

bit X, `CONSTANT`: expose support for feature X
bit Y, `CONSTANT`: expose support for feature Y
---

which I think will read easier, and also makes exceptions and special
cases stand out more.

[...]
>      handshake and continue the negotiation in the encrypted channel. If
> -    the server is unwilling to perform TLS, it should reply with
> -    `NBD_REP_ERR_POLICY`. For backwards compatibility, a client should
> +    the server is unwilling to perform TLS, it SHOULD reply with
> +    `NBD_REP_ERR_POLICY`. For backwards compatibility, a client MUST
>      also be prepared to handle `NBD_REP_ERR_UNSUP`. If the client sent

Not convinced about MUST here (happens a few times further down the
line).

[...]

No other comments.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12