Hey, this patch breaks starting VMs through virsh for me: $ virsh start win7-home-pr erreur :Impossible de démarrer le domaine win7-home-pr erreur :internal error: early end of file from monitor: possible problem: ((null):27254): Spice-ERROR **: spice_timer_queue.c:226:spice_timer_queue_get_timeout_ms: assertion `queue != NULL' failed Thread 8 (Thread 0x7fc404d98700 (LWP 27259)): #0 0x00007fc4204cc89d in __lll_lock_wait () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135 #1 0x00007fc4204c69cd in __GI___pthread_mutex_lock (mutex=mutex@entry=0x55c29555c2e0 <qemu_global_mutex>) at ../nptl/pthread_mutex_lock.c:81 #2 0x000055c2950567a9 in qemu_mutex_lock (mutex=mutex@entry=0x55c29555c2e0 <qemu_global_mutex>) at util/qemu-thread-posix.c:73 #3 0x000055c294d97991 in qemu_mutex_lock_iothread () at /usr/src/debug/qemu-2.4.0.1/cpus.c:1170 #4 0x000055c29506410e in call_rcu_thread (opaque=<optimized out>) at util/rcu.c:243 #5 0x00007fc4204c460a in start_thread (arg=0x7fc404d98700) at pthread_create.c:334 #6 0x00007fc411d48bbd in clone () at ../sysdeps/unix/sysv/linux/x86_64/c (haven't tried to figure out what's going on) Christophe On Thu, Oct 22, 2015 at 10:49:33AM -0500, Jonathon Jongsma wrote: > On Wed, 2015-10-21 at 08:21 -0400, Frediano Ziglio wrote: > > > > > > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> > > > > > > --- > > > server/dispatcher.c | 13 +++++++++++++ > > > server/dispatcher.h | 2 ++ > > > server/red_dispatcher.c | 21 +++------------------ > > > server/red_dispatcher.h | 6 +----- > > > server/red_worker.c | 33 ++++++++++++++++++++++++++++++--- > > > server/red_worker.h | 5 ++++- > > > 6 files changed, 53 insertions(+), 27 deletions(-) > > > > > > diff --git a/server/dispatcher.c b/server/dispatcher.c > > > index d6c03ca..d334117 100644 > > > --- a/server/dispatcher.c > > > +++ b/server/dispatcher.c > > > @@ -32,6 +32,7 @@ > > > #include "common/mem.h" > > > #include "common/spice_common.h" > > > #include "dispatcher.h" > > > +#include "red_dispatcher.h" > > > > > > //#define DEBUG_DISPATCHER > > > > > > @@ -200,6 +201,18 @@ unlock: > > > pthread_mutex_unlock(&dispatcher->lock); > > > } > > > > > > +uint32_t dispatcher_read_message(Dispatcher *dispatcher) > > > +{ > > > + uint32_t message; > > > + > > > + spice_return_val_if_fail(dispatcher, 0); > > > + spice_return_val_if_fail(dispatcher->send_fd != -1, 0); > > > + > > > + receive_data(dispatcher->send_fd, &message, sizeof(message)); > > > + > > > + return message; > > > +} > > > + > > > void dispatcher_register_async_done_callback( > > > Dispatcher *dispatcher, > > > > > > dispatcher_handle_async_done > > > handler) > > > diff --git a/server/dispatcher.h b/server/dispatcher.h > > > index c3e7c74..8882532 100644 > > > --- a/server/dispatcher.h > > > +++ b/server/dispatcher.h > > > @@ -19,6 +19,7 @@ > > > #define DISPATCHER_H > > > > > > #include <spice.h> > > > +#include "spice_server_utils.h" > > > > > > typedef struct Dispatcher Dispatcher; > > > > > > > This include can be removed > > > > > @@ -63,6 +64,7 @@ struct Dispatcher { > > > */ > > > void dispatcher_send_message(Dispatcher *dispatcher, uint32_t > > > message_type, > > > void *payload); > > > +uint32_t dispatcher_read_message(Dispatcher *dispatcher); > > > > > > /* > > > * dispatcher_init > > > diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c > > > index 7ad860c..0bc853d 100644 > > > --- a/server/red_dispatcher.c > > > +++ b/server/red_dispatcher.c > > > @@ -56,7 +56,6 @@ struct RedDispatcher { > > > QXLWorker base; > > > QXLInstance *qxl; > > > Dispatcher dispatcher; > > > - pthread_t worker_thread; > > > uint32_t pending; > > > int primary_active; > > > int x_res; > > > @@ -1064,14 +1063,10 @@ static RedChannel > > > *red_dispatcher_cursor_channel_create(RedDispatcher *dispatche > > > void red_dispatcher_init(QXLInstance *qxl) > > > { > > > RedDispatcher *red_dispatcher; > > > - RedWorkerMessage message; > > > WorkerInitData init_data; > > > QXLDevInitInfo init_info; > > > - int r; > > > RedChannel *display_channel; > > > RedChannel *cursor_channel; > > > - sigset_t thread_sig_mask; > > > - sigset_t curr_sig_mask; > > > ClientCbs client_cbs = { NULL, }; > > > > > > spice_return_if_fail(qxl->st->dispatcher == NULL); > > > @@ -1135,19 +1130,9 @@ void red_dispatcher_init(QXLInstance *qxl) > > > > > > num_active_workers = 1; > > > > > > - sigfillset(&thread_sig_mask); > > > - sigdelset(&thread_sig_mask, SIGILL); > > > - sigdelset(&thread_sig_mask, SIGFPE); > > > - sigdelset(&thread_sig_mask, SIGSEGV); > > > - pthread_sigmask(SIG_SETMASK, &thread_sig_mask, > > > &curr_sig_mask); > > > - if ((r = pthread_create(&red_dispatcher->worker_thread, NULL, > > > red_worker_main, &init_data))) { > > > - spice_error("create thread failed %d", r); > > > - } > > > - pthread_sigmask(SIG_SETMASK, &curr_sig_mask, NULL); > > > - > > > - read_message(red_dispatcher->dispatcher.send_fd, &message); > > > - spice_assert(message == RED_WORKER_MESSAGE_READY); > > > - > > > + // TODO: reference and free > > > + RedWorker *worker = red_worker_new(&init_data); > > > + red_worker_run(worker); > > > display_channel = > > > red_dispatcher_display_channel_create(red_dispatcher); > > > > > > if (display_channel) { > > > diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h > > > index dbe65d1..2121007 100644 > > > --- a/server/red_dispatcher.h > > > +++ b/server/red_dispatcher.h > > > @@ -18,6 +18,7 @@ > > > #ifndef _H_RED_DISPATCHER > > > #define _H_RED_DISPATCHER > > > > > > +#include <unistd.h> > > > #include <errno.h> > > > #include "spice_server_utils.h" > > > #include "red_channel.h" > > > > Even this include > > > > > @@ -83,11 +84,6 @@ static inline void receive_data(int fd, void > > > *in_buf, int > > > n) > > > } while (n); > > > } > > > > > > -static inline void read_message(int fd, RedWorkerMessage *message) > > > -{ > > > - receive_data(fd, message, sizeof(RedWorkerMessage)); > > > -} > > > - > > > enum { > > > RED_WORKER_MESSAGE_NOP, > > > RED_WORKER_MESSAGE_UPDATE, > > > diff --git a/server/red_worker.c b/server/red_worker.c > > > index 225c272..daa233b 100644 > > > --- a/server/red_worker.c > > > +++ b/server/red_worker.c > > > @@ -868,6 +868,7 @@ typedef struct ItemTrace { > > > #define NUM_CURSORS 100 > > > > > > typedef struct RedWorker { > > > + pthread_t thread; > > > DisplayChannel *display_channel; > > > CursorChannel *cursor_channel; > > > QXLInstance *qxl; > > > @@ -11849,7 +11850,7 @@ static void handle_dev_input(int fd, int > > > event, void > > > *opaque) > > > > > > dispatcher_handle_recv_read(red_dispatcher_get_dispatcher(worker > > > ->red_dispatcher)); > > > } > > > > > > -static RedWorker* red_worker_new(WorkerInitData *init_data) > > > +RedWorker* red_worker_new(WorkerInitData *init_data) > > > { > > > RedWorker *worker = spice_new0(RedWorker, 1); > > > RedWorkerMessage message; > > > @@ -11954,14 +11955,15 @@ static void > > > red_display_cc_free_glz_drawables(RedChannelClient *rcc) > > > red_display_handle_glz_drawables_to_free(dcc); > > > } > > > > > > -SPICE_GNUC_NORETURN void *red_worker_main(void *arg) > > > +SPICE_GNUC_NORETURN static void *red_worker_main(void *arg) > > > { > > > - RedWorker *worker = red_worker_new(arg); > > > + RedWorker *worker = arg; > > > > > > spice_info("begin"); > > > spice_assert(MAX_PIPE_SIZE > WIDE_CLIENT_ACK_WINDOW && > > > MAX_PIPE_SIZE > NARROW_CLIENT_ACK_WINDOW); //ensure > > > wakeup by ack > > > message > > > > > > + // TODO: call once unconditionnally > > > if (pthread_getcpuclockid(pthread_self(), &clock_id)) { > > > spice_error("pthread_getcpuclockid failed"); > > > } > > > @@ -12028,3 +12030,28 @@ SPICE_GNUC_NORETURN void > > > *red_worker_main(void *arg) > > > > > > spice_warn_if_reached(); > > > } > > > + > > > +bool red_worker_run(RedWorker *worker) > > > +{ > > > + uint32_t message; > > > + sigset_t thread_sig_mask; > > > + sigset_t curr_sig_mask; > > > + int r; > > > + > > > + spice_return_val_if_fail(worker, FALSE); > > > + spice_return_val_if_fail(!worker->thread, FALSE); > > > + > > > + sigfillset(&thread_sig_mask); > > > + sigdelset(&thread_sig_mask, SIGILL); > > > + sigdelset(&thread_sig_mask, SIGFPE); > > > + sigdelset(&thread_sig_mask, SIGSEGV); > > > + pthread_sigmask(SIG_SETMASK, &thread_sig_mask, > > > &curr_sig_mask); > > > + if ((r = pthread_create(&worker->thread, NULL, > > > red_worker_main, > > > worker))) { > > > + spice_error("create thread failed %d", r); > > > + } > > > + pthread_sigmask(SIG_SETMASK, &curr_sig_mask, NULL); > > > + > > > + message = > > > dispatcher_read_message(red_dispatcher_get_dispatcher(worker > > > ->red_dispatcher)); > > > + > > > + return message == RED_WORKER_MESSAGE_READY; > > > +} > > > diff --git a/server/red_worker.h b/server/red_worker.h > > > index c71e9c8..c935e0a 100644 > > > --- a/server/red_worker.h > > > +++ b/server/red_worker.h > > > @@ -37,6 +37,8 @@ enum { > > > RED_RENDERER_LAST > > > }; > > > > > > +typedef struct RedWorker RedWorker; > > > + > > > typedef struct WorkerInitData { > > > struct QXLInstance *qxl; > > > int id; > > > @@ -56,6 +58,7 @@ typedef struct WorkerInitData { > > > RedDispatcher *red_dispatcher; > > > } WorkerInitData; > > > > > > -void *red_worker_main(void *arg); > > > +RedWorker* red_worker_new(WorkerInitData *init_data); > > > +bool red_worker_run(RedWorker *worker); > > > > > > #endif > > > > Beside these small changes I agree with this patch. > > Agree, ACK from me as well. > > > > > > Frediano > > _______________________________________________ > > Spice-devel mailing list > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel