From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1KTgRy-0004pP-Cy
	for qemu-devel@nongnu.org; Thu, 14 Aug 2008 13:17:26 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1KTgRw-0004oX-BT
	for qemu-devel@nongnu.org; Thu, 14 Aug 2008 13:17:25 -0400
Received: from [199.232.76.173] (port=37832 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1KTgRw-0004oK-3D
	for qemu-devel@nongnu.org; Thu, 14 Aug 2008 13:17:24 -0400
Received: from mx2.suse.de ([195.135.220.15]:49564)
	by monty-python.gnu.org with esmtps
	(TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60)
	(envelope-from <kwolf@suse.de>) id 1KTgRv-0000Eg-UB
	for qemu-devel@nongnu.org; Thu, 14 Aug 2008 13:17:24 -0400
Message-ID: <48A4686B.1020608@suse.de>
Date: Thu, 14 Aug 2008 19:16:27 +0200
From: Kevin Wolf <kwolf@suse.de>
MIME-Version: 1.0
References: <20080813145900.265987809@bull.net>
	<20080813145944.132440698@bull.net> <48A454E0.2010303@suse.de>
	<48A45B43.5070904@codemonkey.ws>
In-Reply-To: <48A45B43.5070904@codemonkey.ws>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [patch 4/5][v3] Aggregate same type clusters.
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Laurent.Vivier@bull.net, qemu-devel@nongnu.org

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