Am 02.03.2026 um 13:54 hat Fiona Ebner geschrieben: > Am 26.02.26 um 5:31 PM schrieb Kevin Wolf: > > Am 26.02.2026 um 16:20 hat Fiona Ebner geschrieben: > >> Am 24.02.26 um 3:34 PM schrieb Kevin Wolf: > >>> Am 16.01.2026 um 15:39 hat Fiona Ebner geschrieben: > >>>> With '-drive', it is possible to specify the throttle configuration > >>>> directly on the commandline. Add the possibility to do the same when > >>>> using the modern way with '-blockdev' and a front-end device. Using a > >>>> throttle filter block node is not always an option: in particular, the > >>>> mirror block job operates on a root block node and it might be desired > >>>> to throttle only the guest IO, but not to the block job. > >>> > >>> Hm, is there still a reason why we require a root node for the source? > > Do you happen to know what the original reason for the restriction was? > From git history I cannot really see anything special, just:
No, I don't remember. Maybe some mailing list archaeology could help, but I'm not planning to do that right now. > > commit 07eec652722f3d12b07c5b28d0671ddfc22fe6a5 > > Author: Kevin Wolf <[email protected]> > > Date: Thu Jun 23 14:20:24 2016 +0200 > > > > block: Accept node-name for blockdev-mirror > > > > In order to remove the necessity to use BlockBackend names in the > > external API, we want to allow node-names everywhere. This converts > > blockdev-mirror to accept a node-name without lifting the restriction > > that we're operating at a root node. > > For operations like backup and stream, the same restriction was lifted: > 930fe17f99 ("blockdev: enable non-root nodes for backup source") > 554b614765 ("block: Add QMP support for streaming to an intermediate layer") > > We have the following code snippet in blockdev_mirror_common(): > > > if (!replaces) { > > /* We want to mirror from @bs, but keep implicit filters on top */ > > unfiltered_bs = bdrv_skip_implicit_filters(bs); > > if (unfiltered_bs != bs) { > > replaces = unfiltered_bs->node_name; > > } > > } > > Doing this as well for non-root nodes would still match the documentation: > > > # @replaces: with sync=full graph node name to be replaced by the new > > # image when a whole image copy is done. This can be used to > > # repair broken Quorum files. By default, @device is replaced, > > # although implicitly created filters on it are kept. Yes, I think this would be the expected behaviour. > But I guess we could also do something like > > > diff --git a/blockdev.c b/blockdev.c > > index 6e6932ddea..0ff058bed0 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -2931,7 +2931,14 @@ static void blockdev_mirror_common(const char > > *job_id, BlockDriverState *bs, > > /* We want to mirror from @bs, but keep implicit filters on top */ > > unfiltered_bs = bdrv_skip_implicit_filters(bs); > > if (unfiltered_bs != bs) { > > - replaces = unfiltered_bs->node_name; > > + if (bdrv_is_root_node(bs)) { > > + replaces = unfiltered_bs->node_name; > > + } else { > > + error_setg(errp, "Detected non-root block node which is an" > > + " implicit filter, please specify > > @replaces" > > + " parameter explicitly"); > > + return; > > + } > > } > > } > > > > to avoid any surprises for people who explicitly specify a non-root > implicit filter as the source? Or what do you think? What would the surprise be? That the filters are kept instead of being replaced? But wouldn't it be inconsistent then that the same setup works if it's a root node? I suppose what would have been consistent is if BlockBackend names kept implicit filters, but node names didn't. It's too late for that, though. > Reading through the code, I don't see any other issues with using a > non-root node as the source and an initial test case for a node below a > throttle node seems to work fine too :) That sounds promising. Given that the 11.0 freeze is on Tuesday, do we still want to move forward with a v2 of this patch, or do you think we should rather focus on making throttle nodes usable in all of the cases we care about, even if that means delaying it to 11.1? Kevin
