From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37255) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUZEx-00052c-Fa for qemu-devel@nongnu.org; Thu, 18 Sep 2014 06:47:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XUZEn-0006OE-8X for qemu-devel@nongnu.org; Thu, 18 Sep 2014 06:47:07 -0400 Received: from mail-wg0-x233.google.com ([2a00:1450:400c:c00::233]:61969) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUZEm-0006If-Lf for qemu-devel@nongnu.org; Thu, 18 Sep 2014 06:46:57 -0400 Received: by mail-wg0-f51.google.com with SMTP id k14so675982wgh.10 for ; Thu, 18 Sep 2014 03:46:50 -0700 (PDT) Date: Thu, 18 Sep 2014 11:46:47 +0100 From: Stefan Hajnoczi Message-ID: <20140918104647.GE8847@stefanha-thinkpad.redhat.com> References: <1410961715-21347-1-git-send-email-cnanakos@grnet.gr> <1410961715-21347-2-git-send-email-cnanakos@grnet.gr> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="47eKBCiAZYFK5l32" Content-Disposition: inline In-Reply-To: <1410961715-21347-2-git-send-email-cnanakos@grnet.gr> Subject: Re: [Qemu-devel] [PATCH v4] async: aio_context_new(): Handle event_notifier_init failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chrysostomos Nanakos Cc: kwolf@redhat.com, pingfank@linux.vnet.ibm.com, famz@redhat.com, benoit@irqsave.net, jan.kiszka@siemens.com, mjt@tls.msk.ru, qemu-devel@nongnu.org, stefanha@redhat.com, sw@weilnetz.de, pbonzini@redhat.com, kroosec@gmail.com, afaerber@suse.de, aliguori@amazon.com --47eKBCiAZYFK5l32 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 17, 2014 at 04:48:34PM +0300, Chrysostomos Nanakos wrote: > diff --git a/iothread.c b/iothread.c > index d9403cf..6e394a0 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" > =20 > #define IOTHREADS_PATH "/objects" > =20 > @@ -53,6 +54,9 @@ static void iothread_instance_finalize(Object *obj) > { > IOThread *iothread =3D IOTHREAD(obj); > =20 > + if (!iothread->ctx) { > + return; > + } > iothread->stopping =3D true; > aio_notify(iothread->ctx); > qemu_thread_join(&iothread->thread); > @@ -63,10 +67,15 @@ static void iothread_instance_finalize(Object *obj) > =20 > static void iothread_complete(UserCreatable *obj, Error **errp) > { > + Error *local_error =3D NULL; > IOThread *iothread =3D IOTHREAD(obj); > =20 > iothread->stopping =3D false; > - iothread->ctx =3D aio_context_new(); > + iothread->ctx =3D aio_context_new(&local_error); > + if (!iothread->ctx) { > + error_propagate(errp, local_error); > + return; > + } > iothread->thread_id =3D -1; Please move this under ->stopping =3D false so that ->thread_id is initialized. That way qmp_query_iothreads() will display thread_id -1 for failed IOThread objects instead of an uninitialized value. Normally QEMU should exit or the IOThread object should be deleted before anyone has a chance to call qmp_query_iothreads() but you never know so let's get the lifecycle right... > diff --git a/main-loop.c b/main-loop.c > index 3cc79f8..2f83d80 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -126,10 +126,11 @@ void qemu_notify_event(void) > =20 > static GArray *gpollfds; > =20 > -int qemu_init_main_loop(void) > +int qemu_init_main_loop(Error **errp) > { > int ret; > GSource *src; > + Error *local_error =3D NULL; > =20 > init_clocks(); > =20 > @@ -138,8 +139,12 @@ int qemu_init_main_loop(void) > return ret; > } > =20 > + qemu_aio_context =3D aio_context_new(&local_error); > + if (!qemu_aio_context) { > + error_propagate(errp, local_error); > + return -1; This function returns -errno so -1 would be -EPERM. Please use an actual errno constant like -EMFILE. > diff --git a/tests/test-aio.c b/tests/test-aio.c > index c6a8713..029716f 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" > =20 > static AioContext *ctx; > =20 > @@ -810,11 +811,18 @@ static void test_source_timer_schedule(void) > =20 > int main(int argc, char **argv) > { > + Error *local_error; local_error =3D NULL It must be NULL otherwise error_setg() will read uninitialized memory (it checks that the error hasn't been set yet because Error objects can only be set once). > diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c > index f40b7fc..57c0836 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" > =20 > 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; Same. > diff --git a/tests/test-throttle.c b/tests/test-throttle.c > index 000ae31..42141a0 100644 > --- a/tests/test-throttle.c > +++ b/tests/test-throttle.c > @@ -14,6 +14,7 @@ > #include > #include "block/aio.h" > #include "qemu/throttle.h" > +#include "qemu/error-report.h" > =20 > 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; Same. --47eKBCiAZYFK5l32 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUGrgXAAoJEJykq7OBq3PIw+IIAMH4B0+gQbXeYhSNgZ8nBATN fLSOIM8zpePXgNtZOurNcWlEN/rajzgN5Z72rcLe/Z5hvUoemBcbzGYuUstiP8c3 d/+gcWigRe0qOkJ6Lp2MCyXIZHNxpipWcKYOWrOW3/xBAijYS1pieQBQtodaj3/w qM70BMLTdyP742Xl6yIcXVImxuYCk7wfUBk2RlEzc34bhig/HOn/vTl7QYyMn7iq p+QONwz+UrfxvJIHo0ieQ/9m3PEr7ypl98WNHyUwz4NDhhA+3ELR4VPzlWJijp+1 ShvU0usv5caGy+447zEzzl5d9AogJEEhllgRRtIKikwZn0oDq5IdSrHfUzMep5o= =Zwp2 -----END PGP SIGNATURE----- --47eKBCiAZYFK5l32--