Re: [Qemu-block] [Qemu-devel] [PATCH] qapi: add dirty-bitmaps to query-named-block-nodes result

[John Snow]

On 5/31/19 10:55 AM, Eric Blake wrote:
> On 5/30/19 11:26 AM, John Snow wrote:
>>
>>
>> On 5/30/19 10:39 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Let's add a possibility to query dirty-bitmaps not only on root nodes.
>>> It is useful when dealing both with snapshots and incremental backups.
>>>
> 
>>> +++ b/block/qapi.c
>>> @@ -78,6 +78,11 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend 
>>> *blk,
>>>          info->backing_file = g_strdup(bs->backing_file);
>>>      }
>>>  
>>> +    if (!QLIST_EMPTY(&bs->dirty_bitmaps)) {
>>> +        info->has_dirty_bitmaps = true;
>>> +        info->dirty_bitmaps = bdrv_query_dirty_bitmaps(bs);
>>> +    }
>>> +
>>>      info->detect_zeroes = bs->detect_zeroes;
>>>  
>>>      if (blk && blk_get_public(blk)->throttle_group_member.throttle_state) {
>>>
>>
>> So query-block uses bdrv_query_info, which calls bdrv_block_device_info,
>> so we'll duplicate the bitmap output when doing the old-fashioned block
>> query, but that's probably harmless overall.
> 
> We already know that none of our existing query- interfaces are sane
> (either too little information, or too much).  Duplication starts to
> push an interface towards too much (it takes processor time to bundle up
> the extra JSON, especially if the other end is not going to care if it
> was present). I know Kevin still has somewhere on his to-do list the
> implementation of a saner query- command for the information we really
> want (about each block, without redundant information, and where we
> don't repeat information in a nested manner, but where we also don't
> omit information that would otherwise require multiple existing query-
> to reconstruct).
> 
>>
>> We can continue to support the output in both places, or we could opt to
>> deprecate the older interface; I think this is one of the last chances
>> we'd get to do so before libvirt and wider adoption.
>>
>> I think that's probably Eric's choice.
> 
> If you want to try to deprecate the old location, introspection at least
> works to allow libvirt to know which place to look for it on a given
> qemu. If you don't think deprecation is necessary, the duplication is
> probably tolerable for now (as ideally we'd be deprecating ALL of our
> not-quite-perfect query- block interfaces in favor of whatever sane
> interface Kevin comes up with).
> 

It sounds like it's probably the right move to deprecate the entire
legacy interface, but still... If you have 20 or 30 bitmaps on a root
node, you will see 40 or 60 entries.

What's the smart way to deprecate it? We're not adding new flags or
showing new arguments or anything. There might not be bitmaps, so you
can't rely on that field being present or absent.

Recommendations?