On Wed, Sep 26, 2018 at 7:13 PM Fei Li <f...@suse.com> wrote:
>
>
>
> 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]. ;)
This makes an inconsistent function contract: on error, does errno get
set? For Linux implementation it is yes, but for Windows it is no.
Which I think is wrong. Am I missing anything?
Fam
> >> + 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
> >
> >
> >
>
