[Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Chrysostomos Nanakos]

v1->v2
------

* aio_context_new() returns NULL if the initialization of event notifier fails.
* Add descriptive error messages if aio_context_new() and event_notifier_init()
  fail.
* Fix gpollfds leak.

Chrysostomos Nanakos (1):
  async: aio_context_new(): Handle event_notifier_init failure

 async.c                  |   17 ++++++++++++-----
 include/block/aio.h      |    2 +-
 include/qemu/main-loop.h |    2 +-
 iothread.c               |   10 +++++++++-
 main-loop.c              |    9 +++++++--
 qemu-img.c               |   11 ++++++++++-
 qemu-io.c                |   10 +++++++++-
 qemu-nbd.c               |   12 +++++++++---
 tests/test-aio.c         |   10 +++++++++-
 tests/test-thread-pool.c |   10 +++++++++-
 tests/test-throttle.c    |   10 +++++++++-
 vl.c                     |    7 +++++--
 12 files changed, 90 insertions(+), 20 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Chrysostomos Nanakos]

If event_notifier_init fails QEMU exits without printing
any error information to the user. This commit adds an error
message on failure:

 # qemu [...]
 qemu: Failed to initialize event notifier: Too many open files in system
 qemu: qemu_init_main_loop failed

Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
---
 async.c                  |   17 ++++++++++++-----
 include/block/aio.h      |    2 +-
 include/qemu/main-loop.h |    2 +-
 iothread.c               |   10 +++++++++-
 main-loop.c              |    9 +++++++--
 qemu-img.c               |   11 ++++++++++-
 qemu-io.c                |   10 +++++++++-
 qemu-nbd.c               |   12 +++++++++---
 tests/test-aio.c         |   10 +++++++++-
 tests/test-thread-pool.c |   10 +++++++++-
 tests/test-throttle.c    |   10 +++++++++-
 vl.c                     |    7 +++++--
 12 files changed, 90 insertions(+), 20 deletions(-)

diff --git a/async.c b/async.c
index a99e7f6..1839754 100644
--- a/async.c
+++ b/async.c
@@ -289,18 +289,25 @@ static void aio_rfifolock_cb(void *opaque)
     aio_notify(opaque);
 }
 
