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@xxxxxxxxxx> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> >> --- >> +++ 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