Commit 0a6c86d0 authored by John Snow's avatar John Snow
Browse files

blockdev: acquire aio_context for bitmap add/remove 

When bitmaps are persistent, they may incur a disk read or write when bitmaps
are added or removed. For configurations like virtio-dataplane, failing to
acquire this lock will abort QEMU when disk IO occurs.

We used to acquire aio_context as part of the bitmap lookup, so re-introduce
the lock for just the cases that have an IO penalty. Commit 2119882c removed
these locks, and I failed to notice this when we committed fd5ae4cc, so this
has been broken since persistent bitmaps were introduced.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1672010


Reported-By: default avatarAihua Liang <aliang@redhat.com>
Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Message-id: 20190218233154.19303-1-jsnow@redhat.com
Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
parent 73ab5d60

blockdev.c

+20 −6
Original line number Diff line number Diff line
@@ -2820,6 +2820,7 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name, 
{

    BlockDriverState *bs;

    BdrvDirtyBitmap *bitmap;

    AioContext *aio_context = NULL;



    if (!name || name[0] == '\0') {

        error_setg(errp, "Bitmap name cannot be empty");
@@ -2854,15 +2855,17 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name, 
        disabled = false;

    }



    if (persistent &&

        !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))

    {

        return;

    if (persistent) {

        aio_context = bdrv_get_aio_context(bs);

        aio_context_acquire(aio_context);

        if (!bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp)) {

            goto out;

        }

    }



    bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);

    if (bitmap == NULL) {

        return;

        goto out;

    }



    if (disabled) {
@@ -2870,6 +2873,10 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name, 
    }



    bdrv_dirty_bitmap_set_persistance(bitmap, persistent);

 out:

    if (aio_context) {

        aio_context_release(aio_context);

    }

}



void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
@@ -2878,6 +2885,7 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, 
    BlockDriverState *bs;

    BdrvDirtyBitmap *bitmap;

    Error *local_err = NULL;

    AioContext *aio_context = NULL;



    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);

    if (!bitmap || !bs) {
@@ -2892,14 +2900,20 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, 
    }



    if (bdrv_dirty_bitmap_get_persistance(bitmap)) {

        aio_context = bdrv_get_aio_context(bs);

        aio_context_acquire(aio_context);

        bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err);

        if (local_err != NULL) {

            error_propagate(errp, local_err);

            return;

            goto out;

        }

    }



    bdrv_release_dirty_bitmap(bs, bitmap);

 out:

    if (aio_context) {

        aio_context_release(aio_context);

    }

}



/**