-AioContext *aio_context_new(void)
+AioContext *aio_context_new(Error **errp)
 {
+    int ret;
     AioContext *ctx;
     ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
+    ret = event_notifier_init(&ctx->notifier, false);
+    if (ret < 0) {
+        g_source_destroy(&ctx->source);
+        error_setg_errno(errp, -ret, "Failed to initialize event notifier");
+        errno = -ret;
+        return NULL;
+    }
+    aio_set_event_notifier(ctx, &ctx->notifier,
+                           (EventNotifierHandler *)
+                           event_notifier_test_and_clear);
     ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
     ctx->thread_pool = NULL;
     qemu_mutex_init(&ctx->bh_lock);
     rfifolock_init(&ctx->lock, aio_rfifolock_cb, ctx);
-    event_notifier_init(&ctx->notifier, false);
-    aio_set_event_notifier(ctx, &ctx->notifier, 
-                           (EventNotifierHandler *)
-                           event_notifier_test_and_clear);
     timerlistgroup_init(&ctx->tlg, aio_timerlist_notify, ctx);
 
     return ctx;
diff --git a/include/block/aio.h b/include/block/aio.h
index 4603c0f..f3d5517 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -99,7 +99,7 @@ void aio_set_dispatching(AioContext *ctx, bool dispatching);
  * They also provide bottom halves, a service to execute a piece of code
  * as soon as possible.
  */
-AioContext *aio_context_new(void);
+AioContext *aio_context_new(Error **errp);
 
 /**
  * aio_context_ref:
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 6f0200a..62c68c0 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -42,7 +42,7 @@
  *
  * In the case of QEMU tools, this will also start/initialize timers.
  */
-int qemu_init_main_loop(void);
+int qemu_init_main_loop(Error **errp);
 
 /**
  * main_loop_wait: Run one iteration of the main loop.
diff --git a/iothread.c b/iothread.c
index d9403cf..bd96ae7 100644
--- a/iothread.c
+++ b/iothread.c
@@ -17,6 +17,7 @@
 #include "block/aio.h"
 #include "sysemu/iothread.h"
 #include "qmp-commands.h"
+#include "qemu/error-report.h"
 
 #define IOTHREADS_PATH "/objects"
 
@@ -63,10 +64,17 @@ static void iothread_instance_finalize(Object *obj)
 
 static void iothread_complete(UserCreatable *obj, Error **errp)
 {
+    Error *local_error = NULL;
     IOThread *iothread = IOTHREAD(obj);
 
     iothread->stopping = false;
-    iothread->ctx = aio_context_new();
+    iothread->ctx = aio_context_new(&local_error);
+    if (!iothread->ctx) {
+        error_report("%s", error_get_pretty(local_error));
+        error_report("Failed to create AIO context");
+        error_free(local_error);
+        exit(1);
+    }
     iothread->thread_id = -1;
 
     qemu_mutex_init(&iothread->init_done_lock);
diff --git a/main-loop.c b/main-loop.c
index 3cc79f8..40c0afa 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -126,10 +126,11 @@ void qemu_notify_event(void)
 
 static GArray *gpollfds;
 
-int qemu_init_main_loop(void)
+int qemu_init_main_loop(Error **errp)
 {
     int ret;
     GSource *src;
+    Error *local_error = NULL;
 
     init_clocks();
 
@@ -138,8 +139,12 @@ int qemu_init_main_loop(void)
         return ret;
     }
 
+    qemu_aio_context = aio_context_new(&local_error);
+    if (!qemu_aio_context) {
+        error_propagate(errp, local_error);
+        return -errno;
+    }
     gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
-    qemu_aio_context = aio_context_new();
     src = aio_get_g_source(qemu_aio_context);
     g_source_attach(src, NULL);
     g_source_unref(src);
diff --git a/qemu-img.c b/qemu-img.c
index 91d1ac3..6b15bbf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2879,7 +2879,9 @@ int main(int argc, char **argv)
 {
     const img_cmd_t *cmd;
     const char *cmdname;
+    Error *local_error = NULL;
     int c;
+    int ret;
     static const struct option long_options[] = {
         {"help", no_argument, 0, 'h'},
         {"version", no_argument, 0, 'v'},
@@ -2893,7 +2895,14 @@ int main(int argc, char **argv)
     error_set_progname(argv[0]);
     qemu_init_exec_dir(argv[0]);
 
-    qemu_init_main_loop();
+    ret = qemu_init_main_loop(&local_error);
+    if (ret < 0) {
+        error_report("%s", error_get_pretty(local_error));
+        error_report("qemu_init_main_loop failed");
+        error_free(local_error);
+        exit(EXIT_FAILURE);
+    }
+
     bdrv_init();
     if (argc < 2) {
         error_exit("Not enough arguments");
diff --git a/qemu-io.c b/qemu-io.c
index d2ab694..e632d59 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -377,8 +377,10 @@ int main(int argc, char **argv)
         { NULL, 0, NULL, 0 }
     };
     int c;
+    int ret;
     int opt_index = 0;
     int flags = BDRV_O_UNMAP;
+    Error *local_error = NULL;
 
 #ifdef CONFIG_POSIX
     signal(SIGPIPE, SIG_IGN);
@@ -444,7 +446,13 @@ int main(int argc, char **argv)
         exit(1);
     }
 
-    qemu_init_main_loop();
+    ret = qemu_init_main_loop(&local_error);
+    if (ret < 0) {
+        error_report("%s", error_get_pretty(local_error));
+        error_report("qemu_init_main_loop failed");
+        error_free(local_error);
+        return ret;
+    }
     bdrv_init();
 
     /* initialize commands */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 9bc152e..ed206ed 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -498,13 +498,13 @@ int main(int argc, char **argv)
                                 BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF,
                                 &local_err);
             if (local_err) {
-                errx(EXIT_FAILURE, "Failed to parse detect_zeroes mode: %s", 
+                errx(EXIT_FAILURE, "Failed to parse detect_zeroes mode: %s",
                      error_get_pretty(local_err));
             }
             if (detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
                 !(flags & BDRV_O_UNMAP)) {
                 errx(EXIT_FAILURE, "setting detect-zeroes to unmap is not allowed "
-                                   "without setting discard operation to unmap"); 
+                                   "without setting discard operation to unmap");
             }
             break;
         case 'b':
