On 09/16/2014 12:04 PM, Chrysostomos Nanakos wrote: > If event_notifier_init fails QEMU exits without printing > any error information to the user. This commit adds an error > message on failure: > > # qemu [...]
Showing the actual command line you used would be helpful.
> qemu: Failed to initialize event notifier: Too many open files in system
>
> Signed-off-by: Chrysostomos Nanakos <cnana...@grnet.gr>
> ---
> async.c | 16 +++++++++++-----
> include/block/aio.h | 2 +-
> include/qemu/main-loop.h | 2 +-
> iothread.c | 11 ++++++++++-
> main-loop.c | 9 +++++++--
> qemu-img.c | 8 +++++++-
> qemu-io.c | 7 ++++++-
> qemu-nbd.c | 6 +++++-
> tests/test-aio.c | 10 +++++++++-
> tests/test-thread-pool.c | 10 +++++++++-
> tests/test-throttle.c | 10 +++++++++-
> vl.c | 5 +++--
> 12 files changed, 78 insertions(+), 18 deletions(-)
>
> -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);
Does g_source_destroy() guarantee that errno is unmolested? If not,
> + error_setg_errno(errp, -ret, "Failed to initialize event notifier");
then this logs the wrong error. Swap the lines to be safe.
> + 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);
Is hoisting the event notifier init to occur before the mutex init going
to cause any grief?
> +++ b/main-loop.c
> @@ -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 -1;
> + }
Can the earlier call to qemu_signal_init() consume any resources that
are now leaked?
> @@ -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("Failed to create AIO Context: \'%s\'",
Use of \' inside "" is unusual. (Multiple times in your patch)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
