Re: [Qemu-devel] [PATCH RFC v4 7/7] qemu_thread_create: propagate the error to callers to handle

[Fei Li]

On 09/26/2018 06:36 PM, Fam Zheng wrote:
> On Wed, 09/26 18:02, Fei Li wrote:
> > diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> > index 289af4fab5..8b044e2798 100644
> > --- a/util/qemu-thread-posix.c
> > +++ b/util/qemu-thread-posix.c
> > @@ -15,6 +15,7 @@
> >   #include "qemu/atomic.h"
> >   #include "qemu/notify.h"
> >   #include "qemu-thread-common.h"
> > +#include "qapi/error.h"
> >   
> >   static bool name_threads;
> >   
> > @@ -504,9 +505,9 @@ static void *qemu_thread_start(void *args)
> >       return start_routine(arg);
> >   }
> >   
> > -void qemu_thread_create(QemuThread *thread, const char *name,
> > -                       void *(*start_routine)(void*),
> > -                       void *arg, int mode)
> > +bool qemu_thread_create(QemuThread *thread, const char *name,
> > +                        void *(*start_routine)(void *),
> > +                        void *arg, int mode, Error **errp)
> >   {
> >       sigset_t set, oldset;
> >       int err;
> > @@ -515,7 +516,9 @@ void qemu_thread_create(QemuThread *thread, const char 
> > *name,
> >   
> >       err = pthread_attr_init(&attr);
> >       if (err) {
> > -        error_exit(err, __func__);
> > +        error_setg(errp, "pthread_attr_init failed: %s", strerror(err));
> This can use error_setg_errno.
>
> > +        errno = err;
> Is errno used anywhere in this series? The windows implementation doesn't set
> it.
Yes, this is used for the return value in qemu_signal_init() for patch 
1/7, I use
"return -1" for the further judgement in qemu_init_main_loop() in 
previous versions
but in this version I keep "return -errno" and add the "errno = err" 
here, just as I
explained in [v3 1/7]. ;)
> >   +        return false;
> >       }
> >   
> >       if (mode == QEMU_THREAD_DETACHED) {
> > @@ -530,16 +533,21 @@ void qemu_thread_create(QemuThread *thread, const char 
> > *name,
> >       qemu_thread_args->name = g_strdup(name);
> >       qemu_thread_args->start_routine = start_routine;
> >       qemu_thread_args->arg = arg;
> > -
> >       err = pthread_create(&thread->thread, &attr,
> >                            qemu_thread_start, qemu_thread_args);
> > -
> > -    if (err)
> > -        error_exit(err, __func__);
> > +    if (err) {
> > +        error_setg(errp, "pthread_create failed: %s", strerror(err));
> > +        errno = err;
> Same questions here.
>
> > +        pthread_attr_destroy(&attr);
> > +        g_free(qemu_thread_args->name);
> > +        g_free(qemu_thread_args);
> > +        return false;
> > +    }
> >   
> >       pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> >   
> >       pthread_attr_destroy(&attr);
> > +    return true;
> >   }
> >   
> >   void qemu_thread_get_self(QemuThread *thread)
> > diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
> > index 1a27e1cf6f..96e5d19ca3 100644
> > --- a/util/qemu-thread-win32.c
> > +++ b/util/qemu-thread-win32.c
> > @@ -20,6 +20,7 @@
> >   #include "qemu/thread.h"
> >   #include "qemu/notify.h"
> >   #include "qemu-thread-common.h"
> > +#include "qapi/error.h"
> >   #include <process.h>
> >   
> >   static bool name_threads;
> > @@ -388,9 +389,9 @@ void *qemu_thread_join(QemuThread *thread)
> >       return ret;
> >   }
> >   
> > -void qemu_thread_create(QemuThread *thread, const char *name,
> > -                       void *(*start_routine)(void *),
> > -                       void *arg, int mode)
> > +bool qemu_thread_create(QemuThread *thread, const char *name,
> > +                        void *(*start_routine)(void *),
> > +                        void *arg, int mode, Error **errp)
> >   {
> >       HANDLE hThread;
> >       struct QemuThreadData *data;
> > @@ -409,10 +410,17 @@ void qemu_thread_create(QemuThread *thread, const char 
> > *name,
> >       hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine,
> >                                         data, 0, &thread->tid);
> >       if (!hThread) {
> > -        error_exit(GetLastError(), __func__);
> > +        if (data->mode != QEMU_THREAD_DETACHED) {
> > +            DeleteCriticalSection(&data->cs);
> > +        }
> > +        error_setg_win32(errp, GetLastError(),
> > +                         "failed to create win32_start_routine");
> > +        g_free(data);
> > +        return false;
> >       }
> >       CloseHandle(hThread);
> >       thread->data = data;
> > +    return true;
> >   }
> >   
> >   void qemu_thread_get_self(QemuThread *thread)
> Fam