[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 04/11] block: remove 'x' prefix from experime
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v6 04/11] block: remove 'x' prefix from experimental bitmap APIs |
Date: |
Tue, 8 Jan 2019 10:55:29 +0000 |
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 <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...
>
Reasonable, I've no objections.
--
Best regards,
Vladimir