[Qemu-devel] [PATCH 0/2] tests: avoid aio_flush() in test cases

[Qemu-devel] [PATCH 0/2] tests: avoid aio_flush() in test cases

[Stefan Hajnoczi]

There is a patch to drop aio_flush().  Most callers shouldn't use that
interface.  It turns out that the aio and thread pool test cases *do* need
low-level flush functionality so they can test the aio code.

Convert test-aio.c and test-thread-pool.c to use replacements for
qemu_aio_flush() and aio_flush().

Stefan Hajnoczi (2):
  tests: use aio_poll() instead of aio_flush() in test-aio.c
  tests: avoid qemu_aio_flush() in test-thread-pool.c

 tests/test-aio.c         | 31 +++++++++++++++----------------
 tests/test-thread-pool.c | 20 ++++++++++++++------
 2 files changed, 29 insertions(+), 22 deletions(-)

-- 
1.8.0.1

[Qemu-devel] [PATCH 1/2] tests: use aio_poll() instead of aio_flush() in test-aio.c

[Stefan Hajnoczi]

There has been confusion between various aio wait and flush functions.
It's time to get rid of qemu_aio_flush() but in the aio test cases we
really do want this low-level functionality.

Therefore declare a local wait_for_aio() helper for the test cases.
Drop the aio_flush() test case.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-aio.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/tests/test-aio.c b/tests/test-aio.c
index f53c908..a8a4f0c 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -15,6 +15,14 @@
 
 AioContext *ctx;
 
+/* Wait until there are no more BHs or AIO requests */
+static void wait_for_aio(void)
+{
+    while (aio_poll(ctx, true)) {
+        /* Do nothing */
+    }
+}
+
 /* Simple callbacks for testing.  */
 
 typedef struct {
@@ -78,14 +86,6 @@ static void test_notify(void)
     g_assert(!aio_poll(ctx, false));
 }
 
-static void test_flush(void)
-{
-    g_assert(!aio_poll(ctx, false));
-    aio_notify(ctx);
-    aio_flush(ctx);
-    g_assert(!aio_poll(ctx, false));
-}
-
 static void test_bh_schedule(void)
 {
     BHTestData data = { .n = 0 };
@@ -116,7 +116,7 @@ static void test_bh_schedule10(void)
     g_assert(aio_poll(ctx, true));
     g_assert_cmpint(data.n, ==, 2);
 
-    aio_flush(ctx);
+    wait_for_aio();
     g_assert_cmpint(data.n, ==, 10);
 
     g_assert(!aio_poll(ctx, false));
@@ -164,7 +164,7 @@ static void test_bh_delete_from_cb(void)
     qemu_bh_schedule(data1.bh);
     g_assert_cmpint(data1.n, ==, 0);
 
-    aio_flush(ctx);
+    wait_for_aio();
     g_assert_cmpint(data1.n, ==, data1.max);
     g_assert(data1.bh == NULL);
 
@@ -200,7 +200,7 @@ static void test_bh_delete_from_cb_many(void)
     g_assert_cmpint(data4.n, ==, 1);
     g_assert(data1.bh == NULL);
 
-    aio_flush(ctx);
+    wait_for_aio();
     g_assert_cmpint(data1.n, ==, data1.max);
     g_assert_cmpint(data2.n, ==, data2.max);
     g_assert_cmpint(data3.n, ==, data3.max);
@@ -219,7 +219,7 @@ static void test_bh_flush(void)
     qemu_bh_schedule(data.bh);
     g_assert_cmpint(data.n, ==, 0);
 
-    aio_flush(ctx);
+    wait_for_aio();
     g_assert_cmpint(data.n, ==, 1);
 
     g_assert(!aio_poll(ctx, false));
@@ -281,7 +281,7 @@ static void test_flush_event_notifier(void)
     g_assert_cmpint(data.active, ==, 9);
     g_assert(aio_poll(ctx, false));
 
-    aio_flush(ctx);
+    wait_for_aio();
     g_assert_cmpint(data.n, ==, 10);
     g_assert_cmpint(data.active, ==, 0);
     g_assert(!aio_poll(ctx, false));
@@ -325,7 +325,7 @@ static void test_wait_event_notifier_noflush(void)
     g_assert_cmpint(data.n, ==, 2);
 
     event_notifier_set(&dummy.e);
-    aio_flush(ctx);
+    wait_for_aio();
     g_assert_cmpint(data.n, ==, 2);
     g_assert_cmpint(dummy.n, ==, 1);
     g_assert_cmpint(dummy.active, ==, 0);
@@ -346,7 +346,7 @@ static void test_wait_event_notifier_noflush(void)
  * - sometimes both the AioContext and the glib main loop wake
  *   themselves up.  Hence, some "g_assert(!aio_poll(ctx, false));"
  *   are replaced by "while (g_main_context_iteration(NULL, false));".
- * - there is no exact replacement for aio_flush's blocking wait.
+ * - there is no exact replacement for a blocking wait.
  *   "while (g_main_context_iteration(NULL, true)" seems to work,
  *   but it is not documented _why_ it works.  For these tests a
  *   non-blocking loop like "while (g_main_context_iteration(NULL, false)"
@@ -637,7 +637,6 @@ int main(int argc, char **argv)
 
     g_test_init(&argc, &argv, NULL);
     g_test_add_func("/aio/notify",                  test_notify);
-    g_test_add_func("/aio/flush",                   test_flush);
     g_test_add_func("/aio/bh/schedule",             test_bh_schedule);
     g_test_add_func("/aio/bh/schedule10",           test_bh_schedule10);
     g_test_add_func("/aio/bh/cancel",               test_bh_cancel);
-- 
1.8.0.1

[Qemu-devel] [PATCH 2/2] tests: avoid qemu_aio_flush() in test-thread-pool.c

[Stefan Hajnoczi]

We need to eliminate calls to qemu_aio_flush() since the function is
being removed.  Most callers will use bdrv_drain_all() instead but
test-thread-pool.c is lower level.

Since the test uses the global AioContext we can loop on qemu_aio_wait()
to wait for aio and bh activity to complete.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-thread-pool.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index fea0445..ea8e676 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -47,11 +47,19 @@ static void qemu_aio_wait_nonblocking(void)
     qemu_aio_wait();
 }
 
