On Wed, 2015-10-21 at 13:00 +0100, Frediano Ziglio wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>
> Move code around to declare and place it where it fits better.
> ---
> server/red_dispatcher.c | 106 +++++++++++++-------------------------
> ----------
> server/red_dispatcher.h | 7 ++++
> server/red_worker.c | 56 +++++++++++++------------
> server/red_worker.h | 35 +---------------
> server/reds.c | 44 +++++++++++++++++++-
> server/reds.h | 16 ++++++++
> 6 files changed, 125 insertions(+), 139 deletions(-)
>
> diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> index c43da7d..3e7a5e1 100644
> --- a/server/red_dispatcher.c
> +++ b/server/red_dispatcher.c
> @@ -68,11 +68,6 @@ struct RedDispatcher {
> unsigned int max_monitors;
> };
>
> -extern uint32_t streaming_video;
> -extern SpiceImageCompression image_compression;
> -extern spice_wan_compression_t jpeg_state;
> -extern spice_wan_compression_t zlib_glz_state;
> -
> static RedDispatcher *dispatchers = NULL;
>
> static int red_dispatcher_check_qxl_version(RedDispatcher *rd, int
> major, int minor)
> @@ -204,46 +199,6 @@ static void
> red_dispatcher_cursor_migrate(RedChannelClient *rcc)
> &payload);
> }
>
> -typedef struct RendererInfo {
> - int id;
> - const char *name;
> -} RendererInfo;
> -
> -static RendererInfo renderers_info[] = {
> - {RED_RENDERER_SW, "sw"},
> -#ifdef USE_OPENGL
> - {RED_RENDERER_OGL_PBUF, "oglpbuf"},
> - {RED_RENDERER_OGL_PIXMAP, "oglpixmap"},
> -#endif
> - {RED_RENDERER_INVALID, NULL},
> -};
> -
> -static uint32_t renderers[RED_RENDERER_LAST];
> -static uint32_t num_renderers = 0;
> -
> -static RendererInfo *find_renderer(const char *name)
> -{
> - RendererInfo *inf = renderers_info;
> - while (inf->name) {
> - if (strcmp(name, inf->name) == 0) {
> - return inf;
> - }
> - inf++;
> - }
> - return NULL;
> -}
> -
> -int red_dispatcher_add_renderer(const char *name)
> -{
> - RendererInfo *inf;
> -
> - if (num_renderers == RED_RENDERER_LAST || !(inf =
> find_renderer(name))) {
> - return FALSE;
> - }
> - renderers[num_renderers++] = inf->id;
> - return TRUE;
> -}
> -
> int red_dispatcher_qxl_count(void)
> {
> return num_active_workers;
> @@ -623,14 +578,24 @@ static void qxl_worker_reset_memslots(QXLWorker
> *qxl_worker)
> red_dispatcher_reset_memslots((RedDispatcher*)qxl_worker);
> }
>
> +static bool red_dispatcher_set_pending(RedDispatcher *dispatcher,
> int pending)
> +{
> + // TODO: this is not atomic, do we need atomic?
> + if (test_bit(pending, dispatcher->pending)) {
> + return TRUE;
> + }
> +
> + set_bit(pending, &dispatcher->pending);
> + return FALSE;
> +}
> +
> static void red_dispatcher_wakeup(RedDispatcher *dispatcher)
> {
> RedWorkerMessageWakeup payload;
>
> - if (test_bit(RED_WORKER_PENDING_WAKEUP, dispatcher->pending)) {
> + if (red_dispatcher_set_pending(dispatcher,
> RED_DISPATCHER_PENDING_WAKEUP))
> return;
> - }
> - set_bit(RED_WORKER_PENDING_WAKEUP, &dispatcher->pending);
> +
> dispatcher_send_message(&dispatcher->dispatcher,
> RED_WORKER_MESSAGE_WAKEUP,
> &payload);
> @@ -645,10 +610,9 @@ static void red_dispatcher_oom(RedDispatcher
> *dispatcher)
> {
> RedWorkerMessageOom payload;
>
> - if (test_bit(RED_WORKER_PENDING_OOM, dispatcher->pending)) {
> + if (red_dispatcher_set_pending(dispatcher,
> RED_DISPATCHER_PENDING_OOM))
> return;
> - }
> - set_bit(RED_WORKER_PENDING_OOM, &dispatcher->pending);
> +
These "pending" changes seem unrelated. I would split them into a separate commit.
> dispatcher_send_message(&dispatcher->dispatcher,
> RED_WORKER_MESSAGE_OOM,
> &payload);
> @@ -1063,12 +1027,11 @@ static RedChannel
> *red_dispatcher_cursor_channel_create(RedDispatcher *dispatche
> RedDispatcher *red_dispatcher_new(QXLInstance *qxl)
> {
> RedDispatcher *red_dispatcher;
> - WorkerInitData init_data;
> - QXLDevInitInfo init_info;
> RedChannel *display_channel;
> RedChannel *cursor_channel;
> ClientCbs client_cbs = { NULL, };
>
> + spice_return_val_if_fail(qxl != NULL, NULL);
> spice_return_val_if_fail(qxl->st->dispatcher == NULL, NULL);
>
> static gsize initialized = FALSE;
> @@ -1082,22 +1045,11 @@ RedDispatcher *red_dispatcher_new(QXLInstance
> *qxl)
> }
>
> red_dispatcher = spice_new0(RedDispatcher, 1);
> + red_dispatcher->qxl = qxl;
> ring_init(&red_dispatcher->async_commands);
> spice_debug("red_dispatcher->async_commands.next %p",
> red_dispatcher->async_commands.next);
> dispatcher_init(&red_dispatcher->dispatcher,
> RED_WORKER_MESSAGE_COUNT, NULL);
> - init_data.qxl = red_dispatcher->qxl = qxl;
> - init_data.id = qxl->id;
> - init_data.red_dispatcher = red_dispatcher;
> - init_data.pending = &red_dispatcher->pending;
> - init_data.num_renderers = num_renderers;
> - memcpy(init_data.renderers, renderers,
> sizeof(init_data.renderers));
> -
> pthread_mutex_init(&red_dispatcher->async_lock, NULL);
> - init_data.image_compression = image_compression;
> - init_data.jpeg_state = jpeg_state;
> - init_data.zlib_glz_state = zlib_glz_state;
> - init_data.streaming_video = streaming_video;
> -
> red_dispatcher->base.major_version = SPICE_INTERFACE_QXL_MAJOR;
> red_dispatcher->base.minor_version = SPICE_INTERFACE_QXL_MINOR;
> red_dispatcher->base.wakeup = qxl_worker_wakeup;
> @@ -1111,7 +1063,6 @@ RedDispatcher *red_dispatcher_new(QXLInstance
> *qxl)
> red_dispatcher->base.destroy_surfaces =
> qxl_worker_destroy_surfaces;
> red_dispatcher->base.create_primary_surface =
> qxl_worker_create_primary_surface;
> red_dispatcher->base.destroy_primary_surface =
> qxl_worker_destroy_primary_surface;
> -
This whitespace change is rather superfluous. I'd probably just omit
it.
> red_dispatcher->base.reset_image_cache =
> qxl_worker_reset_image_cache;
> red_dispatcher->base.reset_cursor = qxl_worker_reset_cursor;
> red_dispatcher->base.destroy_surface_wait =
> qxl_worker_destroy_surface_wait;
> @@ -1119,20 +1070,12 @@ RedDispatcher *red_dispatcher_new(QXLInstance
> *qxl)
>
> red_dispatcher->max_monitors = UINT_MAX;
>
> - qxl->st->qif->get_init_info(qxl, &init_info);
> -
> - init_data.memslot_id_bits = init_info.memslot_id_bits;
> - init_data.memslot_gen_bits = init_info.memslot_gen_bits;
> - init_data.num_memslots = init_info.num_memslots;
> - init_data.num_memslots_groups = init_info.num_memslots_groups;
> - init_data.internal_groupslot_id =
> init_info.internal_groupslot_id;
> - init_data.n_surfaces = init_info.n_surfaces;
> + // TODO: reference and free
> + RedWorker *worker = red_worker_new(qxl, red_dispatcher);
> + red_worker_run(worker);
>
> num_active_workers = 1;
>
> - // 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) {
> @@ -1173,8 +1116,15 @@ struct Dispatcher
> *red_dispatcher_get_dispatcher(RedDispatcher *red_dispatcher)
> return &red_dispatcher->dispatcher;
> }
>
> -void red_dispatcher_set_dispatcher_opaque(struct RedDispatcher
> *red_dispatcher,
> +void red_dispatcher_set_dispatcher_opaque(RedDispatcher
> *red_dispatcher,
> void *opaque)
unrelated style change. split?
> {
> dispatcher_set_opaque(&red_dispatcher->dispatcher, opaque);
> }
> +
> +void red_dispatcher_clear_pending(RedDispatcher *red_dispatcher, int
> pending)
> +{
> + spice_return_if_fail(red_dispatcher != NULL);
> +
> + clear_bit(pending, &red_dispatcher->pending);
> +}
> diff --git a/server/red_dispatcher.h b/server/red_dispatcher.h
> index f487317..3bdeac0 100644
> --- a/server/red_dispatcher.h
> +++ b/server/red_dispatcher.h
> @@ -293,4 +293,11 @@ typedef struct
> RedWorkerMessageMonitorsConfigAsync {
> typedef struct RedWorkerMessageDriverUnload {
> } RedWorkerMessageDriverUnload;
>
> +enum {
> + RED_DISPATCHER_PENDING_WAKEUP,
> + RED_DISPATCHER_PENDING_OOM,
> +};
> +
> +void red_dispatcher_clear_pending(RedDispatcher *red_dispatcher, int
> pending);
> +
Another "pending" change. Split to a separate commit.
> #endif
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 65961db..7f73409 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -869,10 +869,11 @@ typedef struct RedWorker {
> int channel;
> int id;
> int running;
> - uint32_t *pending;
Again.
> struct pollfd poll_fds[MAX_EVENT_SOURCES];
> struct SpiceWatch watches[MAX_EVENT_SOURCES];
> unsigned int event_timeout;
> +
> + gint timeout;
> uint32_t repoll_cmd_ring;
> uint32_t repoll_cursor_ring;
> uint32_t num_renderers;
> @@ -11271,8 +11272,8 @@ void handle_dev_wakeup(void *opaque, void
> *payload)
> {
> RedWorker *worker = opaque;
>
> - clear_bit(RED_WORKER_PENDING_WAKEUP, worker->pending);
> stat_inc_counter(worker->wakeup_counter, 1);
> + red_dispatcher_clear_pending(worker->red_dispatcher,
> RED_DISPATCHER_PENDING_WAKEUP);
> }
>
> void handle_dev_oom(void *opaque, void *payload)
> @@ -11305,7 +11306,7 @@ void handle_dev_oom(void *opaque, void
> *payload)
> worker->current_size,
> worker->display_channel ?
> red_channel_sum_pipes_size(display_red_channel) :
> 0);
> - clear_bit(RED_WORKER_PENDING_OOM, worker->pending);
> + red_dispatcher_clear_pending(worker->red_dispatcher,
> RED_DISPATCHER_PENDING_OOM);
two more "pending" chunks to split.
> }
>
> void handle_dev_reset_cursor(void *opaque, void *payload)
> @@ -11852,15 +11853,20 @@ static void handle_dev_input(int fd, int
> event, void *opaque)
> dispatcher_handle_recv_read(red_dispatcher_get_dispatcher(worker
> ->red_dispatcher));
> }
>
> -RedWorker* red_worker_new(WorkerInitData *init_data)
> +RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher
> *red_dispatcher)
> {
> - RedWorker *worker = spice_new0(RedWorker, 1);
> + QXLDevInitInfo init_info;
> + RedWorker *worker;
> Dispatcher *dispatcher;
> int i;
> const char *record_filename;
>
> spice_assert(sizeof(CursorItem) <= QXL_CURSUR_DEVICE_DATA_SIZE);
>
> + qxl->st->qif->get_init_info(qxl, &init_info);
> +
> + worker = spice_new0(RedWorker, 1);
> +
> record_filename = getenv("SPICE_WORKER_RECORD_FILENAME");
> if (record_filename) {
> static const char header[] = "SPICE_REPLAY 1\n";
> @@ -11873,27 +11879,27 @@ RedWorker* red_worker_new(WorkerInitData
> *init_data)
> spice_error("failed to write replay header");
> }
> }
> - dispatcher = red_dispatcher_get_dispatcher(init_data
> ->red_dispatcher);
> + dispatcher = red_dispatcher_get_dispatcher(red_dispatcher);
> dispatcher_set_opaque(dispatcher, worker);
> - worker->red_dispatcher = init_data->red_dispatcher;
> - worker->qxl = init_data->qxl;
> - worker->id = init_data->id;
> +
> + worker->red_dispatcher = red_dispatcher;
> + worker->qxl = qxl;
> + worker->id = qxl->id;
> worker->channel = dispatcher_get_recv_fd(dispatcher);
> register_callbacks(dispatcher);
> if (worker->record_fd) {
> dispatcher_register_universal_handler(dispatcher,
> worker_dispatcher_record);
> }
> - worker->pending = init_data->pending;
> worker->cursor_visible = TRUE;
> - spice_assert(init_data->num_renderers > 0);
> - worker->num_renderers = init_data->num_renderers;
> - memcpy(worker->renderers, init_data->renderers, sizeof(worker
> ->renderers));
> + spice_assert(num_renderers > 0);
> + worker->num_renderers = num_renderers;
> + memcpy(worker->renderers, renderers, sizeof(worker->renderers));
> worker->renderer = RED_RENDERER_INVALID;
> worker->mouse_mode = SPICE_MOUSE_MODE_SERVER;
> - worker->image_compression = init_data->image_compression;
> - worker->jpeg_state = init_data->jpeg_state;
> - worker->zlib_glz_state = init_data->zlib_glz_state;
> - worker->streaming_video = init_data->streaming_video;
> + worker->image_compression = image_compression;
> + worker->jpeg_state = jpeg_state;
> + worker->zlib_glz_state = zlib_glz_state;
> + worker->streaming_video = streaming_video;
> worker->driver_cap_monitors_config = 0;
> ring_init(&worker->current_list);
> image_cache_init(&worker->image_cache);
> @@ -11922,14 +11928,14 @@ RedWorker* red_worker_new(WorkerInitData
> *init_data)
> worker->watches[0].watch_func_opaque = worker;
>
> red_memslot_info_init(&worker->mem_slots,
> - init_data->num_memslots_groups,
> - init_data->num_memslots,
> - init_data->memslot_gen_bits,
> - init_data->memslot_id_bits,
> - init_data->internal_groupslot_id);
> -
> - spice_warn_if(init_data->n_surfaces > NUM_SURFACES);
> - worker->n_surfaces = init_data->n_surfaces;
> + init_info.num_memslots_groups,
> + init_info.num_memslots,
> + init_info.memslot_gen_bits,
> + init_info.memslot_id_bits,
> + init_info.internal_groupslot_id);
> +
> + spice_warn_if(init_info.n_surfaces > NUM_SURFACES);
> + worker->n_surfaces = init_info.n_surfaces;
>
> if (!spice_timer_queue_create()) {
> spice_error("failed to create timer queue");
> diff --git a/server/red_worker.h b/server/red_worker.h
> index c935e0a..c1adca4 100644
> --- a/server/red_worker.h
> +++ b/server/red_worker.h
> @@ -23,42 +23,9 @@
> #include "red_common.h"
> #include "red_dispatcher.h"
>
> -enum {
> - RED_WORKER_PENDING_WAKEUP,
> - RED_WORKER_PENDING_OOM,
> -};
> -
> -enum {
> - RED_RENDERER_INVALID,
> - RED_RENDERER_SW,
> - RED_RENDERER_OGL_PBUF,
> - RED_RENDERER_OGL_PIXMAP,
> -
> - RED_RENDERER_LAST
> -};
> -
> typedef struct RedWorker RedWorker;
>
> -typedef struct WorkerInitData {
> - struct QXLInstance *qxl;
> - int id;
> - uint32_t *pending;
> - uint32_t num_renderers;
> - uint32_t renderers[RED_RENDERER_LAST];
> - SpiceImageCompression image_compression;
> - spice_wan_compression_t jpeg_state;
> - spice_wan_compression_t zlib_glz_state;
> - int streaming_video;
> - uint32_t num_memslots;
> - uint32_t num_memslots_groups;
> - uint8_t memslot_gen_bits;
> - uint8_t memslot_id_bits;
> - uint8_t internal_groupslot_id;
> - uint32_t n_surfaces;
> - RedDispatcher *red_dispatcher;
> -} WorkerInitData;
> -
> -RedWorker* red_worker_new(WorkerInitData *init_data);
> +RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher
> *red_dispatcher);
> bool red_worker_run(RedWorker *worker);
>
> #endif
> diff --git a/server/reds.c b/server/reds.c
> index d53ea66..90d14b5 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -3364,6 +3364,46 @@ SPICE_GNUC_VISIBLE SpiceServer
> *spice_server_new(void)
> return reds;
> }
>
> +typedef struct RendererInfo {
> + int id;
> + const char *name;
> +} RendererInfo;
> +
> +static RendererInfo renderers_info[] = {
> + {RED_RENDERER_SW, "sw"},
> +#ifdef USE_OPENGL
> + {RED_RENDERER_OGL_PBUF, "oglpbuf"},
> + {RED_RENDERER_OGL_PIXMAP, "oglpixmap"},
> +#endif
> + {RED_RENDERER_INVALID, NULL},
> +};
> +
> +uint32_t renderers[RED_RENDERER_LAST];
> +uint32_t num_renderers = 0;
> +
> +static RendererInfo *find_renderer(const char *name)
> +{
> + RendererInfo *inf = renderers_info;
> + while (inf->name) {
> + if (strcmp(name, inf->name) == 0) {
> + return inf;
> + }
> + inf++;
> + }
> + return NULL;
> +}
> +
> +static int red_add_renderer(const char *name)
> +{
> + RendererInfo *inf;
> +
> + if (num_renderers == RED_RENDERER_LAST || !(inf =
> find_renderer(name))) {
> + return FALSE;
> + }
> + renderers[num_renderers++] = inf->id;
> + return TRUE;
> +}
> +
> SPICE_GNUC_VISIBLE int spice_server_init(SpiceServer *s,
> SpiceCoreInterface *core)
> {
> int ret;
> @@ -3371,7 +3411,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_init(SpiceServer *s, SpiceCoreInterface *cor
> spice_assert(reds == s);
> ret = do_spice_init(core);
> if (default_renderer) {
> - red_dispatcher_add_renderer(default_renderer);
> + red_add_renderer(default_renderer);
> }
> return ret;
> }
> @@ -3664,7 +3704,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_is_server_mouse(SpiceServer *s)
> SPICE_GNUC_VISIBLE int spice_server_add_renderer(SpiceServer *s,
> const char *name)
> {
> spice_assert(reds == s);
> - if (!red_dispatcher_add_renderer(name)) {
> + if (!red_add_renderer(name)) {
> return -1;
> }
> default_renderer = NULL;
> diff --git a/server/reds.h b/server/reds.h
> index a9c2df9..7937d2d 100644
> --- a/server/reds.h
> +++ b/server/reds.h
> @@ -59,7 +59,23 @@ int reds_get_agent_mouse(void); // used by
> inputs_channel
> int reds_has_vdagent(void); // used by inputs channel
> void reds_handle_agent_mouse_event(const VDAgentMouseState
> *mouse_state); // used by inputs_channel
>
> +enum {
> + RED_RENDERER_INVALID,
> + RED_RENDERER_SW,
> + RED_RENDERER_OGL_PBUF,
> + RED_RENDERER_OGL_PIXMAP,
> +
> + RED_RENDERER_LAST
> +};
> +
> +extern uint32_t renderers[RED_RENDERER_LAST];
> +extern uint32_t num_renderers;
> +
> extern struct SpiceCoreInterface *core;
> +extern uint32_t streaming_video;
> +extern SpiceImageCompression image_compression;
> +extern spice_wan_compression_t jpeg_state;
> +extern spice_wan_compression_t zlib_glz_state;
>
> // Temporary measures to make splitting reds.c to inputs_channel.c
> easier
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel