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 <eblake(a)redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov(a)virtuozzo.com>
> ---
> +++ 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