On Wed, Aug 02, 2017 at 01:34:46PM +0300, Manos Pitsidianakis wrote:
> On Wed, Aug 02, 2017 at 11:07:24AM +0100, Stefan Hajnoczi wrote:
> > On Tue, Aug 01, 2017 at 04:49:07PM +0300, Manos Pitsidianakis wrote:
> > > @@ -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?
> >
>
> If I haven't misunderstood this, BdrvChild holds only the child part of the
> parent-child relationship and there's no way to access a parent from
> bs->parents. bdrv_replace_node() will thus only replace the child part in
> BdrvChild from the aspect of the parent. In the old child bs's perspective,
> one of the nodes of bs->parents is removed and in the new child bs's
> perspective a new node in bs->parents was inserted. parent_bs thus remains
> immutable.
>
> child->parent_bs is needed in this patch because in jobs if a job-ID is not
> specified the parent name is used, but this fails if the parent is an
> implicit node instead of BlockBackend and causes a regression (certain job
> setups suddenly need an explicit job ID instead of just working).Please see Kevin's reply to my email. > > > - 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. > > I can include a patch to move throttle_node to BlockBackend and remove all > BlockBackendPublic code, is that okay? That would be a nice cleanup, thanks!
signature.asc
Description: PGP signature
