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

[Denis V. Lunev]

On 04/04/2016 11:45 PM, Eric Blake wrote:
> On 04/04/2016 02:27 PM, Denis V. Lunev wrote:
> > On 04/04/2016 11:15 PM, Alex Bligh wrote:
> > > On 4 Apr 2016, at 21:13, Denis V. Lunev <address@hidden> wrote:
> > >
> > > > As far as I remember that text we have had a number in request
> > > > specifying which bitmap to query and the server should reply with one
> > > > bitmap at a time.
> > > >
> > > > Would this work for you?
> > > I think that would be much better, yes, though I'd suggest the
> > > bitmap had an ID other than a number 0..15 so other people
> > > can use it too.
> > bitmap requires to negotiate granularity which is
> > not that easy thing.
> >
> > If we have different granularities for 'dirty' and 'allocated'
> > bitmaps and we can report this using this proto and
> > can not do this cleanly with bitmap approach assuming
> > that we will add 'NBD_STATE_DIRTY_DEALLOCATED' (0x2) state
> I'm not sure what you are trying to propose adding here.  'state' is a
> bitmap - it is either a representation of two bits of information
> (NBD_CMD_FLAG_DIRTY was clear, so state is the bitwise OR of
> NBD_STATE_HOLE and NBD_STATE_ZERO), or the representation of one bit of
> information (NBD_CMD_FLAG_DIRTY was set, so state is the bitwise OR of
> NBD_STATE_CLEAN).
sorry for vague description. I have messed up.

In v1 we have had 'status' field, which can have the
following values for dirty request:

+      - `NBD_STATE_DIRTY` (0x0), LBA extent is dirty;
+      - `NBD_STATE_CLEAN` (0x1), LBA extent is clean.

in the extent structure:

+        * 64 bits, offset (unsigned)
+        * 32 bits, length (unsigned)
+        * 16 bits, status (unsigned)

with an additional NBD_STATE_DIRTY_HOLE or (DIRTY_DEALLOCATED)
we could report the entire state using one query. The user could be
able to read entire state which is useful for backup at once.

Your current proposal is more tricky and it was misunderstood by Alex:

+        * 32 bits, status flags

and you describe flags as

+      - `NBD_STATE_HOLE` (bit 0); if set, the block represents a hole
+        (and future writes to that area may cause fragmentation or
+        encounter an `ENOSPC` error); if clear, the block is allocated
+        or the server could not otherwise determine its status.  Note
+        that the use of `NBD_CMD_TRIM` is related to this status, but
+        that the server MAY report a hole even where trim has not been
+        requested, and also that a server MAY report allocation even
+        where a trim has been requested.
+      - `NBD_STATE_ZERO` (bit 1), if set, the block contents read as
+        all zeroes; if clear, the block contents are not known.  Note
+        that the use of `NBD_CMD_WRITE_ZEROES` is related to this
+        status, but that the server MAY report zeroes even where write
+        zeroes has not been requested, and also that a server MAY
+        report unknown content even where write zeroes has been
+        requested.

+      - `NBD_STATE_CLEAN` (bit 2); if set, the block represents a
+        portion of the file that is still clean because it has not
+        been written; if clear, the block represents a portion of the
+        file that is dirty, or where the server could not otherwise
+        determine its status.

and opinion of Alex was that all 3 bits could be set in reply to 
NBD_CMD_BLOCK_STATUS
with NBD_CMD_FLAG_STATUS_DIRTY set.

This confused him. This confuses me too.

If allocated state is not replied to command with NBD_CMD_FLAG_STATUS_DIRTY
then why to have different meaning of bits.


> I'm not sure where you are reading into this that granularity has to be
> negotiated.  The client never mentions granularity; and the server is
> perfectly free to report data in descriptors as large or as small as it
> wants (although I did document that the server SHOULD stick to
> descriptors that are at least 512 bytes at a time, and SHOULD coalese
> descriptors so that two consecutive descriptors have distinct state values).
>
> Whether something is allocated or not has no direct bearing on whether
> it is dirty or not; and it is feasible that a server could report the
> act of NBD_CMD_TRIM as causing a portion of the file to become dirty.