From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46420) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9XkB-0005Um-Em for qemu-devel@nongnu.org; Thu, 17 Dec 2015 07:33:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9Xk7-00089a-A9 for qemu-devel@nongnu.org; Thu, 17 Dec 2015 07:33:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52684) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9Xk7-00089S-1O for qemu-devel@nongnu.org; Thu, 17 Dec 2015 07:33:11 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 8CD4332D3AC for ; Thu, 17 Dec 2015 12:33:10 +0000 (UTC) Date: Thu, 17 Dec 2015 20:33:06 +0800 From: Fam Zheng Message-ID: <20151217123306.GB26335@ad.usersys.redhat.com> References: <1450290827-30508-5-git-send-email-pbonzini@redhat.com> <20151217005946.GA20007@ad.usersys.redhat.com> <567275D4.7030605@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <567275D4.7030605@redhat.com> Subject: Re: [Qemu-devel] [PATCH] block: fix bdrv_ioctl called from coroutine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On Thu, 12/17 09:44, Paolo Bonzini wrote: > > > On 17/12/2015 01:59, Fam Zheng wrote: > > On Wed, 12/16 19:33, Paolo Bonzini wrote: > >> When called from a coroutine, bdrv_ioctl must be asynchronous just like > >> e.g. bdrv_flush. The code was incorrectly making it synchronous, fix > >> it. > >> > >> Signed-off-by: Paolo Bonzini > >> --- > >> Fam, any reason why you did it this way? I don't see > >> any coroutine caller, but it doesn't make much sense. :) > > > > That is a surprising question! From a coroutine, it is bdrv_flush -> > > bdrv_flush_co_entry -> bdrv_co_flush, which I think is always synchronous, > > especially, noticing the code around calling bs->bdrv_aio_flush: > > > > acb = bs->drv->bdrv_aio_flush(bs, bdrv_co_io_em_complete, &co); > > if (acb == NULL) { > > ret = -EIO; > > } else { > > qemu_coroutine_yield(); > > ret = co.ret; > > } > > > > Am I missing something? > > In the coroutine case, the yield is hidden in the drivers, and it may or > may not happen. For example, qcow2_co_flush_to_os starts with > > qemu_co_mutex_lock(&s->lock); > > which can yield. bdrv_ioctl, on the contrary, is emulated with .bdrv_aio_ioctl, so it always yields (unless -ENOTSUP), that's why I think aio_poll() is necessary in both branches. Fam