Re: [PATCH 02/10] server: remove dispatching creation of worker channels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> 
> ---
>  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 0c91b9f..df5be98 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -404,8 +404,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];
> @@ -9703,23 +9701,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;
> @@ -9811,21 +9792,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;
> @@ -10162,16 +10128,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),
> @@ -10236,7 +10192,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);
> @@ -10265,7 +10220,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;
> @@ -10290,6 +10245,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;
>  }
>  
> @@ -10316,6 +10275,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;
> @@ -10400,3 +10362,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

I'll split it

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]