From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MJ9I6-0003y2-Lc for qemu-devel@nongnu.org; Tue, 23 Jun 2009 12:56:14 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MJ9I2-0003vy-72 for qemu-devel@nongnu.org; Tue, 23 Jun 2009 12:56:14 -0400 Received: from [199.232.76.173] (port=58104 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MJ9I1-0003vn-Ki for qemu-devel@nongnu.org; Tue, 23 Jun 2009 12:56:09 -0400 Received: from mx2.redhat.com ([66.187.237.31]:33405) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MJ9I0-0007mu-Uj for qemu-devel@nongnu.org; Tue, 23 Jun 2009 12:56:09 -0400 Message-ID: <4A41095D.7070608@redhat.com> Date: Tue, 23 Jun 2009 19:57:01 +0300 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] block: Clean up after deleting BHs References: <1245763236-23464-1-git-send-email-avi@redhat.com> <20090623165006.GC27211@lst.de> In-Reply-To: <20090623165006.GC27211@lst.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christoph Hellwig Cc: qemu-devel@nongnu.org On 06/23/2009 07:50 PM, Christoph Hellwig wrote: > On Tue, Jun 23, 2009 at 04:20:36PM +0300, Avi Kivity wrote: > >> Commit 6a7ad299 ("Call qemu_bh_delete at bdrv_aio_bh_cb") deletes emulated >> aio bottom halves to prevent endless accumulation. However, it leaves a >> stale ->bh pointer, which is then waited on when the aio is reused. >> >> Zeroing the pointer fixes the issue, allowing vmdk format images to be used. >> > > What operations on vmdk images does this cause to fail? qemu-iotests > seems to do fine on vmdk so it's nothing yet exercised by it. > Just starting qemu with a vmdk image hangs. I think the very first read triggers it. >> --- a/block.c >> +++ b/block.c >> @@ -1374,6 +1374,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb) >> { >> BlockDriverAIOCBSync *acb = (BlockDriverAIOCBSync *)blockacb; >> qemu_bh_delete(acb->bh); >> + acb->bh = NULL; >> qemu_aio_release(acb); >> } >> >> @@ -1391,6 +1392,7 @@ static void bdrv_aio_bh_cb(void *opaque) >> qemu_vfree(acb->bounce); >> acb->common.cb(acb->common.opaque, acb->ret); >> qemu_bh_delete(acb->bh); >> + acb->bh = NULL; >> qemu_aio_release(acb); >> } >> > > I think not having the state of the private acb area cleared over a > free/realloc cycle is pretty dangerous. Wouldn't it be better to always > clear that space in qemu_aio_get? > Maybe, but that's a bigger change. Let's start with this (in stable- too) and rework aio later. -- error compiling committee.c: too many arguments to function