From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38831) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1csqCh-0003Wq-8e for qemu-devel@nongnu.org; Tue, 28 Mar 2017 08:26:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1csqCg-0000qj-8d for qemu-devel@nongnu.org; Tue, 28 Mar 2017 08:26:27 -0400 Date: Tue, 28 Mar 2017 20:26:13 +0800 From: Fam Zheng Message-ID: <20170328122613.GD5207@lemon.lan> References: <20170316212351.13797-1-jsnow@redhat.com> <20170324152755.GR12560@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20170324152755.GR12560@stefanha-x1.localdomain> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/3] block: pause block jobs for bdrv_drain_begin/end List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Paolo Bonzini , Kevin Wolf , John Snow , qemu-devel , qemu block On Fri, 03/24 15:27, Stefan Hajnoczi wrote: > On Thu, Mar 23, 2017 at 06:57:14PM +0100, Paolo Bonzini wrote: > >=20 > >=20 > > On 23/03/2017 18:44, Stefan Hajnoczi wrote: > > >> It's possible to wedge QEMU if the guest tries to reset a virtio-p= ci > > >> device as QEMU is also using the drive for a blockjob. This patchs= et > > >> aims to allow us to safely pause/resume jobs attached to individua= l > > >> nodes in a manner similar to how bdrv_drain_all_begin/end do. > > >=20 > > > Weird, I thought the 0 nanosecond sleep that block jobs do in their > > > main loop allows aio_poll() loops to finish. > >=20 > > The 0 nanosecond sleep is now done in the BDS AioContext rather than = in=20 > > the "non-aio_poll-aware" main loop: > >=20 > > commit 0b9caf9b3166c8deb3c4f3a774c2384b069dc29c > > Author: Fam Zheng > > Date: Tue Aug 26 15:15:43 2014 +0800 > >=20 > > coroutine: Drop co_sleep_ns > > =20 > > block_job_sleep_ns is the only user. Since we are moving towards > > AioContext aware code, it's better to use the explicit version an= d drop > > the old one. > > =20 > > Signed-off-by: Fam Zheng > > Reviewed-by: Beno=EEt Canet > > Signed-off-by: Stefan Hajnoczi >=20 > But we hold the AioContext lock and are calling aio_poll(), so I would > expect our loop to terminate. The blockjob coroutine should still be > leaving this little gap in activity during which the aio_poll() loop > finishes. I am not sure, but at each gap, aio_poll() is free to fire the 0 nanoseco= nd sleep timer cb already, which will generate more I/O. The correct thing i= s, like in this series, set the pause flag. Fam