* [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure @ 2014-09-16 9:06 Chrysostomos Nanakos 2014-09-16 9:06 ` Chrysostomos Nanakos 0 siblings, 1 reply; 8+ messages in thread From: Chrysostomos Nanakos @ 2014-09-16 9:06 UTC (permalink / raw) To: qemu-devel Cc: kwolf, Chrysostomos Nanakos, pingfank, famz, benoit, jan.kiszka, stefanha, mjt, kroosec, sw, pbonzini, afaerber, aliguori 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure 2014-09-16 9:06 [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure Chrysostomos Nanakos @ 2014-09-16 9:06 ` Chrysostomos Nanakos 2014-09-16 13:53 ` Benoît Canet 0 siblings, 1 reply; 8+ messages in thread From: Chrysostomos Nanakos @ 2014-09-16 9:06 UTC (permalink / raw) To: qemu-devel Cc: kwolf, Chrysostomos Nanakos, pingfank, famz, benoit, jan.kiszka, stefanha, mjt, kroosec, sw, pbonzini, afaerber, aliguori 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure 2014-09-16 9:06 ` Chrysostomos Nanakos @ 2014-09-16 13:53 ` Benoît Canet 2014-09-16 13:57 ` Paolo Bonzini 0 siblings, 1 reply; 8+ messages in thread From: Benoît Canet @ 2014-09-16 13:53 UTC (permalink / raw) To: Chrysostomos Nanakos Cc: kwolf, pingfank, famz, kroosec, jan.kiszka, mjt, qemu-devel, stefanha, sw, pbonzini, afaerber, aliguori > 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure 2014-09-16 13:53 ` Benoît Canet @ 2014-09-16 13:57 ` Paolo Bonzini 2014-09-16 15:43 ` Chrysostomos Nanakos 0 siblings, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2014-09-16 13:57 UTC (permalink / raw) To: Benoît Canet, Chrysostomos Nanakos Cc: kwolf, pingfank, famz, kroosec, jan.kiszka, mjt, qemu-devel, stefanha, sw, afaerber, aliguori 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure 2014-09-16 13:57 ` Paolo Bonzini @ 2014-09-16 15:43 ` Chrysostomos Nanakos 2014-09-16 15:45 ` Paolo Bonzini 0 siblings, 1 reply; 8+ messages in thread From: Chrysostomos Nanakos @ 2014-09-16 15:43 UTC (permalink / raw) To: Paolo Bonzini Cc: Benoît Canet, Chrysostomos Nanakos, pingfank, famz, kroosec, jan.kiszka, mjt, qemu-devel, stefanha, sw, kwolf, afaerber, aliguori 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure 2014-09-16 15:43 ` Chrysostomos Nanakos @ 2014-09-16 15:45 ` Paolo Bonzini 2014-09-16 15:50 ` Chrysostomos Nanakos 0 siblings, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2014-09-16 15:45 UTC (permalink / raw) To: Chrysostomos Nanakos Cc: Benoît Canet, kwolf, pingfank, famz, kroosec, jan.kiszka, mjt, qemu-devel, stefanha, sw, afaerber, aliguori 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure 2014-09-16 15:45 ` Paolo Bonzini @ 2014-09-16 15:50 ` Chrysostomos Nanakos 2014-09-16 17:58 ` Chrysostomos Nanakos 0 siblings, 1 reply; 8+ messages in thread From: Chrysostomos Nanakos @ 2014-09-16 15:50 UTC (permalink / raw) To: Paolo Bonzini Cc: Chrysostomos Nanakos, Benoît Canet, pingfank, famz, kroosec, jan.kiszka, mjt, qemu-devel, stefanha, sw, kwolf, afaerber, aliguori 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure 2014-09-16 15:50 ` Chrysostomos Nanakos @ 2014-09-16 17:58 ` Chrysostomos Nanakos 0 siblings, 0 replies; 8+ messages in thread From: Chrysostomos Nanakos @ 2014-09-16 17:58 UTC (permalink / raw) To: Chrysostomos Nanakos Cc: Benoît Canet, kwolf, pingfank, famz, kroosec, jan.kiszka, mjt, qemu-devel, stefanha, sw, Paolo Bonzini, afaerber, aliguori 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-09-16 17:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-16 9:06 [Qemu-devel] [PATCH v2] async: aio_context_new(): Handle event_notifier_init failure Chrysostomos Nanakos 2014-09-16 9:06 ` Chrysostomos Nanakos 2014-09-16 13:53 ` Benoît Canet 2014-09-16 13:57 ` Paolo Bonzini 2014-09-16 15:43 ` Chrysostomos Nanakos 2014-09-16 15:45 ` Paolo Bonzini 2014-09-16 15:50 ` Chrysostomos Nanakos 2014-09-16 17:58 ` Chrysostomos Nanakos
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).