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

[Markus Armbruster]

John Snow <address@hidden> writes:

> 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?

Kevin's "[PATCH v4 0/6] file-posix: Add dynamic-auto-read-only QAPI
feature" adds "feature flags" to the QAPI schema language, limited to
struct types, because that's what he needs.  They're visible in
introspection.  I intend to complete his work, so we can tack
"deprecated" feature flags to pretty much anything

Could that address your need?