From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38586) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvXUl-0000iu-8v for qemu-devel@nongnu.org; Wed, 16 Jan 2013 13:13:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TvXUj-00047V-W0 for qemu-devel@nongnu.org; Wed, 16 Jan 2013 13:13:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57731) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TvXUj-00047K-OM for qemu-devel@nongnu.org; Wed, 16 Jan 2013 13:13:49 -0500 Message-ID: <50F6EDD4.3070206@redhat.com> Date: Wed, 16 Jan 2013 19:13:40 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1358359893-718-1-git-send-email-kwolf@redhat.com> In-Reply-To: <1358359893-718-1-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] aio-posix: Fix return value of aio_poll() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com, Michael Roth Il 16/01/2013 19:11, Kevin Wolf ha scritto: > aio_poll() must return true if any work is still pending, even if it > didn't make progress, so that qemu_aio_wait() doesn't return too early. > The possibility of returning early occasionally lead to a failed > assertion in bdrv_drain_all(), when some in-flight request was missed > and the function didn't really drain all requests. > > In order to make that change, the return value as specified in the > function comment must change for blocking = false; fortunately, the > return value of blocking = false callers is only used in test cases, so > this change shouldn't cause any trouble. > > Signed-off-by: Kevin Wolf Looks good, but please make the same change in aio-win32.c, and add a Cc to qemu-stable@nongnu.org Paolo > --- > aio-posix.c | 3 ++- > include/block/aio.h | 6 ++---- > tests/test-aio.c | 4 ++-- > 3 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/aio-posix.c b/aio-posix.c > index 88d09e1..fe4dbb4 100644 > --- a/aio-posix.c > +++ b/aio-posix.c > @@ -264,5 +264,6 @@ bool aio_poll(AioContext *ctx, bool blocking) > } > } > > - return progress; > + assert(progress || busy); > + return true; > } > diff --git a/include/block/aio.h b/include/block/aio.h > index 0933f05..8eda924 100644 > --- a/include/block/aio.h > +++ b/include/block/aio.h > @@ -173,16 +173,14 @@ bool aio_pending(AioContext *ctx); > * aio as a result of executing I/O completion or bh callbacks. > * > * If there is no pending AIO operation or completion (bottom half), > - * return false. If there are pending bottom halves, return true. > + * return false. If there are pending AIO operations of bottom halves, > + * return true. > * > * If there are no pending bottom halves, but there are pending AIO > * operations, it may not be possible to make any progress without > * blocking. If @blocking is true, this function will wait until one > * or more AIO events have completed, to ensure something has moved > * before returning. > - * > - * If @blocking is false, this function will also return false if the > - * function cannot make any progress without blocking. > */ > bool aio_poll(AioContext *ctx, bool blocking); > > diff --git a/tests/test-aio.c b/tests/test-aio.c > index e4ebef7..c173870 100644 > --- a/tests/test-aio.c > +++ b/tests/test-aio.c > @@ -315,13 +315,13 @@ static void test_wait_event_notifier_noflush(void) > event_notifier_set(&data.e); > g_assert(aio_poll(ctx, false)); > g_assert_cmpint(data.n, ==, 1); > - g_assert(!aio_poll(ctx, false)); > + g_assert(aio_poll(ctx, false)); > g_assert_cmpint(data.n, ==, 1); > > event_notifier_set(&data.e); > g_assert(aio_poll(ctx, false)); > g_assert_cmpint(data.n, ==, 2); > - g_assert(!aio_poll(ctx, false)); > + g_assert(aio_poll(ctx, false)); > g_assert_cmpint(data.n, ==, 2); > > event_notifier_set(&dummy.e); >