From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ryan Harper Subject: Re: [Qemu-devel] Re: [PATCH 1/3] Only call aio flush handler if set Date: Tue, 23 Sep 2008 09:41:52 -0500 Message-ID: <20080923144152.GL31395@us.ibm.com> References: <1222125454-21744-1-git-send-email-ryanh@us.ibm.com> <1222125454-21744-2-git-send-email-ryanh@us.ibm.com> <48D8568F.2060206@us.ibm.com> <20080923142622.GJ31395@us.ibm.com> <48D8FE8F.9000702@us.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Ryan Harper , qemu-devel@nongnu.org, kvm@vger.kernel.org To: Anthony Liguori Return-path: Received: from e34.co.us.ibm.com ([32.97.110.152]:50080 "EHLO e34.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752368AbYIWOl6 (ORCPT ); Tue, 23 Sep 2008 10:41:58 -0400 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e34.co.us.ibm.com (8.13.8/8.13.8) with ESMTP id m8NEfvtO031754 for ; Tue, 23 Sep 2008 10:41:57 -0400 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id m8NEftWJ123668 for ; Tue, 23 Sep 2008 08:41:56 -0600 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m8NEfsJu022107 for ; Tue, 23 Sep 2008 08:41:54 -0600 Content-Disposition: inline In-Reply-To: <48D8FE8F.9000702@us.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: * Anthony Liguori [2008-09-23 09:36]: > Ryan Harper wrote: > >* Anthony Liguori [2008-09-22 21:49]: > > > >>Ryan Harper wrote: > >> > >>>If the aio handler doesn't register an io_flush handler, we'd SEGV; fix > >>>that by > >>>only calling the flush handler if set. BTW, aio handlers *should* > >>>register an > >>>io_flush routine. > >>> > >>>Signed-off-by: Ryan Harper > >>> > >>>diff --git a/aio.c b/aio.c > >>>index 687e4be..2bb3ed4 100644 > >>>--- a/aio.c > >>>+++ b/aio.c > >>>@@ -105,7 +105,8 @@ void qemu_aio_flush(void) > >>> ret = 0; > >>> > >>> LIST_FOREACH(node, &aio_handlers, node) { > >>>- ret |= node->io_flush(node->opaque); > >>>+ if (node->io_flush) > >>>+ ret |= node->io_flush(node->opaque); > >>> } > >>> > >>> > >>Just not doing an io_flush is just hiding the real bug--that the user > >>didn't register an io_flush handler. If the inevitable SEGV is not your > >> > > > >That may be true, but it it is no different than the check for read and > >write handlers in qemu_aio_wait(). > > > > Read and write handlers are optional. I guess in practice one or the > other should be set but neither one is individually required. The > problem with your patch is that it takes something that is a bug, and > makes it more difficult to spot. So it actually makes things worse. I disagree that anything is worse off by not SEGV'ing, In any case, what do you want here? Read or Write must be set along with flush or we error in fd registration? -- Ryan Harper Software Engineer; Linux Technology Center IBM Corp., Austin, Tx (512) 838-9253 T/L: 678-9253 ryanh@us.ibm.com