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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

04.01.2019 2:21, Eric Blake wrote:
> 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 <jsnow@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   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...
> 

Reasonable, I've no objections.


-- 
Best regards,
Vladimir