[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vdi: Fix memory leak
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] vdi: Fix memory leak |
Date: |
Wed, 23 Nov 2011 09:26:40 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111115 Thunderbird/8.0 |
Am 22.11.2011 19:37, schrieb Stefan Weil:
> Am 22.11.2011 17:16, schrieb Kevin Wolf:
>> The block map is allocated in vdi_open, but was never freed.
>>
>> Signed-off-by: Kevin Wolf<address@hidden>
>> ---
>> Applies on top if the migration blocker series.
>>
>> block/vdi.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/block/vdi.c b/block/vdi.c
>> index 7dda522..02da6b4 100644
>> --- a/block/vdi.c
>> +++ b/block/vdi.c
>> @@ -949,6 +949,9 @@ static int vdi_create(const char *filename,
>> QEMUOptionParameter *options)
>> static void vdi_close(BlockDriverState *bs)
>> {
>> BDRVVdiState *s = bs->opaque;
>> +
>> + g_free(s->bmap);
>> +
>> migrate_del_blocker(s->migration_blocker);
>> error_free(s->migration_blocker);
>> }
>
> If vdi_close is called after a jump to label fail_free_bmap,
> g_free(s->bmap) will be called twice.
bdrv_close() may only ever be called on a successfully opened image.
In fact, block.c already ensures this. The BDRVVdiState would already be
freed in the failing bdrv_open_common() and bs->drv would be set to
NULL, so that an invalid bdrv_close() would end up doing nothing.
> Setting s->bmap = NULL after g_free in fail_free_bmap
> would be safer.
>
> Otherwise your patch is fine. If you send an update, you
> can add
>
> Reviewed-by: Stefan Weil <address@hidden>
Thanks.
Kevin