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