On Fri, Jun 23, 2017 at 03:46:59PM +0300, Manos Pitsidianakis wrote:
-void blk_io_limits_disable(BlockBackend *blk)
+void blk_io_limits_disable(BlockBackend *blk, Error **errp)
{
- assert(blk->public.throttle_group_member.throttle_state);
- bdrv_drained_begin(blk_bs(blk));
- throttle_group_unregister_tgm(&blk->public.throttle_group_member);
- bdrv_drained_end(blk_bs(blk));
+ Error *local_err = NULL;
+ BlockDriverState *bs, *throttle_node;
+
+ throttle_node = blk_get_public(blk)->throttle_node;
+
+ assert(throttle_node && throttle_node->refcnt == 1);
I'm not sure if we can enforce refcnt == 1. What stops other graph
manipulation operations from inserting a node above or a BB that uses
throttle_node as the root?
The throttle node affects only the backing file now, but not the new
top-level node. With manually created filter nodes we may need to
provide a way so that the QMP command tells where in the tree the
snapshot is actually taken. With automatically created ones, we need to
make sure that they always stay on top.
Similar problems arise when block jobs manipulate the graph. For
example, think of a commit block job, which will remove the topmost node
from the backing file chain. We don't want it to remove the throttling
filter together with the qcow2 node. (Or do we? It might be something
that needs to be configurable.)
We also have several QMP that currently default to working on the
topmost image of the chain. Some of them may actually want to work on
the topmost node that isn't an automatically inserted filter.
I hope this helps you understand why I keep saying that automatic
insertion/removal of filter nodes is tricky and we should do it as
little as we possibly can.