On Thu 17 Sep 2015 03:48:09 PM CEST, Kevin Wolf <[email protected]> wrote:
> @@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)
> bdrv_unref(backing_hd);
> }
>
> + if (bs->file != NULL) {
> + bdrv_unref(bs->file->bs);
> + bs->file = NULL;
> + }
> +
> QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
> /* TODO Remove bdrv_unref() from drivers' close function and use
> * bdrv_unref_child() here */
> @@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs)
> bs->options = NULL;
> QDECREF(bs->full_open_options);
> bs->full_open_options = NULL;
> -
> - if (bs->file != NULL) {
> - bdrv_unref(bs->file);
> - bs->file = NULL;
> - }
> }
You are moving bdrv_unref(bs->file) up in the function and this seems to
be causing a problem, by turning this:
bs->drv->bdrv_close(bs);
bs->drv = NULL;
bdrv_unref(bs->file);
into this:
bs->drv->bdrv_close(bs);
bdrv_unref(bs->file);
bs->drv = NULL;
In the latter case, closing bs->file calls aio_poll(). This can trigger
new requests on bs, which at that point has a valid pointer to a
BlockDriver that has already been closed. If throttling is enabled on bs
those requests might be queued for later. At the point in which those
requests are scheduled bs->drv is already NULL, crashing QEMU. I can
reproduce this easily with x-data-plane=on.
I sent a separate patch that moves the bdrv_io_limits_disable() call to
the beginning of bdrv_close(). That solves the crash, but I guess that
this patch should also be changed.
Berto
