From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] Re: [PATCH 1/3] Only call aio flush handler if set Date: Tue, 23 Sep 2008 09:34:55 -0500 Message-ID: <48D8FE8F.9000702@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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org To: Ryan Harper Return-path: Received: from e2.ny.us.ibm.com ([32.97.182.142]:59415 "EHLO e2.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751248AbYIWOfy (ORCPT ); Tue, 23 Sep 2008 10:35:54 -0400 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by e2.ny.us.ibm.com (8.13.8/8.13.8) with ESMTP id m8NEZrQI004381 for ; Tue, 23 Sep 2008 10:35:53 -0400 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id m8NEZrCC216488 for ; Tue, 23 Sep 2008 10:35:53 -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 m8NEZqG3024738 for ; Tue, 23 Sep 2008 10:35:53 -0400 In-Reply-To: <20080923142622.GJ31395@us.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: 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