On 10/26/20 5:50 AM, Peter Krempa wrote:
> On Fri, Oct 23, 2020 at 13:36:46 -0500, Eric Blake wrote:
>> Since 'nbd-server-add' is deprecated, and 'block-export-add' is new to
>> 5.2, we can still tweak the interface. Allowing 'bitmaps':['str'] is
>> nicer than 'bitmap':'str'. This wires up the qapi and qemu-nbd
>> changes to permit passing multiple bitmaps as distinct metadata
>> contexts that the NBD client may request, but the actual support for
>> more than one will require a further patch to the server.
>>
>> Signed-off-by: Eric Blake
>> Reviewed-by: Vladimir Sementsov-Ogievskiy
>> ---
>> +++ b/qapi/block-export.json
>> @@ -74,10 +74,10 @@
>> # @description: Free-form description of the export, up to 4096 bytes.
>> # (Since 5.0)
>> #
>> -# @bitmap: Also export the dirty bitmap reachable from @device, so the
>> -# NBD client can use NBD_OPT_SET_META_CONTEXT with the
>> -# metadata context name "qemu:dirty-bitmap:NAME" to inspect the
>> -# bitmap. (since 4.0)
>> +# @bitmaps: Also export each of the named dirty bitmaps reachable from
>> +# @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT with
>> +# the metadata context name "qemu:dirty-bitmap:BITMAP" to inspect
>> +# each bitmap. (since 5.2)
>
> Given unsynchronised release cycles between qemu and management apps
> it's not cool to deprecate an interface without having at least one
> release where the replacement interface is already stable.
>
That's why I'm trying as hard as possible to get the block-export-add
interface perfect in its 5.2 release; if we agree that allowing qemu to
expose more than one bitmap is beneficial (and I argue that it is), then
the new interface MUST support that from the get-go, and not something
where we release it with 5.2 having 'bitmap' and 6.0 adding 'bitmaps'.
> This means that any project wanting to stay up to date will either have
> to use deprecated interfaces for at least one release and develop the
> replacement only when there's a stable interface or hope that they don't
> have to change the interfaces too often.
>
> This specifically impacts libvirt as we have validators which notify us
> that a deprecated interface is used and we want to stay in sync, so that
> there's one less group of users to worry about at the point qemu will
> want to delete the interface.
The deprecated interface is nbd-server-add; for _that_ interface, the
'bitmap' parameter will continue to work until nbd-server-add is removed
in 6.1 (so you have all of 5.2 and 6.0 to switch from nbd-server-add to
block-export-add).
What this patch does, then, is alter the deprecation from merely
changing the command from nbd-server-add to block-export-add with all
parameter names remaining the same, to instead changing both the command
name and the 'bitmap'=>'bitmaps' parameter. But I agree that libvirt
wants to do an all-or-none conversion: there is no reason for libvirt to
use block-export-add until 5.2 is actually released, at which point we
have locked in the new interface; and this patch is a demonstration that
we are still debating about a tweak to that interface before it becomes
locked in.
>
>> # @allocation-depth: Also export the allocation depth map for @device, so
>> #the NBD client can use NBD_OPT_SET_META_CONTEXT with
>> @@ -88,7 +88,7 @@
>> ##
>> { 'struct': 'BlockExportOptionsNbd',
>>'data': { '*name': 'str', '*description': 'str',
>> -'*bitmap': 'str', '*allocation-depth': 'bool' } }
>> +'*bitmaps': ['str'], '*allocation-depth': 'bool' } }
>
> This adds 'bitmaps' also to nbd-server-add, which should not happen. You
> probably want to stop using 'base' for 'NbdServerAddOptions' and just
> duplicate everything else.
I can respin this to NOT add 'bitmaps' to the legacy 'nbd-server-add',
if you think that would be better. It is more complex in the QAPI code,
but not too much more difficulty in the glue code; and the glue code all
goes away in 6.1 when the deprecation cycle ends.
>
>> # @NbdServerAddOptions:
>> @@ -100,12 +100,18 @@
>> # @writable: Whether clients should be able to write to the device via the
>> #NBD connection (default false).
>> #
>> +# @bitmap: Also export a single dirty bitmap reachable from @device, so the
>> +# NBD client can use NBD_OPT_SET_META_CONTEXT with the metadata
>> +# context name "qemu:dirty-bitmap:BITMAP" to inspect the bitmap
>> +# (since 4.0). Mutually exclusive with @bitmaps, and newer
>> +# clients should use that instead.
>
> This doesn't make sense, nbd-server-add never had @bitmaps. Also adding
> features to a deprecated interface doesn't IMO make sense if you want to
> motivate users switch to thne new one.
Fair enough. v6 coming up later today.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
