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<kw...@redhat.com>
>> ---
>> 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 <s...@weilnetz.de>
Thanks.
Kevin
