Re: [PATCH] qapi/block: fix nbd-server-add spec

[Eric Blake <eblake@redhat.com>]

On 12/19/19 9:08 AM, Nir Soffer wrote:

>>>> Let's just fix qapi spec now.
>>>
>>> But qapi documents a better behavior for users. We should fix the code instead
>>> to mach the docs.
>>
>> 1. Using disk name as a bitmap name is a bad behavior, as they are completely
>> different concepts. Especially keeping in mind that user already knows disk name anyway
>> and no reason to write this export name inside metadata context of this export.
> 
> The different concept is expressed by the "qemu:dirty-bitmap:" prefix.
> "qemu:dirty-bitmap:export-name" means the dirty bitmap for this export.
> 
>> 2. It's not directly documented. You assume that NAME == @name. I understand that
>> it may be assumed.. But it's not documented.
> 
> But NAME is likely to be understood as the name argument, and unlikely to be the
> bitmap name.

That's a misunderstanding due to poor docs.  The bitmap name has always 
been what was exposed, ever since we promoted things to stable by 
getting rid of x-.

> 
>> 3. It's never worked like you write. So if we change the behavior, we'll break
>> existing users.
> 
> Do we have existing users? isn't this new feature in 4.2?

No, the feature stems back to 4.0, when we got rid of x-.  There are 
other reasons that dirty bitmaps aren't really usable for incremental 
backups without qemu 4.2, but qemu 4.0 was the first time we exposed a 
stable interface for a bitmap over an NBD export, and that release used 
the bitmap name (and not the export name), so at this point, a code 
change would break expectations of any 4.0 client using bitmaps for 
other reasons.  Libvirt currently has absolute control over the bitmap 
name (my initial code in libvirt created a temporary bitmap with my 
desired name, then merged the contents from the permanent bitmaps 
corresponding to the actual libvirt Checkpoint objects into the 
temporary, so that it could then call nbd-export-add with the temporary 
bitmap name).  But, as you point out...

> 
> Before we had experimental x-block-dirty-bitmap APIs, which are stable, so users
> could not depend on them.

The unstable x-block-dirty-bitmap APIs _did_ have a way to export a 
user-controlled name SEPARATE from the bitmap name.  At the time I was 
removing the x- prefix, I asked if anyone had a strong use case for 
keeping that functionality.  No one spoke up in favor of keeping it 
(Nikolay mentioned using the old interface, but not being stumped by its 
removal), so we nuked it at the time.  We can always add it back (now 
that it sounds like you have a use case where it may be more 
compelling), but it was easier to stabilize less and add more later as 
needed, than to stabilize too much and regret that we had to support the 
flexibility that no one needed.
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg02373.html
https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg01970.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org