Am 05.03.2020 um 16:54 hat Alberto Garcia geschrieben:
> On Thu 27 Feb 2020 07:18:04 PM CET, Kevin Wolf wrote:
> > /*
> > - * TODO: before removing the x- prefix from x-blockdev-reopen we
> > - * should move the new backing file into the right AioContext
> > - * instead of returning an error.
> > + * Check AioContext compatibility so that the bdrv_set_backing_hd()
> > call in
> > + * bdrv_reopen_commit() won't fail.
> > */
> > - if (new_backing_bs) {
> > - if (bdrv_get_aio_context(new_backing_bs) !=
> > bdrv_get_aio_context(bs)) {
> > - error_setg(errp, "Cannot use a new backing file "
> > - "with a different AioContext");
> > - return -EINVAL;
> > - }
> > + if (!bdrv_reopen_can_attach(bs->backing, bs, new_backing_bs, errp)) {
> > + return -EINVAL;
> > }
>
> What happens here now if 'new_backing_bs' is NULL ?
>
> It seems that you would be calling bdrv_can_set_aio_context(NULL, ...),
> and it looks like that would crash.
Not sure why I thought that this check isn't needed any more...
It actually works as long as everything runs in the main loop context
(because bdrv_get_aio_context(NULL) return the main context, so there is
nothing to do), which is why the test cases didn't fail. But as soon as
you move things to a different AioContext, they will fail.
Maybe even worse, the argument order for bdrv_reopen_can_attach() is
wrong.
Thanks for catching this, I'll send a v2.
Kevin
