[Qemu-devel] Re: [patch 4/5][v3] Aggregate same type clusters.

[Kevin Wolf <kwolf@suse.de>]

Anthony Liguori schrieb:
> Kevin Wolf wrote:
>> Laurent.Vivier@bull.net schrieb:
>>  
>>>      /* seek the the l2 offset in the l1 table */
>>>  
>>> -    l1_index = offset >> (s->l2_bits + s->cluster_bits);
>>> +    l1_index = offset >> l1_bits;
>>>      if (l1_index >= s->l1_size)
>>> -        return 0;
>>> +        goto out;
>>>  
>>>      l2_offset = s->l1_table[l1_index];
>>>  
>>>      /* seek the l2 table of the given l2 offset */
>>>  
>>>      if (!l2_offset)
>>> -        return 0;
>>> +        goto out;
>>>  
>>>      /* load the l2 table in memory */
>>>  
>>>      l2_offset &= ~QCOW_OFLAG_COPIED;
>>>      l2_table = l2_load(bs, l2_offset);
>>>      if (l2_table == NULL)
>>> -        return 0;
>>> +        goto out;
>>>     
>>
>> You agreed that return 0 is actually the right thing to do here because
>> this is a real error.
>>   
> 
> I'm inclined to apply this patch (and the rest of the series) and then
> when Laurent gets back, we can have another patch that changes this back
> to return 0.  Any objections?

I'd prefer that you change it before committing. If we don't return 0
here, the L2 table is considered free if it can't be loaded. So
returning 0 is definitely safer.

But then, if you can't load the L2 table, you're in trouble anyway...
It's your decision, I can live with both.

Kevin