[Spice-devel] [PATCH 02/10] server: remove dispatching creation of worker channels

Frediano Ziglio fziglio at redhat.com
Thu Nov 5 01:14:55 PST 2015


From: Marc-André Lureau <marcandre.lureau at gmail.com>

---
 server/dispatcher.c         |  6 ++--
 server/red_dispatcher.c     | 74 ++++++++++++++-------------------------------
 server/red_dispatcher.h     | 36 ++--------------------
 server/red_worker.c         | 68 ++++++++++++++---------------------------
 server/red_worker.h         |  2 ++
 server/spice_server_utils.h |  1 -
 6 files changed, 51 insertions(+), 136 deletions(-)

diff --git a/server/dispatcher.c b/server/dispatcher.c
index d334117..945edba 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -32,7 +32,6 @@
 #include "common/mem.h"
 #include "common/spice_common.h"
 #include "dispatcher.h"
-#include "red_dispatcher.h"
 
 //#define DEBUG_DISPATCHER
 
@@ -203,12 +202,13 @@ unlock:
 
 uint32_t dispatcher_read_message(Dispatcher *dispatcher)
 {
-    uint32_t message;
+    uint32_t message = 0;
 
     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));
+    if (read_safe(dispatcher->send_fd, (uint8_t*)&message, sizeof(message), 1) == -1)
+        spice_warn_if_reached();
 
     return message;
 }
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index 43f061d..7855776 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -1001,35 +1001,10 @@ void red_dispatcher_async_complete(struct RedDispatcher *dispatcher,
     free(async_command);
 }
 
