Re: Qemu block filter insertion/removal API

[Max Reitz <mreitz@redhat.com>]

On 17.05.21 14:44, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> I'd like to be sure that we know where we are going to.
> 
> In blockdev-era where qemu user is aware about block nodes, all nodes 
> have good names and controlled by user we can efficiently use block 
> filters.
> 
> We already have some useful filters: copy-on-read, throttling, compress. 
> In my parallel series I make backup-top filter public and useful without 
> backup block jobs. But now filters could be inserted only together with 
> opening their child. We can specify filters in qemu cmdline, or filter 
> can take place in the block node chain created by blockdev-add.
> 
> Still, it would be good to insert/remove filters on demand.
> 
> Currently we are going to use x-blockdev-reopen for this. Still it can't 
> be used to insert a filter above root node (as x-blockdev-reopen can 
> change only block node options and their children). In my series "[PATCH 
> 00/21] block: publish backup-top filter" I propose (as Kevin suggested) 
> to modify qom-set, so that it can set drive option of running device. 
> That's not difficult, but it means that we have different scenario of 
> inserting/removing filters:
> 
> 1. filter above root node X:
> 
> inserting:
> 
>    - do blockdev-add to add a filter (and specify X as its child)
>    - do qom-set to set new filter as a rood node instead of X
> 
> removing
> 
>    - do qom-set to make X a root node again
>    - do blockdev-del to drop a filter
> 
> 2. filter between two block nodes P and X. (For example, X is a backing 
> child of P)
> 
> inserting
> 
>    - do blockdev-add to add a filter (and specify X as its child)
>    - do blockdev-reopen to set P.backing = filter
> 
> remvoing
> 
>    - do blockdev-reopen to set P.backing = X
>    - do blockdev-del to drop a filter
> 
> 
> And, probably we'll want transaction support for all these things.
> 
> 
> Is it OK? Or do we need some kind of additional blockdev-replace 
> command, that can replace one node by another, so in both cases we will do
> 
> inserting:
> 
>    - blockdev-add filter
>    - blockdev-replace (make all parents of X to point to the new filter 
> instead (except for the filter itself of course)
> 
> removing
>    - blockdev-replace (make all parante of filter to be parents of X 
> instead)
>    - blockdev-del filter
> 
> 
> It's simple to implement, and it seems for me that it is simpler to use. 
> Any thoughts?

I’m afraid as a non-user of the blockdev interface, I can’t give a 
valuable opinion that would have some actual weight.

Doesn’t stop me from giving my personal and potentially invaluable 
opinion, though, obviously:

I think we expect all users to know the block graph, so they should be 
able to distinguish between cases 1 and 2.  However, I can imagine 
having to distinguish still is kind of a pain, especially if it were 
trivial for qemu to let the user not having to worry about it at all.

Also, if you want a filter unconditionally above some node, all the 
qom-set and blockdev-reopen operations for all of the original node’s 
parents would need to happen atomically.  As you say, those operations 
should perhaps be transactionable anyway, but...  Implementing 
blockdev-replace would provide this for much less cost now, I suppose?

I guess it can be argued that the downside is that having 
blockdev-replace means less pressure to make qom-set for drive and 
blockdev-reopen transactionable.

But well.  I don’t really have anything against a blockdev-replace, but 
again, I don’t know whether my opinion on this topic really has weight.

Max