@@ -674,7 +674,13 @@ int main(int argc, char **argv)
         snprintf(sockpath, 128, SOCKET_PATH, basename(device));
     }
 
-    qemu_init_main_loop();
+    ret = qemu_init_main_loop(&local_err);
+    if (ret < 0) {
+        error_report("%s", error_get_pretty(local_err));
+        error_report("qemu_init_main_loop failed");
+        error_free(local_err);
+        exit(EXIT_FAILURE);
+    }
     bdrv_init();
     atexit(bdrv_close_all);
 
diff --git a/tests/test-aio.c b/tests/test-aio.c
index c6a8713..4cb3783 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -14,6 +14,7 @@
 #include "block/aio.h"
 #include "qemu/timer.h"
 #include "qemu/sockets.h"
+#include "qemu/error-report.h"
 
 static AioContext *ctx;
 
@@ -810,11 +811,18 @@ static void test_source_timer_schedule(void)
 
 int main(int argc, char **argv)
 {
+    Error *local_error;
     GSource *src;
 
     init_clocks();
 
-    ctx = aio_context_new();
+    ctx = aio_context_new(&local_error);
+    if (!ctx) {
+        error_report("%s", error_get_pretty(local_error));
+        error_report("Failed to create AIO context");
+        error_free(local_error);
+        exit(1);
+    }
     src = aio_get_g_source(ctx);
     g_source_attach(src, NULL);
     g_source_unref(src);
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index f40b7fc..ff6a8b0 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -4,6 +4,7 @@
 #include "block/thread-pool.h"
 #include "block/block.h"
 #include "qemu/timer.h"
+#include "qemu/error-report.h"
 
 static AioContext *ctx;
 static ThreadPool *pool;
@@ -205,10 +206,17 @@ static void test_cancel(void)
 int main(int argc, char **argv)
 {
     int ret;
+    Error *local_error;
 
     init_clocks();
 
-    ctx = aio_context_new();
+    ctx = aio_context_new(&local_error);
+    if (!ctx) {
+        error_report("%s", error_get_pretty(local_error));
+        error_report("Failed to create AIO context");
+        error_free(local_error);
+        exit(1);
+    }
     pool = aio_get_thread_pool(ctx);
 
     g_test_init(&argc, &argv, NULL);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 000ae31..7be607a 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -14,6 +14,7 @@
 #include <math.h>
 #include "block/aio.h"
 #include "qemu/throttle.h"
+#include "qemu/error-report.h"
 
 static AioContext     *ctx;
 static LeakyBucket    bkt;
@@ -492,10 +493,17 @@ static void test_accounting(void)
 int main(int argc, char **argv)
 {
     GSource *src;
+    Error *local_error;
 
     init_clocks();
 
-    ctx = aio_context_new();
+    ctx = aio_context_new(&local_error);
+    if (!ctx) {
+        error_report("%s", error_get_pretty(local_error));
+        error_report("Failed to create AIO context");
+        error_free(local_error);
+        exit(1);
+    }
     src = aio_get_g_source(ctx);
     g_source_attach(src, NULL);
     g_source_unref(src);
diff --git a/vl.c b/vl.c
index 5db0d08..1340f6f 100644
--- a/vl.c
+++ b/vl.c
@@ -2968,6 +2968,7 @@ int main(int argc, char **argv, char **envp)
     ram_addr_t maxram_size = default_ram_size;
     uint64_t ram_slots = 0;
     FILE *vmstate_dump_file = NULL;
+    Error *main_loop_err = NULL;
 
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
@@ -3998,8 +3999,10 @@ int main(int argc, char **argv, char **envp)
 
     os_daemonize();
 
-    if (qemu_init_main_loop()) {
-        fprintf(stderr, "qemu_init_main_loop failed\n");
+    if (qemu_init_main_loop(&main_loop_err)) {
+        error_report("%s", error_get_pretty(main_loop_err));
+        error_report("qemu_init_main_loop failed");
+        error_free(main_loop_err);
         exit(1);
     }
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Benoît Canet]

>  static void iothread_complete(UserCreatable *obj, Error **errp)
>  {
> +    Error *local_error = NULL;
>      IOThread *iothread = IOTHREAD(obj);
>  
>      iothread->stopping = false;
> -    iothread->ctx = aio_context_new();
> +    iothread->ctx = aio_context_new(&local_error);
> +    if (!iothread->ctx) {
> +        error_report("%s", error_get_pretty(local_error));
> +        error_report("Failed to create AIO context");

I think reporting one line is sufficient.

You could do something in the vein of.
error_report("Failed to create AIO Context: \'%s\'", error_get_pretty(local_error));


> +        exit(1);
Also here I don't know if exiting in the middle of a class initialization
completion function is good taste.
You should ask to someone knowing QOM.

>      signal(SIGPIPE, SIG_IGN);
> @@ -444,7 +446,13 @@ int main(int argc, char **argv)
>          exit(1);
>      }
>  
> -    qemu_init_main_loop();
> +    ret = qemu_init_main_loop(&local_error);
> +    if (ret < 0) {
> +        error_report("%s", error_get_pretty(local_error));
> +        error_report("qemu_init_main_loop failed");
> +        error_free(local_error);

> +        return ret;

We are in main here.
Should it be exit(something) like above ?


>              if (local_err) {
> -                errx(EXIT_FAILURE, "Failed to parse detect_zeroes mode: %s", 
> +                errx(EXIT_FAILURE, "Failed to parse detect_zeroes mode: %s",
>                       error_get_pretty(local_err));
>              }
>              if (detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP &&
>                  !(flags & BDRV_O_UNMAP)) {
>                  errx(EXIT_FAILURE, "setting detect-zeroes to unmap is not allowed "
> -                                   "without setting discard operation to unmap"); 
> +                                   "without setting discard operation to unmap");

No cosmetic fixes on parts of the code unmodified by a patch please.
Your editor is probably removing these extra EOL spaces on your behalf. Fix it.

>              }
>              break;
>          case 'b':
> @@ -674,7 +674,13 @@ int main(int argc, char **argv)
>          snprintf(sockpath, 128, SOCKET_PATH, basename(device));
>      }
>  
> -    qemu_init_main_loop();
> +    ret = qemu_init_main_loop(&local_err);
> +    if (ret < 0) {
> +        error_report("%s", error_get_pretty(local_err));
> +        error_report("qemu_init_main_loop failed");
> +        error_free(local_err);
> +        exit(EXIT_FAILURE);
> +    }
>      bdrv_init();
>      atexit(bdrv_close_all);
>  
> diff --git a/tests/test-aio.c b/tests/test-aio.c
> index c6a8713..4cb3783 100644
> --- a/tests/test-aio.c
> +++ b/tests/test-aio.c
> @@ -14,6 +14,7 @@
>  #include "block/aio.h"
>  #include "qemu/timer.h"
>  #include "qemu/sockets.h"
> +#include "qemu/error-report.h"
>  
>  static AioContext *ctx;
>  
> @@ -810,11 +811,18 @@ static void test_source_timer_schedule(void)
>  
>  int main(int argc, char **argv)
>  {
> +    Error *local_error;
>      GSource *src;
>  
>      init_clocks();
>  
> -    ctx = aio_context_new();
> +    ctx = aio_context_new(&local_error);
> +    if (!ctx) {
> +        error_report("%s", error_get_pretty(local_error));
> +        error_report("Failed to create AIO context");
> +        error_free(local_error);
> +        exit(1);
> +    }
>      src = aio_get_g_source(ctx);
>      g_source_attach(src, NULL);
>      g_source_unref(src);
> diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
> index f40b7fc..ff6a8b0 100644
> --- a/tests/test-thread-pool.c
> +++ b/tests/test-thread-pool.c
> @@ -4,6 +4,7 @@
>  #include "block/thread-pool.h"
>  #include "block/block.h"
>  #include "qemu/timer.h"
> +#include "qemu/error-report.h"
>  
>  static AioContext *ctx;
>  static ThreadPool *pool;
> @@ -205,10 +206,17 @@ static void test_cancel(void)
>  int main(int argc, char **argv)
>  {
>      int ret;
> +    Error *local_error;
>  
>      init_clocks();
>  
> -    ctx = aio_context_new();
> +    ctx = aio_context_new(&local_error);
> +    if (!ctx) {
> +        error_report("%s", error_get_pretty(local_error));
> +        error_report("Failed to create AIO context");
> +        error_free(local_error);
> +        exit(1);
> +    }
>      pool = aio_get_thread_pool(ctx);
>  
>      g_test_init(&argc, &argv, NULL);
> diff --git a/tests/test-throttle.c b/tests/test-throttle.c
> index 000ae31..7be607a 100644
> --- a/tests/test-throttle.c
> +++ b/tests/test-throttle.c
> @@ -14,6 +14,7 @@
>  #include <math.h>
>  #include "block/aio.h"
>  #include "qemu/throttle.h"
> +#include "qemu/error-report.h"
>  
>  static AioContext     *ctx;
>  static LeakyBucket    bkt;
> @@ -492,10 +493,17 @@ static void test_accounting(void)
>  int main(int argc, char **argv)
>  {
>      GSource *src;
> +    Error *local_error;
>  
>      init_clocks();
>  
> -    ctx = aio_context_new();
> +    ctx = aio_context_new(&local_error);
> +    if (!ctx) {
> +        error_report("%s", error_get_pretty(local_error));
> +        error_report("Failed to create AIO context");
> +        error_free(local_error);
> +        exit(1);
> +    }
>      src = aio_get_g_source(ctx);
>      g_source_attach(src, NULL);
>      g_source_unref(src);
> diff --git a/vl.c b/vl.c
> index 5db0d08..1340f6f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2968,6 +2968,7 @@ int main(int argc, char **argv, char **envp)
>      ram_addr_t maxram_size = default_ram_size;
>      uint64_t ram_slots = 0;
>      FILE *vmstate_dump_file = NULL;
> +    Error *main_loop_err = NULL;
>  
>      atexit(qemu_run_exit_notifiers);
>      error_set_progname(argv[0]);
> @@ -3998,8 +3999,10 @@ int main(int argc, char **argv, char **envp)
>  
>      os_daemonize();
>  
> -    if (qemu_init_main_loop()) {
> -        fprintf(stderr, "qemu_init_main_loop failed\n");
> +    if (qemu_init_main_loop(&main_loop_err)) {
> +        error_report("%s", error_get_pretty(main_loop_err));
> +        error_report("qemu_init_main_loop failed");
> +        error_free(main_loop_err);
>          exit(1);
>      }
>  
> -- 
> 1.7.10.4
> 
> 

Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Paolo Bonzini]

Il 16/09/2014 15:53, Benoît Canet ha scritto:
>>  static void iothread_complete(UserCreatable *obj, Error **errp)
>> >  {
>> > +    Error *local_error = NULL;
>> >      IOThread *iothread = IOTHREAD(obj);
>> >  
>> >      iothread->stopping = false;
>> > -    iothread->ctx = aio_context_new();
>> > +    iothread->ctx = aio_context_new(&local_error);
>> > +    if (!iothread->ctx) {
>> > +        error_report("%s", error_get_pretty(local_error));
>> > +        error_report("Failed to create AIO context");
> I think reporting one line is sufficient.
> 
> You could do something in the vein of.
> error_report("Failed to create AIO Context: \'%s\'", error_get_pretty(local_error));
> 
> 
>> > +        exit(1);
> Also here I don't know if exiting in the middle of a class initialization
> completion function is good taste.
> You should ask to someone knowing QOM.
> 

Indeed, the right thing to do is simply

    error_propagate(errp, local_error);
    return;

Paolo

Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Chrysostomos Nanakos]

On Tue, Sep 16, 2014 at 03:57:25PM +0200, Paolo Bonzini wrote:
> Il 16/09/2014 15:53, Benoît Canet ha scritto:
> >>  static void iothread_complete(UserCreatable *obj, Error **errp)
> >> >  {
> >> > +    Error *local_error = NULL;
> >> >      IOThread *iothread = IOTHREAD(obj);
> >> >  
> >> >      iothread->stopping = false;
> >> > -    iothread->ctx = aio_context_new();
> >> > +    iothread->ctx = aio_context_new(&local_error);
> >> > +    if (!iothread->ctx) {
> >> > +        error_report("%s", error_get_pretty(local_error));
> >> > +        error_report("Failed to create AIO context");
> > I think reporting one line is sufficient.
> > 
> > You could do something in the vein of.
> > error_report("Failed to create AIO Context: \'%s\'", error_get_pretty(local_error));
> > 
> > 
> >> > +        exit(1);
> > Also here I don't know if exiting in the middle of a class initialization
> > completion function is good taste.
> > You should ask to someone knowing QOM.
> > 
> 
> Indeed, the right thing to do is simply
> 
>     error_propagate(errp, local_error);
>     return;
Just to note that after propagating the error and returning, QEMU fails
silently without printing the error message.

I will make the proposed changes and try to resolve this in the next patch
series if this is possible.


Regards,
Chrysostomos.

Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Paolo Bonzini]

Il 16/09/2014 17:43, Chrysostomos Nanakos ha scritto:
>> >     error_propagate(errp, local_error);
>> >     return;
> Just to note that after propagating the error and returning, QEMU fails
> silently without printing the error message.

What is your testcase?

Paolo

Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Chrysostomos Nanakos]

On Tue, Sep 16, 2014 at 05:45:16PM +0200, Paolo Bonzini wrote:
> Il 16/09/2014 17:43, Chrysostomos Nanakos ha scritto:
> >> >     error_propagate(errp, local_error);
> >> >     return;
> > Just to note that after propagating the error and returning, QEMU fails
> > silently without printing the error message.
> 
> What is your testcase?

I am starting QEMU with the options below and explicitly set iothread->ctx to
NULL. Is that ok as a testcase or should I reduce my open files limit to
produce the error?

qemu --enable-kvm -smp 2 -m 1024 -object iothread,id=iothread0 -drive file=archipelago:fedora_stable
-vnc 0.0.0.0:0 -qmp tcp:127.0.0.1:8888,server,nowait

No error message, at least the propagated one, fails silently.

Regards,
Chrysostomos.

Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure

[Chrysostomos Nanakos]

On Tue, Sep 16, 2014 at 06:50:38PM +0300, Chrysostomos Nanakos wrote:
> On Tue, Sep 16, 2014 at 05:45:16PM +0200, Paolo Bonzini wrote:
> > Il 16/09/2014 17:43, Chrysostomos Nanakos ha scritto:
> > >> >     error_propagate(errp, local_error);
> > >> >     return;
> > > Just to note that after propagating the error and returning, QEMU fails
> > > silently without printing the error message.
> > 
> > What is your testcase?
> 
> I am starting QEMU with the options below and explicitly set iothread->ctx to
> NULL. Is that ok as a testcase or should I reduce my open files limit to
> produce the error?
> 
> qemu --enable-kvm -smp 2 -m 1024 -object iothread,id=iothread0 -drive file=archipelago:fedora_stable
> -vnc 0.0.0.0:0 -qmp tcp:127.0.0.1:8888,server,nowait
> 
> No error message, at least the propagated one, fails silently.
> 
The segfault is caused when QOM tries to object_unref() the iothread object,
type->instance_finalize(obj) is called which calls the registered
iothread_instance_finalize function. A check there for the iothread->ctx
seems to solve the problem.

I will include the fix in the next patch series.

Regards,
Chrysostomos.