Re: [Qemu-devel] [PATCH v3 1/5] block: Allow replacement of a BDS by its overlay

[Kevin Wolf <kwolf@redhat.com>]

Am 12.06.2016 um 05:15 hat Fam Zheng geschrieben:
> On Fri, 06/10 20:57, Max Reitz wrote:
> > change_parent_backing_link() asserts that the BDS to be replaced is not
> > used as a backing file. However, we may want to replace a BDS by its
> > overlay in which case that very link should not be redirected.
> > 
> > For instance, when doing a sync=none drive-mirror operation, we may have
> > the following BDS/BB forest before block job completion:
> > 
> >   target
> > 
> >   base <- source <- BlockBackend
> > 
> > During job completion, we want to establish the source BDS as the
> > target's backing node:
> > 
> >           target
> >             |
> >             v
> >   base <- source <- BlockBackend
> > 
> > This makes the target a valid replacement for the source:
> > 
> >           target <- BlockBackend
> >             |
> >             v
> >   base <- source
> > 
> > Without this modification to change_parent_backing_link() we have to
> > inject the target into the graph before the source is its backing node,
> > thus temporarily creating a wrong graph:
> > 
> >   target <- BlockBackend
> > 
> >   base <- source
> > 
> > Signed-off-by: Max Reitz <mreitz@redhat.com>
> > ---
> >  block.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block.c b/block.c
> > index f54bc25..dc76c159 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -2224,9 +2224,23 @@ void bdrv_close_all(void)
> >  static void change_parent_backing_link(BlockDriverState *from,
> >                                         BlockDriverState *to)
> >  {
> > -    BdrvChild *c, *next;
> > +    BdrvChild *c, *next, *to_c;
> >  
> >      QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
> > +        if (c->role == &child_backing) {
> > +            /* @from is generally not allowed to be a backing file, except for
> > +             * when @to is the overlay. In that case, @from may not be replaced
> > +             * by @to as @to's backing node. */
> > +            QLIST_FOREACH(to_c, &to->children, next) {
> > +                if (to_c == c) {
> > +                    break;
> > +                }
> > +            }
> 
> Why not just test "c->bs == to" here, why is it necessary to iterate through
> to->children list?

Because c->bs == from. We want to check whether 'to' is the parent, not
whether it is the child.

Kevin