qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Denis V. Lunev
Subject: Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension
Date: Tue, 5 Apr 2016 00:06:06 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1

On 04/04/2016 11:34 PM, Eric Blake wrote:
On 04/04/2016 02:08 PM, Denis V. Lunev wrote:
This again makes me think this should be a different
command from something which is obviously useful and
comprehensible to more than one server/client (i.e.
allocation).

original design of this command has used 16 number
to specify the NUMBER of the bitmap which was
exported by the server.
The original design abused the 16-bit 'flags' field of each command to
instead try and treat that value as a bitmap number, instead of a
bitwise-or'd set of flags.  That was one of the complaints against v1,
and was fixed in v2 by having a single boolean flag, NBD_CMD_FLAG_DIRTY,
which was off for (default) allocation queries, and set for dirtiness
queries.  We can add other flags for any other type of queries, and the
principle of each query being a run-length-encoded listing still applies.

We have reserved number 0 for 'used' bitmap, i.e.
bitmap of allocated blocks and number 1 for 'dirty'
bitmap. Though we can skip specification of the
belonging of any number except '0' and put them
to server-client negotiations. Or we could reserve
'1' for dirtiness state as server-client agrees and
allow other applications to register their own bitmaps
as the deserve to.

Why not to do things this original way?
If you want to encode particular ids, you should do so in a separate
field, and not overload the 'flags' field.

As it is, we don't have structured writes - right now, you can write a
wire sniffer for the client side, where all commands except
NBD_CMD_WRITE are fixed size, and NBD_CMD_WRITE describes its own size
via its length field; the extension NBD_CMD_WRITE_ZEROES still fits into
this scheme.  All NBD implementations have to supply NBD_CMD_WRITE, but
any extension commands do NOT have to be universal.  Writing a wire
sniffer that special-cases NBD_CMD_WRITE is easy (since that command
will always exist), but writing a wire sniffer that special-cases
arbitrary commands, particularly where those arbitrary commands do not
also self-describe the length of the command, is hard.  We can't
overload the flags field to say which bitmap id to grab, but we also
can't arbitrarily add 4 bytes to the command size when the command is
NBD_CMD_BLOCK_STATUS (because wire sniffers that don't know about
NBD_CMD_BLOCK_STATUS wouldn't know to expect those four bytes to be part
of the current packet rather than starting a new packet).

The recent work on structured reads made it possible for an arbitrary
wire sniffer to gracefully skip over the variable-length return size
reply to NBD_CMD_BLOCK_STATUS, and any other extension command that we
might add later.  But right now, I'm not seeing a compelling reason to
add structured commands to the NBD protocol.

thank you for pointing this out. I think I got an idea



reply via email to

[Prev in Thread] Current Thread [Next in Thread]