Re: [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

2011-08-04 Thread Stefan Hajnoczi
On Thu, Aug 4, 2011 at 11:38 AM, Fam Zheng  wrote:
> On Thu, Aug 4, 2011 at 6:34 PM, Stefan Hajnoczi  wrote:
>> On Thu, Aug 4, 2011 at 4:09 AM, Fam Zheng  wrote:
>>> +static void vmdk_free_last_extent(BlockDriverState *bs)
>>> +{
>>> +    BDRVVmdkState *s = bs->opaque;
>>> +
>>> +    if (s->num_extents == 0) {
>>> +        return;
>>> +    }
>>> +    s->num_extents--;
>>> +    s->extents = qemu_realloc(s->extents, s->num_extents * 
>>> sizeof(VmdkExtent));
>>
>> vmdk_free_extents() frees extent->l1_table, extent->l2_cache, and
>> extent->l1_backup_table.  Are they being leaked here?
>
> No, it's only called after vmdk_init_tables fails, where no table is
> actually allocated to the extent.

Okay, thanks for explaining.

Stefan



Re: [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

2011-08-04 Thread Fam Zheng
On Thu, Aug 4, 2011 at 6:34 PM, Stefan Hajnoczi  wrote:
> On Thu, Aug 4, 2011 at 4:09 AM, Fam Zheng  wrote:
>> +static void vmdk_free_last_extent(BlockDriverState *bs)
>> +{
>> +    BDRVVmdkState *s = bs->opaque;
>> +
>> +    if (s->num_extents == 0) {
>> +        return;
>> +    }
>> +    s->num_extents--;
>> +    s->extents = qemu_realloc(s->extents, s->num_extents * 
>> sizeof(VmdkExtent));
>
> vmdk_free_extents() frees extent->l1_table, extent->l2_cache, and
> extent->l1_backup_table.  Are they being leaked here?

No, it's only called after vmdk_init_tables fails, where no table is
actually allocated to the extent.


-- 
Best regards!
Fam Zheng



Re: [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

2011-08-04 Thread Stefan Hajnoczi
On Thu, Aug 4, 2011 at 4:09 AM, Fam Zheng  wrote:
> +static void vmdk_free_last_extent(BlockDriverState *bs)
> +{
> +    BDRVVmdkState *s = bs->opaque;
> +
> +    if (s->num_extents == 0) {
> +        return;
> +    }
> +    s->num_extents--;
> +    s->extents = qemu_realloc(s->extents, s->num_extents * 
> sizeof(VmdkExtent));

vmdk_free_extents() frees extent->l1_table, extent->l2_cache, and
extent->l1_backup_table.  Are they being leaked here?

Stefan