On Tue, Aug 01, 2017 at 04:49:07PM +0300, Manos Pitsidianakis wrote: > diff --git a/block.c b/block.c > index 9ebdba28b0..c6aad25286 100644 > --- a/block.c > +++ b/block.c > @@ -1975,6 +1975,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState > *child_bs, > child = g_new(BdrvChild, 1); > *child = (BdrvChild) { > .bs = NULL, > + .parent_bs = NULL, > .name = g_strdup(child_name), > .role = child_role, > .perm = perm, > @@ -2009,6 +2010,7 @@ BdrvChild *bdrv_attach_child(BlockDriverState > *parent_bs, > if (child == NULL) { > return NULL; > } > + child->parent_bs = parent_bs; > > QLIST_INSERT_HEAD(&parent_bs->children, child, next); > return child; > @@ -3729,6 +3731,12 @@ const char *bdrv_get_parent_name(const > BlockDriverState *bs) > return name; > } > } > + if (c->parent_bs && c->parent_bs->implicit) { > + name = bdrv_get_parent_name(c->parent_bs); > + if (name && *name) { > + return name; > + } > + } > } > > return NULL;
This should be a separate patch. Who updates parent_bs if the parent is changed (e.g. bdrv_replace_node())? We already have bs->parents. Why is BdrvChild->parent_bs needed? > -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)); Is it safe to drop drained_begin? We must ensure that no I/O requests run during this function. > - throttle_group_unregister_tgm(&blk->public.throttle_group_member); > - bdrv_drained_end(blk_bs(blk)); > + BlockDriverState *bs, *throttle_node; > + > + throttle_node = blk_get_public(blk)->throttle_node; Is blk_get_public() still necessary? Perhaps we can do away with the concept of the public struct now. It doesn't need to be done in this patch though. > + > + assert(throttle_node && throttle_node->refcnt == 1); Are you sure the throttle_node->refcnt == 1 assertion holds? For example, does the built-in NBD server have a reference to the throttle node if nbd-server-add is called after throttling has been enabled? Since we have the blk->throttle_node pointer we know we're the owner. Others may be using the node too but we may choose to remove it at any time.
signature.asc
Description: PGP signature