From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34719) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V0E9u-0004ib-SP for qemu-devel@nongnu.org; Fri, 19 Jul 2013 13:08:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V0E9t-0004qJ-V5 for qemu-devel@nongnu.org; Fri, 19 Jul 2013 13:07:58 -0400 Received: from mail-ee0-x236.google.com ([2a00:1450:4013:c00::236]:52045) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V0E9t-0004qE-Of for qemu-devel@nongnu.org; Fri, 19 Jul 2013 13:07:57 -0400 Received: by mail-ee0-f54.google.com with SMTP id t10so2512714eei.41 for ; Fri, 19 Jul 2013 10:07:57 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51E9725E.8090201@redhat.com> Date: Fri, 19 Jul 2013 19:07:42 +0200 From: Paolo Bonzini 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> In-Reply-To: <20130719073344.GA9667@stefanha-thinkpad.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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: pl@kamp.de, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com 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. Paolo