From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ki8zg-0003Z7-7S for qemu-devel@nongnu.org; Tue, 23 Sep 2008 10:36:00 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Ki8ze-0003Xu-ED for qemu-devel@nongnu.org; Tue, 23 Sep 2008 10:35:59 -0400 Received: from [199.232.76.173] (port=53500 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ki8ze-0003Xr-3k for qemu-devel@nongnu.org; Tue, 23 Sep 2008 10:35:58 -0400 Received: from e4.ny.us.ibm.com ([32.97.182.144]:39481) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Ki8zd-0007Ly-Jj for qemu-devel@nongnu.org; Tue, 23 Sep 2008 10:35:58 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e4.ny.us.ibm.com (8.13.8/8.13.8) with ESMTP id m8NEZrSW004781 for ; Tue, 23 Sep 2008 10:35:53 -0400 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id m8NEZq8d230628 for ; Tue, 23 Sep 2008 10:35:52 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m8NEZqFx024738 for ; Tue, 23 Sep 2008 10:35:52 -0400 Message-ID: <48D8FE8F.9000702@us.ibm.com> Date: Tue, 23 Sep 2008 09:34:55 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH 1/3] Only call aio flush handler if set 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> In-Reply-To: <20080923142622.GJ31395@us.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ryan Harper Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org 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. Regards, Anthony Liguori