Re: [Qemu-devel] [PATCH v6 04/11] block: remove 'x' prefix from experimental bitmap APIs

[Eric Blake]

On 12/21/18 3:35 AM, John Snow wrote:
> The 'x' prefix was added because I was uncertain of the direction we'd
> take for the libvirt API. With the general approach solidified, I feel
> comfortable committing to this API for 4.0.
> 
> Signed-off-by: John Snow <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  blockdev.c             | 22 +++++++++++-----------
>  qapi/block-core.json   | 34 +++++++++++++++++-----------------
>  qapi/transaction.json  | 12 ++++++------
>  tests/qemu-iotests/223 |  4 ++--
>  4 files changed, 36 insertions(+), 36 deletions(-)

Misses the conversion of x-nbd-server-add-bitmap in qapi/block.json
(could be a separate patch), but that's also one of the commands that
libvirt will need to use alongside all the bitmap changes.

Speaking of x-nbd-server-add-bitmap, does it actually have to be a
separate command, or should it just be an additional parameter to the
existing nbd-server-add?  There are two directions this could go:

As a single command, it enforces that there is no window of time where
the export is available without the dirty bitmap. However, it also means
we can only export at most one bitmap, unless we make the parameter take
an array of bitmaps to export.  Fewer commands also means less roll-back
scenarios in libvirt (no need to worry about the export being added but
the bitmap failing).

As a separate command, we could in the future allow the server to export
multiple bitmaps at once by calling the add-bitmap command more than
once; but right now, the code base does not allow multiple bitmap
exports (a second bitmap export attempt fails).

I'm leaning towards the former (drop x-nbd-server-add-bitmap altogether,
and instead add a bitmap parameter to nbd-server-add).  Also, I'm
leaning towards locking in that we only ever export one
qemu:dirty-bitmap:FOO bitmap per export, and thus not worry about an
array.  Besides, if we make the command take an array now but enforce
that the array has at most one element, then later relax it to take more
than one element, there is no way to introspect that change; but if we
make the command take a single string now, and later have a strong
reason why exporting more than one bitmap at once makes sense, we could
still maintain backcompat by using a QAPI alternate type between a
string and an array, which would be introspectible.

I guess I should propose a patch for that, then...

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature