18.05.2021 19:49, Max Reitz wrote:
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.
I discussed it yesterday with my colleagues from Virtuozzo, who will have to be
users of that interface. And they of course prefer one command for all the
cases :)
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.
Thanks, actually my opinion is the same. I think, I'll prepare a patch a day
later if no answers here, and we'll be able to continue discussion on top of
new patch.
Hmm I have one additional (weak, but still) argument for blockdev-replace: it
just seems good to avoid touching extra subsystem in block-graph operations.
For block-jobs we don't need to touch qdev guest block devices, we are good now
with node-names and blockdev-add. So, it's good to save this bit of interface
beauty if we don't have strict reason to drop it.
--
Best regards,
Vladimir