Re: [Qemu-devel] [PATCH] aio-posix: Fix return value of aio_poll()

[Paolo Bonzini <pbonzini@redhat.com>]

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 <kwolf@redhat.com>

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);
>