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"
>
> #define IOTHREADS_PATH "/objects"
>
> @@ -53,6 +54,9 @@ static void iothread_instance_finalize(Object *obj)
> {
> IOThread *iothread = IOTHREAD(obj);
>
> + if (!iothread->ctx) {
> + return;
> + }
> iothread->stopping = true;
> aio_notify(iothread->ctx);
> qemu_thread_join(&iothread->thread);
> @@ -63,10 +67,15 @@ 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_propagate(errp, local_error);
> + return;
> + }
> iothread->thread_id = -1;
Please move this under ->stopping = 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)
>
> 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 -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"
>
> static AioContext *ctx;
>
> @@ -810,11 +811,18 @@ static void test_source_timer_schedule(void)
>
> int main(int argc, char **argv)
> {
> + Error *local_error;
local_error = 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"
>
> 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 <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;
Same.
pgpc6jRRUAVRb.pgp
Description: PGP signature
