From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47799)
	by lists.gnu.org with esmtp (Exim 4.71) (envelope-from <pl@kamp.de>)
	id 1V1jz3-0002VU-3D
	for qemu-devel@nongnu.org; Tue, 23 Jul 2013 17:19:04 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pl@kamp.de>) id 1V1jz0-000675-J4
	for qemu-devel@nongnu.org; Tue, 23 Jul 2013 17:19:00 -0400
Received: from mx.ipv6.kamp.de ([2a02:248:0:51::16]:55690 helo=mx01.kamp.de)
	by eggs.gnu.org with smtp (Exim 4.71) (envelope-from <pl@kamp.de>)
	id 1V1jyz-00066v-Ss
	for qemu-devel@nongnu.org; Tue, 23 Jul 2013 17:18:58 -0400
Message-ID: <51EEF33E.1020906@kamp.de>
Date: Tue, 23 Jul 2013 23:18:54 +0200
From: Peter Lieven <pl@kamp.de>
MIME-Version: 1.0
References: <1373992168-26043-1-git-send-email-pbonzini@redhat.com>
	<1373992168-26043-18-git-send-email-pbonzini@redhat.com>
	<20130719073344.GA9667@stefanha-thinkpad.redhat.com>
	<51E9725E.8090201@redhat.com>
In-Reply-To: <51E9725E.8090201@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 17/17] block: look for zero blocks in
	bs->file
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com

Am 19.07.2013 19:07, schrieb Paolo Bonzini:
> Il 19/07/2013 09:33, Stefan Hajnoczi ha scritto:
>> On Tue, Jul 16, 2013 at 06:29:28PM +0200, Paolo Bonzini wrote:
>>> diff --git a/block.c b/block.c
>>> index 557ce29..2d7d71f 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -2977,7 +2977,7 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
>>>                                                       int nb_sectors, int *pnum)
>>>  {
>>>      int64_t n;
>>> -    int64_t ret;
>>> +    int64_t ret, ret2;
>>>  
>>>      if (sector_num >= bs->total_sectors) {
>>>          *pnum = 0;
>>> @@ -3003,6 +3003,14 @@ static int64_t coroutine_fn bdrv_co_get_block_status(BlockDriverState *bs,
>>>          ret |= BDRV_BLOCK_ZERO;
>>>      }
>>>  
>>> +    if (bs->file &&
>>> +        (ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&
>>> +        (ret & BDRV_BLOCK_OFFSET_VALID)) {
>>> +        ret2 = bdrv_co_get_block_status(bs->file, ret >> BDRV_SECTOR_BITS,
>>> +                                        *pnum, pnum);
>>> +        ret |= (ret2 & BDRV_BLOCK_ZERO);
>>> +    }
>> This patch breaks qemu-iotests 030 (image streaming).
>>
>> The problem is that bdrv_co_get_block_status() uses bs->total_sectors
>> directly instead of calling bdv_get_geometry()/bdrv_getlength().
>>
>> With qcow2 the bs->file can grow on disk.  We don't update
>> bs->total_sectors.
>>
>> Then this patch calls bdrv_co_get_block_status(bs->file) where we fail
>> with *pnum = 0, ret = 0 because bs->total_sectors suggests it is beyond
>> the end of the file.
>>
>> The result is that 030 goes into an infinite loop.
>>
>> As a quick test I switched the direct bs->total_sectors accesses to
>> bdrv_get_geometry() and it stopped hanging.  Perhaps the
>> bs->total_sectors caching needs to be improved though.
> Yes, fixing the caching also resolves the failure.  I'll send a v3 next
> Monday or perhaps Sunday.
Do you have the v3 ready?