+/* Wait until all aio and bh activity has finished */
+static void qemu_aio_wait_all(void)
+{
+    while (qemu_aio_wait()) {
+        /* Do nothing */
+    }
+}
+
 static void test_submit(void)
 {
     WorkerTestData data = { .n = 0 };
     thread_pool_submit(worker_cb, &data);
-    qemu_aio_flush();
+    qemu_aio_wait_all();
     g_assert_cmpint(data.n, ==, 1);
 }
 
@@ -63,7 +71,7 @@ static void test_submit_aio(void)
     /* The callbacks are not called until after the first wait.  */
     active = 1;
     g_assert_cmpint(data.ret, ==, -EINPROGRESS);
-    qemu_aio_flush();
+    qemu_aio_wait_all();
     g_assert_cmpint(active, ==, 0);
     g_assert_cmpint(data.n, ==, 1);
     g_assert_cmpint(data.ret, ==, 0);
@@ -84,7 +92,7 @@ static void co_test_cb(void *opaque)
     data->ret = 0;
     active--;
 
-    /* The test continues in test_submit_co, after qemu_aio_flush... */
+    /* The test continues in test_submit_co, after qemu_aio_wait_all... */
 }
 
 static void test_submit_co(void)
@@ -99,9 +107,9 @@ static void test_submit_co(void)
     g_assert_cmpint(active, ==, 1);
     g_assert_cmpint(data.ret, ==, -EINPROGRESS);
 
-    /* qemu_aio_flush will execute the rest of the coroutine.  */
+    /* qemu_aio_wait_all will execute the rest of the coroutine.  */
 
-    qemu_aio_flush();
+    qemu_aio_wait_all();
 
     /* Back here after the coroutine has finished.  */
 
@@ -184,7 +192,7 @@ static void test_cancel(void)
     }
 
     /* Finish execution and execute any remaining callbacks.  */
-    qemu_aio_flush();
+    qemu_aio_wait_all();
     g_assert_cmpint(active, ==, 0);
     for (i = 0; i < 100; i++) {
         if (data[i].n == 3) {
-- 
1.8.0.1

Re: [Qemu-devel] [PATCH 0/2] tests: avoid aio_flush() in test cases

[Paolo Bonzini]

Il 04/12/2012 16:12, Stefan Hajnoczi ha scritto:
> There is a patch to drop aio_flush().  Most callers shouldn't use that
> interface.  It turns out that the aio and thread pool test cases *do* need
> low-level flush functionality so they can test the aio code.
> 
> Convert test-aio.c and test-thread-pool.c to use replacements for
> qemu_aio_flush() and aio_flush().
> 
> Stefan Hajnoczi (2):
>   tests: use aio_poll() instead of aio_flush() in test-aio.c
>   tests: avoid qemu_aio_flush() in test-thread-pool.c
> 
>  tests/test-aio.c         | 31 +++++++++++++++----------------
>  tests/test-thread-pool.c | 20 ++++++++++++++------
>  2 files changed, 29 insertions(+), 22 deletions(-)
> 

Looks good.

Paolo

Re: [Qemu-devel] [PATCH 0/2] tests: avoid aio_flush() in test cases

[Kevin Wolf]

Am 04.12.2012 17:59, schrieb Paolo Bonzini:
> Il 04/12/2012 16:12, Stefan Hajnoczi ha scritto:
>> There is a patch to drop aio_flush().  Most callers shouldn't use that
>> interface.  It turns out that the aio and thread pool test cases *do* need
>> low-level flush functionality so they can test the aio code.
>>
>> Convert test-aio.c and test-thread-pool.c to use replacements for
>> qemu_aio_flush() and aio_flush().
>>
>> Stefan Hajnoczi (2):
>>   tests: use aio_poll() instead of aio_flush() in test-aio.c
>>   tests: avoid qemu_aio_flush() in test-thread-pool.c
>>
>>  tests/test-aio.c         | 31 +++++++++++++++----------------
>>  tests/test-thread-pool.c | 20 ++++++++++++++------
>>  2 files changed, 29 insertions(+), 22 deletions(-)
>>
> 
> Looks good.

Thanks, applied to the block branch.

Kevin