-static RedChannel *red_dispatcher_display_channel_create(RedDispatcher *dispatcher)
-{
-    RedWorkerMessageDisplayChannelCreate payload;
-    RedChannel *display_channel;
-
-    dispatcher_send_message(&dispatcher->dispatcher,
-                            RED_WORKER_MESSAGE_DISPLAY_CHANNEL_CREATE,
-                            &payload);
-    receive_data(dispatcher->dispatcher.send_fd, &display_channel, sizeof(RedChannel *));
-    return display_channel;
-}
-
-static RedChannel *red_dispatcher_cursor_channel_create(RedDispatcher *dispatcher)
-{
-    RedWorkerMessageCursorChannelCreate payload;
-    RedChannel *cursor_channel;
-
-    dispatcher_send_message(&dispatcher->dispatcher,
-                            RED_WORKER_MESSAGE_CURSOR_CHANNEL_CREATE,
-                            &payload);
-    receive_data(dispatcher->dispatcher.send_fd, &cursor_channel, sizeof(RedChannel *));
-    return cursor_channel;
-}
-
 void red_dispatcher_init(QXLInstance *qxl)
 {
     RedDispatcher *red_dispatcher;
-    RedChannel *display_channel;
-    RedChannel *cursor_channel;
+    RedChannel *channel;
     ClientCbs client_cbs = { NULL, };
 
     spice_return_if_fail(qxl != NULL);
@@ -1074,34 +1049,29 @@ void red_dispatcher_init(QXLInstance *qxl)
 
     // TODO: reference and free
     RedWorker *worker = red_worker_new(qxl, red_dispatcher);
-    red_worker_run(worker);
-
-    num_active_workers = 1;
 
-    display_channel = red_dispatcher_display_channel_create(red_dispatcher);
-
-    if (display_channel) {
-        client_cbs.connect = red_dispatcher_set_display_peer;
-        client_cbs.disconnect = red_dispatcher_disconnect_display_peer;
-        client_cbs.migrate = red_dispatcher_display_migrate;
-        red_channel_register_client_cbs(display_channel, &client_cbs);
-        red_channel_set_data(display_channel, red_dispatcher);
-        red_channel_set_cap(display_channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
-        red_channel_set_cap(display_channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
-        red_channel_set_cap(display_channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
-        reds_register_channel(display_channel);
-    }
-
-    cursor_channel = red_dispatcher_cursor_channel_create(red_dispatcher);
+    // TODO: move to their respective channel files
+    channel = red_worker_get_cursor_channel(worker);
+    client_cbs.connect = red_dispatcher_set_cursor_peer;
+    client_cbs.disconnect = red_dispatcher_disconnect_cursor_peer;
+    client_cbs.migrate = red_dispatcher_cursor_migrate;
+    red_channel_register_client_cbs(channel, &client_cbs);
+    red_channel_set_data(channel, red_dispatcher);
+    reds_register_channel(channel);
+
+    channel = red_worker_get_display_channel(worker);
+    client_cbs.connect = red_dispatcher_set_display_peer;
+    client_cbs.disconnect = red_dispatcher_disconnect_display_peer;
+    client_cbs.migrate = red_dispatcher_display_migrate;
+    red_channel_register_client_cbs(channel, &client_cbs);
+    red_channel_set_data(channel, red_dispatcher);
+    red_channel_set_cap(channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
+    red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
+    red_channel_set_cap(channel, SPICE_DISPLAY_CAP_STREAM_REPORT);
+    reds_register_channel(channel);
 
-    if (cursor_channel) {
-        client_cbs.connect = red_dispatcher_set_cursor_peer;
-        client_cbs.disconnect = red_dispatcher_disconnect_cursor_peer;
-        client_cbs.migrate = red_dispatcher_cursor_migrate;
-        red_channel_register_client_cbs(cursor_channel, &client_cbs);
-        red_channel_set_data(cursor_channel, red_dispatcher);
-        reds_register_channel(cursor_channel);
-    }
+    red_worker_run(worker);
+    num_active_workers = 1;
 
     qxl->st->dispatcher = red_dispatcher;
     red_dispatcher->next = dispatchers;
diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
index 242143a..02337b8 100644
--- a/server/red_dispatcher.h
+++ b/server/red_dispatcher.h
@@ -45,38 +45,6 @@ void red_dispatcher_client_monitors_config(VDAgentMonitorsConfig *monitors_confi
 
 typedef uint32_t RedWorkerMessage;
 
-static inline void send_data(int fd, void *in_buf, int n)
-{
-    uint8_t *buf = in_buf;
-    do {
-        int now;
-        if ((now = write(fd, buf, n)) == -1) {
-            if (errno == EINTR) {
-                continue;
-            }
-            spice_error("%s", strerror(errno));
-        }
-        buf += now;
-        n -= now;
-    } while (n);
-}
-
-static inline void receive_data(int fd, void *in_buf, int n)
-{
-    uint8_t *buf = in_buf;
-    do {
-        int now;
-        if ((now = read(fd, buf, n)) == -1) {
-            if (errno == EINTR) {
-                continue;
-            }
-            spice_error("%s", strerror(errno));
-        }
-        buf += now;
-        n -= now;
-    } while (n);
-}
-
 /* Keep message order, only append new messages!
  * Replay code store enum values into save files.
  */
@@ -119,8 +87,8 @@ enum {
     /* suspend/windows resolution change command */
     RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC,
 
-    RED_WORKER_MESSAGE_DISPLAY_CHANNEL_CREATE,
-    RED_WORKER_MESSAGE_CURSOR_CHANNEL_CREATE,
+    RED_WORKER_MESSAGE_DISPLAY_CHANNEL_CREATE, /* unused */
+    RED_WORKER_MESSAGE_CURSOR_CHANNEL_CREATE, /* unused */
 
     RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC,
     RED_WORKER_MESSAGE_DRIVER_UNLOAD,
diff --git a/server/red_worker.c b/server/red_worker.c
index 821ea75..d8a8a4a 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -436,8 +436,6 @@ typedef struct RedWorker {
     clockid_t clockid;
     QXLInstance *qxl;
     RedDispatcher *red_dispatcher;
-
-    int channel;
     int running;
     struct pollfd poll_fds[MAX_EVENT_SOURCES];
     struct SpiceWatch watches[MAX_EVENT_SOURCES];
@@ -9903,23 +9901,6 @@ void handle_dev_create_primary_surface_async(void *opaque, void *payload)
     dev_create_primary_surface(worker, msg->surface_id, msg->surface);
 }
 
-/* exception for Dispatcher, data going from red_worker to main thread,
- * TODO: use a different dispatcher?
- * TODO: leave direct usage of channel(fd)? It's only used right after the
- * pthread is created, since the channel duration is the lifetime of the spice
- * server. */
-
-void handle_dev_display_channel_create(void *opaque, void *payload)
-{
-    RedWorker *worker = opaque;
-
-    RedChannel *red_channel;
-    // TODO: handle seemless migration. Temp, setting migrate to FALSE
-    display_channel_create(worker, FALSE);
-    red_channel = &worker->display_channel->common.base;
-    send_data(worker->channel, &red_channel, sizeof(RedChannel *));
-}
-
 void handle_dev_display_connect(void *opaque, void *payload)
 {
     RedWorkerMessageDisplayConnect *msg = payload;
@@ -10011,21 +9992,6 @@ static void handle_dev_monitors_config_async(void *opaque, void *payload)
 }
 
 /* TODO: special, perhaps use another dispatcher? */
-void handle_dev_cursor_channel_create(void *opaque, void *payload)
-{
-    RedWorker *worker = opaque;
-    RedChannel *red_channel;
-
-    if (!worker->cursor_channel) {
-        worker->cursor_channel = cursor_channel_new(worker);
-    } else {
-        spice_warning("cursor channel already created");
-    }
-
-    red_channel = RED_CHANNEL(worker->cursor_channel);
-    send_data(worker->channel, &red_channel, sizeof(RedChannel *));
-}
-
 void handle_dev_cursor_connect(void *opaque, void *payload)
 {
     RedWorkerMessageCursorConnect *msg = payload;
@@ -10371,16 +10337,6 @@ static void register_callbacks(Dispatcher *dispatcher)
                                 sizeof(RedWorkerMessageSetMouseMode),
                                 DISPATCHER_NONE);
     dispatcher_register_handler(dispatcher,
-                                RED_WORKER_MESSAGE_DISPLAY_CHANNEL_CREATE,
-                                handle_dev_display_channel_create,
-                                sizeof(RedWorkerMessageDisplayChannelCreate),
-                                DISPATCHER_NONE);
-    dispatcher_register_handler(dispatcher,
-                                RED_WORKER_MESSAGE_CURSOR_CHANNEL_CREATE,
-                                handle_dev_cursor_channel_create,
-                                sizeof(RedWorkerMessageCursorChannelCreate),
-                                DISPATCHER_NONE);
-    dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT,
                                 handle_dev_destroy_surface_wait,
                                 sizeof(RedWorkerMessageDestroySurfaceWait),
@@ -10445,7 +10401,6 @@ RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher)
 
     worker->red_dispatcher = red_dispatcher;
     worker->qxl = qxl;
-    worker->channel = dispatcher_get_recv_fd(dispatcher);
     register_callbacks(dispatcher);
     if (worker->record_fd) {
         dispatcher_register_universal_handler(dispatcher, worker_dispatcher_record);
@@ -10474,7 +10429,7 @@ RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher)
         worker->poll_fds[i].fd = -1;
     }
 
-    worker->poll_fds[0].fd = worker->channel;
+    worker->poll_fds[0].fd = dispatcher_get_recv_fd(dispatcher);
     worker->poll_fds[0].events = POLLIN;
     worker->watches[0].worker = worker;
     worker->watches[0].watch_func = handle_dev_input;
@@ -10499,6 +10454,10 @@ RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher)
     red_init_zlib(worker);
     worker->event_timeout = INF_EVENT_WAIT;
 
+    worker->cursor_channel = cursor_channel_new(worker);
+    // TODO: handle seemless migration. Temp, setting migrate to FALSE
+    display_channel_create(worker, FALSE);
+
     return worker;
 }
 
@@ -10525,6 +10484,9 @@ SPICE_GNUC_NORETURN static void *red_worker_main(void *arg)
         spice_warning("getcpuclockid failed");
     }
 
+    RED_CHANNEL(worker->cursor_channel)->thread_id = pthread_self();
+    RED_CHANNEL(worker->display_channel)->thread_id = pthread_self();
+
     for (;;) {
         int i, num_events;
         unsigned int timeout;
@@ -10609,3 +10571,17 @@ bool red_worker_run(RedWorker *worker)
 
     return r == 0;
 }
+
+RedChannel* red_worker_get_cursor_channel(RedWorker *worker)
+{
+    spice_return_val_if_fail(worker, NULL);
+
+    return RED_CHANNEL(worker->cursor_channel);
+}
+
+RedChannel* red_worker_get_display_channel(RedWorker *worker)
+{
+    spice_return_val_if_fail(worker, NULL);
+
+    return RED_CHANNEL(worker->display_channel);
+}
diff --git a/server/red_worker.h b/server/red_worker.h
index 33dd974..2995b8f 100644
--- a/server/red_worker.h
+++ b/server/red_worker.h
@@ -106,6 +106,8 @@ static inline void red_pipes_add_verb(RedChannel *channel, uint16_t verb)
 RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher);
 bool       red_worker_run(RedWorker *worker);
 QXLInstance* red_worker_get_qxl(RedWorker *worker);
+RedChannel* red_worker_get_cursor_channel(RedWorker *worker);
+RedChannel* red_worker_get_display_channel(RedWorker *worker);
 
 RedChannel *red_worker_new_channel(RedWorker *worker, int size,
                                    const char *name,
diff --git a/server/spice_server_utils.h b/server/spice_server_utils.h
index 1f5b7f1..087ad6f 100644
--- a/server/spice_server_utils.h
+++ b/server/spice_server_utils.h
@@ -37,5 +37,4 @@ static inline int test_bit(int index, uint32_t val)
 {
     return val & (1u << index);
 }
-
 #endif
-- 
2.4.3



More information about the Spice-devel mailing list