Patchwork [v4,03/17] tests: adjust test-aio to new aio_poll() semantics

login
register
mail settings
Submitter Stefan Hajnoczi
Date June 14, 2013, 11:43 a.m.
Message ID <1371210243-6099-4-git-send-email-stefanha@redhat.com>
Download mbox | patch
Permalink /patch/251403/
State New
Headers show

Comments

Stefan Hajnoczi - June 14, 2013, 11:43 a.m.
aio_poll(ctx, true) will soon block if any fd handlers have been set.
Previously it would only block when .io_flush() returned true.

This means that callers must check their wait condition *before*
aio_poll() to avoid deadlock.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/test-aio.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)
Paolo Bonzini - June 27, 2013, 1:15 p.m.
Il 14/06/2013 13:43, Stefan Hajnoczi ha scritto:
> aio_poll(ctx, true) will soon block if any fd handlers have been set.
> Previously it would only block when .io_flush() returned true.
> 
> This means that callers must check their wait condition *before*
> aio_poll() to avoid deadlock.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/test-aio.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/test-aio.c b/tests/test-aio.c
> index c173870..20bf5e6 100644
> --- a/tests/test-aio.c
> +++ b/tests/test-aio.c
> @@ -15,6 +15,13 @@
>  
>  AioContext *ctx;
>  
> +typedef struct {
> +    EventNotifier e;
> +    int n;
> +    int active;
> +    bool auto_set;
> +} EventNotifierTestData;
> +
>  /* Wait until there are no more BHs or AIO requests */
>  static void wait_for_aio(void)
>  {
> @@ -23,6 +30,14 @@ static void wait_for_aio(void)
>      }
>  }
>  
> +/* Wait until event notifier becomes inactive */
> +static void wait_until_inactive(EventNotifierTestData *data)
> +{
> +    while (data->active > 0) {
> +        aio_poll(ctx, true);
> +    }
> +}
> +
>  /* Simple callbacks for testing.  */
>  
>  typedef struct {
> @@ -50,13 +65,6 @@ static void bh_delete_cb(void *opaque)
>      }
>  }
>  
> -typedef struct {
> -    EventNotifier e;
> -    int n;
> -    int active;
> -    bool auto_set;
> -} EventNotifierTestData;
> -
>  static int event_active_cb(EventNotifier *e)
>  {
>      EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
> @@ -281,7 +289,7 @@ static void test_flush_event_notifier(void)
>      g_assert_cmpint(data.active, ==, 9);
>      g_assert(aio_poll(ctx, false));
>  
> -    wait_for_aio();
> +    wait_until_inactive(&data);
>      g_assert_cmpint(data.n, ==, 10);
>      g_assert_cmpint(data.active, ==, 0);
>      g_assert(!aio_poll(ctx, false));
> @@ -325,7 +333,7 @@ static void test_wait_event_notifier_noflush(void)
>      g_assert_cmpint(data.n, ==, 2);
>  
>      event_notifier_set(&dummy.e);
> -    wait_for_aio();
> +    wait_until_inactive(&dummy);
>      g_assert_cmpint(data.n, ==, 2);
>      g_assert_cmpint(dummy.n, ==, 1);
>      g_assert_cmpint(dummy.active, ==, 0);
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini - June 27, 2013, 1:28 p.m.
Il 14/06/2013 13:43, Stefan Hajnoczi ha scritto:
> aio_poll(ctx, true) will soon block if any fd handlers have been set.
> Previously it would only block when .io_flush() returned true.
> 
> This means that callers must check their wait condition *before*
> aio_poll() to avoid deadlock.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/test-aio.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/test-aio.c b/tests/test-aio.c
> index c173870..20bf5e6 100644
> --- a/tests/test-aio.c
> +++ b/tests/test-aio.c
> @@ -15,6 +15,13 @@
>  
>  AioContext *ctx;
>  
> +typedef struct {
> +    EventNotifier e;
> +    int n;
> +    int active;
> +    bool auto_set;
> +} EventNotifierTestData;
> +
>  /* Wait until there are no more BHs or AIO requests */
>  static void wait_for_aio(void)
>  {
> @@ -23,6 +30,14 @@ static void wait_for_aio(void)
>      }
>  }
>  
> +/* Wait until event notifier becomes inactive */
> +static void wait_until_inactive(EventNotifierTestData *data)
> +{
> +    while (data->active > 0) {
> +        aio_poll(ctx, true);
> +    }
> +}
> +
>  /* Simple callbacks for testing.  */
>  
>  typedef struct {
> @@ -50,13 +65,6 @@ static void bh_delete_cb(void *opaque)
>      }
>  }
>  
> -typedef struct {
> -    EventNotifier e;
> -    int n;
> -    int active;
> -    bool auto_set;
> -} EventNotifierTestData;
> -
>  static int event_active_cb(EventNotifier *e)
>  {
>      EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
> @@ -281,7 +289,7 @@ static void test_flush_event_notifier(void)
>      g_assert_cmpint(data.active, ==, 9);
>      g_assert(aio_poll(ctx, false));
>  
> -    wait_for_aio();
> +    wait_until_inactive(&data);
>      g_assert_cmpint(data.n, ==, 10);
>      g_assert_cmpint(data.active, ==, 0);
>      g_assert(!aio_poll(ctx, false));
> @@ -325,7 +333,7 @@ static void test_wait_event_notifier_noflush(void)
>      g_assert_cmpint(data.n, ==, 2);
>  
>      event_notifier_set(&dummy.e);
> -    wait_for_aio();
> +    wait_until_inactive(&dummy);
>      g_assert_cmpint(data.n, ==, 2);
>      g_assert_cmpint(dummy.n, ==, 1);
>      g_assert_cmpint(dummy.active, ==, 0);
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Patch

diff --git a/tests/test-aio.c b/tests/test-aio.c
index c173870..20bf5e6 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -15,6 +15,13 @@ 
 
 AioContext *ctx;
 
+typedef struct {
+    EventNotifier e;
+    int n;
+    int active;
+    bool auto_set;
+} EventNotifierTestData;
+
 /* Wait until there are no more BHs or AIO requests */
 static void wait_for_aio(void)
 {
@@ -23,6 +30,14 @@  static void wait_for_aio(void)
     }
 }
 
+/* Wait until event notifier becomes inactive */
+static void wait_until_inactive(EventNotifierTestData *data)
+{
+    while (data->active > 0) {
+        aio_poll(ctx, true);
+    }
+}
+
 /* Simple callbacks for testing.  */
 
 typedef struct {
@@ -50,13 +65,6 @@  static void bh_delete_cb(void *opaque)
     }
 }
 
-typedef struct {
-    EventNotifier e;
-    int n;
-    int active;
-    bool auto_set;
-} EventNotifierTestData;
-
 static int event_active_cb(EventNotifier *e)
 {
     EventNotifierTestData *data = container_of(e, EventNotifierTestData, e);
@@ -281,7 +289,7 @@  static void test_flush_event_notifier(void)
     g_assert_cmpint(data.active, ==, 9);
     g_assert(aio_poll(ctx, false));
 
-    wait_for_aio();
+    wait_until_inactive(&data);
     g_assert_cmpint(data.n, ==, 10);
     g_assert_cmpint(data.active, ==, 0);
     g_assert(!aio_poll(ctx, false));
@@ -325,7 +333,7 @@  static void test_wait_event_notifier_noflush(void)
     g_assert_cmpint(data.n, ==, 2);
 
     event_notifier_set(&dummy.e);
-    wait_for_aio();
+    wait_until_inactive(&dummy);
     g_assert_cmpint(data.n, ==, 2);
     g_assert_cmpint(dummy.n, ==, 1);
     g_assert_cmpint(dummy.active, ==, 0);