From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:37356) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNlev-0005QL-Hx for qemu-devel@nongnu.org; Mon, 15 Oct 2012 10:28:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TNles-0003pr-FC for qemu-devel@nongnu.org; Mon, 15 Oct 2012 10:28:45 -0400 Message-ID: <507C1D95.9030402@suse.de> Date: Mon, 15 Oct 2012 16:28:37 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20120513160331.5b774c93.yizhouzhou@ict.ac.cn> <4FB0F89F.6080306@redhat.com> <4FD74513.2000500@suse.de> <4FD747BF.3020809@redhat.com> <50783CCA.902@suse.de> <507BD3D1.3010804@redhat.com> In-Reply-To: <507BD3D1.3010804@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] fixing qemu-0.1X endless loop in qcow2_alloc_cluster_offset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, qemu-stable@nongnu.org, Michael Tokarev , Zhouyi Zhou , Bruce Rogers Am 15.10.2012 11:13, schrieb Kevin Wolf: > Am 12.10.2012 17:52, schrieb Andreas F=E4rber: >> Am 12.06.2012 15:44, schrieb Kevin Wolf: >>> Am 12.06.2012 15:33, schrieb Andreas F=E4rber: >>>> Am 14.05.2012 14:20, schrieb Kevin Wolf: >>>>> Am 13.05.2012 10:03, schrieb Zhouyi Zhou: >>>>>> sometimes, qemu/kvm-0.1x will hang in endless loop in qcow2_allo= c_cluster_offset. >>>>> >>>>> The patch looks reasonable to me. Note however that while it fixes = the >>>>> hang, it still causes cluster leaks. I'm not sure if someone is >>>>> interested in picking these up for old stable releases. Andreas, I = think >>>>> you were going to take 0.15? The first version that doesn't have th= e >>>>> problem is 1.0. >>>> >>> It's "fixed" as a side effect of the block layer conversion to >>> coroutines. Not exactly the kind of patches you'd want to cherry-pick >>> for stable-0.15. >>> >>> The better fix for 0.15 could be to backport the new behaviour of >>> coroutine based requests with bdrv_aio_cancel: >>> >>> static void bdrv_aio_co_cancel_em(BlockDriverAIOCB *blockacb) >>> { >>> qemu_aio_flush(); >>> } >>> >>> Using that as the implementation for qcow2_aio_cancel should be safe = and >>> fix this problem. >> >> [...] stable-0.15 does not have coroutines, so I don't >> understand what exactly you're suggesting as alternative here: Backpor= t >> the whole coroutine feature including coroutine function above? Or jus= t >> call qemu_aio_flush() in place of what? This is old qcow2_aio_cancel()= : >=20 > No, that was qcow2_aio_flush. ;-) Ugh, what a copy-and-paste error... ;-) > What I'm suggesting (not even compile tested!) is: >=20 > Signed-off-by: Kevin Wolf >=20 > diff --git a/block/qcow2.c b/block/qcow2.c > index 48e1b95..d665675 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -388,10 +388,7 @@ typedef struct QCowAIOCB { >=20 > static void qcow2_aio_cancel(BlockDriverAIOCB *blockacb) > { > - QCowAIOCB *acb =3D container_of(blockacb, QCowAIOCB, common); > - if (acb->hd_aiocb) > - bdrv_aio_cancel(acb->hd_aiocb); > - qemu_aio_release(acb); > + qemu_aio_flush(); > } >=20 > static AIOPool qcow2_aio_pool =3D { Compiles fine. Is there a particular test case to invoke this code path? Does this attempt to fix the cluster leaks you mentioned as well, or just the cluster allocation endless loop? Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg