From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42296) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDXF1-0005yB-PH for qemu-devel@nongnu.org; Fri, 30 Mar 2012 04:31:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SDXEu-0008AZ-UL for qemu-devel@nongnu.org; Fri, 30 Mar 2012 04:31:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51970) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDXEu-0008AS-Lh for qemu-devel@nongnu.org; Fri, 30 Mar 2012 04:31:20 -0400 Message-ID: <4F756F51.5010603@redhat.com> Date: Fri, 30 Mar 2012 10:31:13 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1333037320-20564-1-git-send-email-pbonzini@redhat.com> <20120330071921.GB16159@stefanha-thinkpad.localdomain> In-Reply-To: <20120330071921.GB16159@stefanha-thinkpad.localdomain> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block: fix streaming/closing race List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com Il 30/03/2012 09:19, Stefan Hajnoczi ha scritto: >> > +void block_job_cancel_sync(BlockJob *job) >> > +{ >> > + BlockDriverState *bs = job->bs; >> > + >> > + assert(bs->job == job); >> > + block_job_cancel(job); >> > + while (bs->job != NULL && bs->job->busy) { > It's not clear to me why we have a busy flag. Because the coroutine does not restart if you do qemu_aio_wait and the coroutine is waiting in co_sleep. So the busy flag communicates that the coroutine is quiescent and, when cancelled, will not issue any new I/O. However, even this is not enough. It fixes a race with closing, but not with deleting bs. So the bug does not show anymore when you quit QEMU (because the coroutine is not restarted), but it is still there if you hot-unplug a device while streaming is in effect. > Is canceling and waiting for bs->job == NULL not enough? Perhaps > you're trying to take a shortcut and not wait until the job is fully > stopped No, simply it's impossible to wait for full completion with qemu_aio_wait. Paolo