Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension

[Wouter Verhelst]

Hi,

Need to look into this in some detail, for which I don't have the time
(or the non-tiredness ;-) right now, but these two caught my eye:

On Mon, Apr 04, 2016 at 10:39:10AM -0600, Eric Blake wrote:
[...]
> +* `NBD_REPLY_TYPE_BLOCK_STATUS`
> +
> +    *length* MUST be a positive integer multiple of 8.  This reply
> +    represents a series of consecutive block descriptors where the sum
> +    of the lengths of the descriptors equals the length of the
> +    original request.  This chunk type MUST appear at most once in a
> +    structured reply. Valid as a reply to `NBD_CMD_BLOCK_STATUS.
> +
> +    The payload is structured as a list of one or more descriptors,
> +    each with this layout:
> +
> +        * 32 bits, length (unsigned, MUST NOT be zero)
> +        * 32 bits, status flags
> +
> +    The definition of the status flags is determined based on the
> +    flags present in the original request.

Might be a good idea to specify what a client can do with flags it
doesn't know about; ignore them, probably?
 
[...]
> +The extension adds the following new command flag:
> +
> +- `NBD_CMD_FLAG_STATUS_DIRTY`; valid during `NBD_CMD_BLOCK_STATUS`.
> +  SHOULD be set to 1 if the client wants to request dirtiness status
> +  rather than provisioning status.

Why only one flag here? I could imagine a client might want to query for
both states at the same time. Obviously that means a client needs to
query for *at least* one of the statuses, otherwise the server should
reply with EINVAL.

Though I'm undecided on whether a bit being set to 0 should mean "give
me that information" or whether 1 should.

-- 
< 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