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