[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] blockdev: acquire aio_context for bitmap add/re
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH] blockdev: acquire aio_context for bitmap add/remove |
Date: |
Tue, 12 Feb 2019 14:43:35 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
On 2/12/19 2:36 PM, Eric Blake wrote:
> On 2/6/19 11:02 AM, John Snow wrote:
>> 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.
>>
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1672010
>> Reported-By: Aihua Liang <address@hidden>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>> blockdev.c | 24 +++++++++++++++++++-----
>> 1 file changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index fb18e9c975..ce458de037 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -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,10 +2855,12 @@ 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;
> }
>
> Oops - this early return fails to release the aio context.
Good spot, thanks! This would be awful to diagnose later.