Re: [PATCH] block: Fail gracefully when blockdev-snapshot creates loops

[Kevin Wolf]

Am 18.10.2021 um 16:22 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 18.10.2021 16:47, Kevin Wolf wrote:
> > Using blockdev-snapshot to append a node as an overlay to itself, or to
> > any of its parents, causes crashes. Catch the condition and return an
> > error for these cases instead.
> > 
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1824363
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   block.c                    | 10 ++++++++++
> >   tests/qemu-iotests/085     | 31 ++++++++++++++++++++++++++++++-
> >   tests/qemu-iotests/085.out | 33 ++++++++++++++++++++++++++++++---
> >   3 files changed, 70 insertions(+), 4 deletions(-)
> > 
> > diff --git a/block.c b/block.c
> > index 45f653a88b..231dddf655 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -84,6 +84,9 @@ static BlockDriverState *bdrv_open_inherit(const char 
> > *filename,
> >                                              BdrvChildRole child_role,
> >                                              Error **errp);
> > +static bool bdrv_recurse_has_child(BlockDriverState *bs,
> > +                                   BlockDriverState *child);
> > +
> >   static void bdrv_replace_child_noperm(BdrvChild *child,
> >                                         BlockDriverState *new_bs);
> >   static void bdrv_remove_file_or_backing_child(BlockDriverState *bs,
> > @@ -2673,6 +2676,7 @@ static void bdrv_replace_child_noperm(BdrvChild 
> > *child,
> >       int drain_saldo;
> >       assert(!child->frozen);
> > +    assert(old_bs != new_bs);
> >       if (old_bs && new_bs) {
> >           assert(bdrv_get_aio_context(old_bs) == 
> > bdrv_get_aio_context(new_bs));
> > @@ -2892,6 +2896,12 @@ static int bdrv_attach_child_noperm(BlockDriverState 
> > *parent_bs,
> >       assert(parent_bs->drv);
> > +    if (bdrv_recurse_has_child(child_bs, parent_bs)) {
> > +        error_setg(errp, "Making '%s' a %s child of '%s' would create a 
> > cycle",
> > +                   parent_bs->node_name, child_name, child_bs->node_name);
> 
> Seems, child_bs and parent_bs should be swapped.

Oops, thanks. I'm fixing it up while applying.

Kevin