[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: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension |
Date: |
Mon, 4 Apr 2016 14:34:26 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 |
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Eric Blake, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Alex Bligh, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Eric Blake, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Denis V. Lunev, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Alex Bligh, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Denis V. Lunev, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension,
Eric Blake <=
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Denis V. Lunev, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Alex Bligh, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Paolo Bonzini, 2016/04/05
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Alex Bligh, 2016/04/05
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Paolo Bonzini, 2016/04/05
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Alex Bligh, 2016/04/05
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Paolo Bonzini, 2016/04/05
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Wouter Verhelst, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Eric Blake, 2016/04/04
- Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Add NBD_CMD_BLOCK_STATUS extension, Wouter Verhelst, 2016/04/05