Re: [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

29.08.2019 17:44, Peter Krempa wrote:
> On Wed, Aug 28, 2019 at 13:48:10 -0400, John Snow wrote:
>> (Peter: search for "pkrempa" down below.)
>>
>> On 8/28/19 5:20 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
> [....]
> 
> 
>> So that's a bit of a change, but only visually. The "reality" is still
>> the same, we just report it more "accurately." libvirt MIGHT need a
>> heads up here. I'm looping pkrempa back in for comment.
>>
>> <pkrempa>
>> Would libvirt be negatively impacted by the revelation of formerly
>> internal ("implicit") nodes created by mirror and commit via query block
>> commands? At the moment, QEMU hides them from you if you do not name them.
> 
> Currently we would not be able to handle that properly at least
> definitely in the pre-blockdev case. In blockdev case I must make sure
> that it will work.
> 
> The thing is that I didn't really want to touch the pre-blockdev case
> code any more,

Aren't you going to deprecate and drop it at some moment?

  but if you decide that we should do it I'm willing to
> investigate this case also for the old commands.
> 
>> </pkrempa>
>>
>>> 3. bdrv_refresh_filename, bdrv_reopen_parse_backing, bdrv_drop_intermediate:
>>>     I think it's not a problem, just drop special case for implicit fitlers
>>>
>>
>> I'm much less certain about what the impact of this would be and would
>> need to audit it (and don't have the time to, personally.)
>>
>> Do you have a POC or RFC patch that demonstrates dropping these special
>> cases? It might be nice to see as proof that it's safe to deprecate.
>>
>>> So, seems the only real change is query-block and query-blockstats output when mirror or commit is started
>>> without specifying filter-node-name (filter would be on top)
>>>
>>> So, how should we deprecate this, or can we just change it?
>>>
>>
>> I'm not sure if it's worth it yet, what does dropping the implicit field
>> buy us? Conceptually I understand that it's simpler without the notion
>> of implicit fields, but I imagine there's some cleanup in particular
>> that motivated this.
>>
>> I'd say to just change the behavior, we should:
>>
>> - Give a standard three-release warning that the behavior will change in
>> an incompatible way
>> - Demonstrate with an RFC patch that special cases around ->implicit in
>> block.c can be removed and do not make the code more complex,
>> - Get blessings from Peter Krempa.
>>
>> As always: Libvirt is not the end-all be-all of QEMU management, but if
>> libvirt is capable of working around design changes then I believe any
>> project out there today also could, so it's a good litmus test.
> 
> For libvirt we really care more whether a node is format/protocol
> related or not rather than whether it's implicit or not.
> 
> In this case we could filter it by the known protocol and format driver
> types and filter out the rest in cases when we e.g. detect the node
> names for the pre-blockdev era cases.
> 
> (Note that even with new qemu, if an SD card is used blockdev will be
> disabled).
> 


-- 
Best regards,
Vladimir