On Wed, 2015-12-09 at 12:17 +0000, Frediano Ziglio wrote:
> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
>
> Clean up, more extensible.
> ---
> server/Makefile.am | 2 -
> server/red-worker.c | 357 ++++++++++++++++++++++++++++----------------
> -
> server/red-worker.h | 1 +
> server/spice_timer_queue.c | 273 ----------------------------------
> server/spice_timer_queue.h | 43 ------
> 5 files changed, 224 insertions(+), 452 deletions(-)
> delete mode 100644 server/spice_timer_queue.c
> delete mode 100644 server/spice_timer_queue.h
>
> diff --git a/server/Makefile.am b/server/Makefile.am
> index d4fc972..88825d8 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -121,8 +121,6 @@ libspice_server_la_SOURCES = \
> spice.h \
> stat.h \
> spicevmc.c \
> - spice_timer_queue.c \
> - spice_timer_queue.h \
> zlib-encoder.c \
> zlib-encoder.h \
> image-cache.h \
> diff --git a/server/red-worker.c b/server/red-worker.c
> index baa8458..9f2aba1 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -49,30 +49,22 @@
>
> #include "spice.h"
> #include "red-worker.h"
> -#include "spice_timer_queue.h"
> #include "cursor-channel.h"
> #include "tree.h"
>
> #define CMD_RING_POLL_TIMEOUT 10 //milli
> #define CMD_RING_POLL_RETRIES 200
>
> -#define MAX_EVENT_SOURCES 20
> #define INF_EVENT_WAIT ~0
>
> -struct SpiceWatch {
> - struct RedWorker *worker;
> - SpiceWatchFunc watch_func;
> - void *watch_func_opaque;
> -};
> -
> struct RedWorker {
> pthread_t thread;
> clockid_t clockid;
> QXLInstance *qxl;
> RedDispatcher *red_dispatcher;
> int running;
> - struct pollfd poll_fds[MAX_EVENT_SOURCES];
> - struct SpiceWatch watches[MAX_EVENT_SOURCES];
> +
> + GMainContext *main_context;
> unsigned int event_timeout;
>
> DisplayChannel *display_channel;
> @@ -99,6 +91,13 @@ struct RedWorker {
> FILE *record_fd;
> };
>
> +GMainContext* red_worker_get_context(RedWorker *worker)
> +{
> + spice_return_val_if_fail(worker, NULL);
> +
> + return worker->main_context;
> +}
I still prefer get_main_context() here.
> +
> QXLInstance* red_worker_get_qxl(RedWorker *worker)
> {
> spice_return_val_if_fail(worker != NULL, NULL);
> @@ -510,81 +509,159 @@ static int
> common_channel_config_socket(RedChannelClient *rcc)
> return TRUE;
> }
>
> -static void worker_watch_update_mask(SpiceWatch *watch, int event_mask)
> +typedef struct SpiceTimer {
> + SpiceTimerFunc func;
> + void *opaque;
> + guint source_id;
> +} SpiceTimer;
> +
> +static SpiceTimer* worker_timer_add(SpiceTimerFunc func, void *opaque)
> {
> - struct RedWorker *worker;
> - int i;
> + SpiceTimer *timer = g_new0(SpiceTimer, 1);
> +
> + timer->func = func;
> + timer->opaque = opaque;
> +
> + return timer;
> +}
> +
> +static gboolean worker_timer_func(gpointer user_data)
> +{
> + SpiceTimer *timer = user_data;
> +
> + timer->source_id = 0;
> + timer->func(timer->opaque);
> + /* timer might be free after func(), don't touch */
> +
> + return FALSE;
> +}
>
> - if (!watch) {
> +static void worker_timer_cancel(SpiceTimer *timer)
> +{
> + if (timer->source_id == 0)
> return;
> - }
>
> - worker = watch->worker;
> - i = watch - worker->watches;
> + g_source_remove(timer->source_id);
> + timer->source_id = 0;
> +}
>
> - worker->poll_fds[i].events = 0;
> - if (event_mask & SPICE_WATCH_EVENT_READ) {
> - worker->poll_fds[i].events |= POLLIN;
> - }
> - if (event_mask & SPICE_WATCH_EVENT_WRITE) {
> - worker->poll_fds[i].events |= POLLOUT;
> +static void worker_timer_start(SpiceTimer *timer, uint32_t ms)
> +{
> + worker_timer_cancel(timer);
> +
> + timer->source_id = g_timeout_add(ms, worker_timer_func, timer);
> +}
> +
> +static void worker_timer_remove(SpiceTimer *timer)
> +{
> + worker_timer_cancel(timer);
> + g_free(timer);
> +}
> +
> +static GIOCondition spice_event_to_giocondition(int event_mask)
> +{
> + GIOCondition condition = 0;
> +
> + if (event_mask & SPICE_WATCH_EVENT_READ)
> + condition |= G_IO_IN;
> + if (event_mask & SPICE_WATCH_EVENT_WRITE)
> + condition |= G_IO_OUT;
> +
> + return condition;
> +}
> +
> +static int giocondition_to_spice_event(GIOCondition condition)
> +{
> + int event = 0;
> +
> + if (condition & G_IO_IN)
> + event |= SPICE_WATCH_EVENT_READ;
> + if (condition & G_IO_OUT)
> + event |= SPICE_WATCH_EVENT_WRITE;
> +
> + return event;
> +}
> +
> +struct SpiceWatch {
> + GIOChannel *channel;
> + GSource *source;
> + RedChannelClient *rcc;
> + SpiceWatchFunc func;
> +};
> +
> +static gboolean watch_func(GIOChannel *source, GIOCondition condition,
> + gpointer data)
> +{
> + SpiceWatch *watch = data;
> + int fd = g_io_channel_unix_get_fd(source);
> +
> + watch->func(fd, giocondition_to_spice_event(condition), watch->rcc);
> +
> + return TRUE;
> +}
> +
> +static void worker_watch_update_mask(SpiceWatch *watch, int events)
> +{
> + RedWorker *worker;
> +
> + spice_return_if_fail(watch != NULL);
> + worker = SPICE_CONTAINEROF(watch->rcc->channel, CommonChannel, base)
> ->worker;
> +
> + if (watch->source) {
> + g_source_destroy(watch->source);
> + watch->source = NULL;
> }
> +
> + if (!events)
> + return;
> +
> + watch->source = g_io_create_watch(watch->channel,
> spice_event_to_giocondition(events));
> + g_source_set_callback(watch->source, (GSourceFunc)watch_func, watch,
> NULL);
> + g_source_attach(watch->source, worker->main_context);
> }
>
> -static SpiceWatch *worker_watch_add(int fd, int event_mask, SpiceWatchFunc
> func, void *opaque)
> +static SpiceWatch* worker_watch_add(int fd, int events, SpiceWatchFunc func,
> void *opaque)
> {
> - /* Since we are a channel core implementation, we always get called from
> - red_channel_client_create(), so opaque always is our rcc */
(repeating my comment from the previous review):
I think I would leave this comment in. It feels a bit fragile to make this
assumption in the first place, so I'd at least like a comment explaining why we
can assume this.
> RedChannelClient *rcc = opaque;
> - struct RedWorker *worker;
> - int i;
> + RedWorker *worker;
> + SpiceWatch *watch;
> +
> + spice_return_val_if_fail(rcc != NULL, NULL);
> + spice_return_val_if_fail(fd != -1, NULL);
> + spice_return_val_if_fail(func != NULL, NULL);
>
> /* Since we are called from red_channel_client_create()
> CommonChannelClient->worker has not been set yet! */
> worker = SPICE_CONTAINEROF(rcc->channel, CommonChannel, base)->worker;
> + spice_return_val_if_fail(worker != NULL, NULL);
> + spice_return_val_if_fail(worker->main_context != NULL, NULL);
>
> - /* Search for a free slot in our poll_fds & watches arrays */
> - for (i = 0; i < MAX_EVENT_SOURCES; i++) {
> - if (worker->poll_fds[i].fd == -1) {
> - break;
> - }
> - }
> - if (i == MAX_EVENT_SOURCES) {
> - spice_warning("could not add a watch for channel type %u id %u",
> - rcc->channel->type, rcc->channel->id);
> - return NULL;
> - }
> + watch = g_new0(SpiceWatch, 1);
> + watch->channel = g_io_channel_unix_new(fd);
> + watch->rcc = rcc;
> + watch->func = func;
>
> - worker->poll_fds[i].fd = fd;
> - worker->watches[i].worker = worker;
> - worker->watches[i].watch_func = func;
> - worker->watches[i].watch_func_opaque = opaque;
> - worker_watch_update_mask(&worker->watches[i], event_mask);
> + worker_watch_update_mask(watch, events);
>
> - return &worker->watches[i];
> + return watch;
> }
>
> static void worker_watch_remove(SpiceWatch *watch)
> {
> - if (!watch) {
> - return;
> - }
> + spice_return_if_fail(watch != NULL);
>
> - /* Note we don't touch the poll_fd here, to avoid the
> - poll_fds/watches table entry getting re-used in the same
> - red_worker_main loop over the fds as it is removed.
> + if (watch->source)
> + g_source_destroy(watch->source);
>
> - This is done because re-using it while events were pending on
> - the fd previously occupying the slot would lead to incorrectly
> - calling the watch_func for the new fd. */
> - memset(watch, 0, sizeof(SpiceWatch));
> + g_io_channel_unref(watch->channel);
> + g_free(watch);
> }
>
> -SpiceCoreInterface worker_core = {
> - .timer_add = spice_timer_queue_add,
> - .timer_start = spice_timer_set,
> - .timer_cancel = spice_timer_cancel,
> - .timer_remove = spice_timer_remove,
> +static const SpiceCoreInterface worker_core = {
> + .timer_add = worker_timer_add,
> + .timer_start = worker_timer_start,
> + .timer_cancel = worker_timer_cancel,
> + .timer_remove = worker_timer_remove,
>
> .watch_update_mask = worker_watch_update_mask,
> .watch_add = worker_watch_add,
> @@ -1527,24 +1604,91 @@ static void register_callbacks(Dispatcher *dispatcher)
>
>
>
> -static void handle_dev_input(int fd, int event, void *opaque)
> +static gboolean worker_dispatcher_cb(GIOChannel *source, GIOCondition
> condition,
> + gpointer data)
> {
> - RedWorker *worker = opaque;
> + RedWorker *worker = data;
>
> + spice_debug(NULL);
Remove this debug output
> dispatcher_handle_recv_read(red_dispatcher_get_dispatcher(worker
> ->red_dispatcher));
> +
> + return TRUE;
> +}
> +
> +typedef struct RedWorkerSource {
> + GSource source;
> + RedWorker *worker;
> +} RedWorkerSource;
> +
> +static gboolean worker_source_prepare(GSource *source, gint *p_timeout)
> +{
> + RedWorkerSource *wsource = (RedWorkerSource *)source;
> + RedWorker *worker = wsource->worker;
> + unsigned int timeout;
> +
> + timeout = MIN(worker->event_timeout,
> + display_channel_get_streams_timeout(worker
> ->display_channel));
> +
> + *p_timeout = (timeout == INF_EVENT_WAIT) ? -1 : timeout;
> + if (*p_timeout == 0)
> + return TRUE;
> +
> + return FALSE; /* do no timeout poll */
(repeating comment from previous review:)
This comment is confusing. it implies that returning FALSE means that we don't
poll? In fact, returning TRUE from the prepare function indicates that the
source is already ready and we don't need to poll...
> +}
> +
> +static gboolean worker_source_check(GSource *source)
> +{
> + RedWorkerSource *wsource = (RedWorkerSource *)source;
> + RedWorker *worker = wsource->worker;
> +
> + return worker->running /* TODO && worker->pending_process */;
> +}
> +
> +static gboolean worker_source_dispatch(GSource *source, GSourceFunc callback,
> + gpointer user_data)
> +{
> + RedWorkerSource *wsource = (RedWorkerSource *)source;
> + RedWorker *worker = wsource->worker;
> + DisplayChannel *display = worker->display_channel;
> + int ring_is_empty;
> +
> + /* during migration, in the dest, the display channel can be initialized
> + while the global lz data not since migrate data msg hasn't been
> + received yet */
> + /* FIXME: why is this here, and not in display_channel_create */
> + display_channel_free_glz_drawables_to_free(display);
(repeating comment from previous review):
In the previous version, this call was inside of a 'if (worker
->display_channel)' branch. Here that is not checked.
> +
> + /* FIXME: could use its own source */
> + stream_timeout(display);
> +
> + worker->event_timeout = INF_EVENT_WAIT;
> + red_process_cursor(worker, MAX_PIPE_SIZE, &ring_is_empty);
> + red_process_display(worker, MAX_PIPE_SIZE, &ring_is_empty);
> +
> + /* FIXME: remove me? that should be handled by watch out condition */
> + red_push(worker);
> +
> + return TRUE;
> }
>
> +/* cannot be const */
> +static GSourceFuncs worker_source_funcs = {
> + .prepare = worker_source_prepare,
> + .check = worker_source_check,
> + .dispatch = worker_source_dispatch,
> +};
> +
> RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher)
> {
> QXLDevInitInfo init_info;
> RedWorker *worker;
> Dispatcher *dispatcher;
> - int i;
> const char *record_filename;
>
> qxl->st->qif->get_init_info(qxl, &init_info);
>
> worker = spice_new0(RedWorker, 1);
> + worker->main_context = g_main_context_new();
>
> record_filename = getenv("SPICE_WORKER_RECORD_FILENAME");
> if (record_filename) {
> @@ -1578,15 +1722,18 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> RedDispatcher *red_dispatcher)
> worker->wakeup_counter = stat_add_counter(worker->stat, "wakeups", TRUE);
> worker->command_counter = stat_add_counter(worker->stat, "commands",
> TRUE);
> #endif
> - for (i = 0; i < MAX_EVENT_SOURCES; i++) {
> - worker->poll_fds[i].fd = -1;
> - }
>
> - 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;
> - worker->watches[0].watch_func_opaque = worker;
> + GIOChannel *channel =
> g_io_channel_unix_new(dispatcher_get_recv_fd(dispatcher));
> + GSource *source = g_io_create_watch(channel, G_IO_IN);
> + g_source_set_callback(source, (GSourceFunc)worker_dispatcher_cb, worker,
> NULL);
> + g_source_attach(source, worker->main_context);
> + g_source_unref(source);
> +
> + source = g_source_new(&worker_source_funcs, sizeof(RedWorkerSource));
> + RedWorkerSource *wsource = (RedWorkerSource *)source;
> + wsource->worker = worker;
> + g_source_attach(source, worker->main_context);
> + g_source_unref(source);
>
> memslot_info_init(&worker->mem_slots,
> init_info.num_memslots_groups,
> @@ -1615,10 +1762,6 @@ SPICE_GNUC_NORETURN static void *red_worker_main(void
> *arg)
> spice_assert(MAX_PIPE_SIZE > WIDE_CLIENT_ACK_WINDOW &&
> MAX_PIPE_SIZE > NARROW_CLIENT_ACK_WINDOW); //ensure wakeup by ack
> message
>
> - if (!spice_timer_queue_create()) {
> - spice_error("failed to create timer queue");
> - }
> -
> if (pthread_getcpuclockid(pthread_self(), &worker->clockid)) {
> spice_warning("getcpuclockid failed");
> }
> @@ -1626,66 +1769,12 @@ SPICE_GNUC_NORETURN static void *red_worker_main(void
> *arg)
> 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;
> -
> - timeout = spice_timer_queue_get_timeout_ms();
> - worker->event_timeout = MIN(timeout, worker->event_timeout);
> - timeout = display_channel_get_streams_timeout(worker
> ->display_channel);
> - worker->event_timeout = MIN(timeout, worker->event_timeout);
> - num_events = poll(worker->poll_fds, MAX_EVENT_SOURCES, worker
> ->event_timeout);
> - stream_timeout(worker->display_channel);
> - spice_timer_queue_cb();
> -
> - if (worker->display_channel) {
> - /* during migration, in the dest, the display channel can be
> initialized
> - while the global lz data not since migrate data msg hasn't
> been
> - received yet */
> - display_channel_free_glz_drawables_to_free(worker
> ->display_channel);
> - }
> -
> - worker->event_timeout = INF_EVENT_WAIT;
> - if (num_events == -1) {
> - if (errno != EINTR) {
> - spice_error("poll failed, %s", strerror(errno));
> - }
> - }
> -
> - for (i = 0; i < MAX_EVENT_SOURCES; i++) {
> - /* The watch may have been removed by the watch-func from
> - another fd (ie a disconnect through the dispatcher),
> - in this case watch_func is NULL. */
> - if (worker->poll_fds[i].revents && worker->watches[i].watch_func)
> {
> - int events = 0;
> - if (worker->poll_fds[i].revents & POLLIN) {
> - events |= SPICE_WATCH_EVENT_READ;
> - }
> - if (worker->poll_fds[i].revents & POLLOUT) {
> - events |= SPICE_WATCH_EVENT_WRITE;
> - }
> - worker->watches[i].watch_func(worker->poll_fds[i].fd, events,
> - worker
> ->watches[i].watch_func_opaque);
> - }
> - }
> -
> - /* Clear the poll_fd for any removed watches, see the comment in
> - watch_remove for why we don't do this there. */
> - for (i = 0; i < MAX_EVENT_SOURCES; i++) {
> - if (!worker->watches[i].watch_func) {
> - worker->poll_fds[i].fd = -1;
> - }
> - }
> -
> - if (worker->running) {
> - int ring_is_empty;
> - red_process_cursor(worker, MAX_PIPE_SIZE, &ring_is_empty);
> - red_process_display(worker, MAX_PIPE_SIZE, &ring_is_empty);
> - }
> - red_push(worker);
> - }
> + GMainLoop *loop = g_main_loop_new(worker->main_context, FALSE);
> + g_main_loop_run(loop);
> + g_main_loop_unref(loop);
>
> - spice_warn_if_reached();
> + /* FIXME: free worker, and join threads */
> + abort();
why abort()?
> }
>
> bool red_worker_run(RedWorker *worker)
> diff --git a/server/red-worker.h b/server/red-worker.h
> index 44f35f7..b55a45c 100644
> --- a/server/red-worker.h
> +++ b/server/red-worker.h
> @@ -97,6 +97,7 @@ 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);
> +GMainContext* red_worker_get_context(RedWorker *worker);
> clockid_t red_worker_get_clockid(RedWorker *worker);
> RedMemSlotInfo* red_worker_get_memslot(RedWorker *worker);
>
> diff --git a/server/spice_timer_queue.c b/server/spice_timer_queue.c
> deleted file mode 100644
> index 60017cc..0000000
> --- a/server/spice_timer_queue.c
> +++ /dev/null
> @@ -1,273 +0,0 @@
> -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> -/*
> - Copyright (C) 2013 Red Hat, Inc.
> -
> - This library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - This library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
> -*/
> -#include <config.h>
> -#include <pthread.h>
> -#include "red-common.h"
> -#include "spice_timer_queue.h"
> -#include "common/ring.h"
> -
> -static Ring timer_queue_list;
> -static int queue_count = 0;
> -static pthread_mutex_t queue_list_lock = PTHREAD_MUTEX_INITIALIZER;
> -
> -static void spice_timer_queue_init(void)
> -{
> - ring_init(&timer_queue_list);
> -}
> -
> -struct SpiceTimer {
> - RingItem link;
> - RingItem active_link;
> -
> - SpiceTimerFunc func;
> - void *opaque;
> -
> - SpiceTimerQueue *queue;
> -
> - int is_active;
> - uint32_t ms;
> - uint64_t expiry_time;
> -};
> -
> -struct SpiceTimerQueue {
> - RingItem link;
> - pthread_t thread;
> - Ring timers;
> - Ring active_timers;
> -};
> -
> -static SpiceTimerQueue *spice_timer_queue_find(void)
> -{
> - pthread_t self = pthread_self();
> - RingItem *queue_item;
> -
> - RING_FOREACH(queue_item, &timer_queue_list) {
> - SpiceTimerQueue *queue = SPICE_CONTAINEROF(queue_item,
> SpiceTimerQueue, link);
> -
> - if (pthread_equal(self, queue->thread) != 0) {
> - return queue;
> - }
> - }
> -
> - return NULL;
> -}
> -
> -static SpiceTimerQueue *spice_timer_queue_find_with_lock(void)
> -{
> - SpiceTimerQueue *queue;
> -
> - pthread_mutex_lock(&queue_list_lock);
> - queue = spice_timer_queue_find();
> - pthread_mutex_unlock(&queue_list_lock);
> - return queue;
> -}
> -
> -int spice_timer_queue_create(void)
> -{
> - SpiceTimerQueue *queue;
> -
> - pthread_mutex_lock(&queue_list_lock);
> - if (queue_count == 0) {
> - spice_timer_queue_init();
> - }
> -
> - if (spice_timer_queue_find() != NULL) {
> - spice_printerr("timer queue was already created for the thread");
> - return FALSE;
> - }
> -
> - queue = spice_new0(SpiceTimerQueue, 1);
> - queue->thread = pthread_self();
> - ring_init(&queue->timers);
> - ring_init(&queue->active_timers);
> -
> - ring_add(&timer_queue_list, &queue->link);
> - queue_count++;
> -
> - pthread_mutex_unlock(&queue_list_lock);
> -
> - return TRUE;
> -}
> -
> -void spice_timer_queue_destroy(void)
> -{
> - RingItem *item;
> - SpiceTimerQueue *queue;
> -
> - pthread_mutex_lock(&queue_list_lock);
> - queue = spice_timer_queue_find();
> -
> - spice_assert(queue != NULL);
> -
> - while ((item = ring_get_head(&queue->timers))) {
> - SpiceTimer *timer;
> -
> - timer = SPICE_CONTAINEROF(item, SpiceTimer, link);
> - spice_timer_remove(timer);
> - }
> -
> - ring_remove(&queue->link);
> - free(queue);
> - queue_count--;
> -
> - pthread_mutex_unlock(&queue_list_lock);
> -}
> -
> -SpiceTimer *spice_timer_queue_add(SpiceTimerFunc func, void *opaque)
> -{
> - SpiceTimer *timer = spice_new0(SpiceTimer, 1);
> - SpiceTimerQueue *queue = spice_timer_queue_find_with_lock();
> -
> - spice_assert(queue != NULL);
> -
> - ring_item_init(&timer->link);
> - ring_item_init(&timer->active_link);
> -
> - timer->opaque = opaque;
> - timer->func = func;
> - timer->queue = queue;
> -
> - ring_add(&queue->timers, &timer->link);
> -
> - return timer;
> -}
> -
> -static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint64_t now)
> -{
> - RingItem *next_item;
> - SpiceTimerQueue *queue;
> -
> - if (timer->is_active) {
> - spice_timer_cancel(timer);
> - }
> -
> - queue = timer->queue;
> - timer->expiry_time = now + ms;
> - timer->ms = ms;
> -
> - RING_FOREACH(next_item, &queue->active_timers) {
> - SpiceTimer *next_timer = SPICE_CONTAINEROF(next_item, SpiceTimer,
> active_link);
> -
> - if (timer->expiry_time <= next_timer->expiry_time) {
> - break;
> - }
> - }
> -
> - if (next_item) {
> - ring_add_before(&timer->active_link, next_item);
> - } else {
> - ring_add_before(&timer->active_link, &queue->active_timers);
> - }
> - timer->is_active = TRUE;
> -}
> -
> -void spice_timer_set(SpiceTimer *timer, uint32_t ms)
> -{
> - struct timespec now;
> -
> - spice_assert(pthread_equal(timer->queue->thread, pthread_self()) != 0);
> -
> - clock_gettime(CLOCK_MONOTONIC, &now);
> - _spice_timer_set(timer, ms,
> - (uint64_t)now.tv_sec * 1000 + (now.tv_nsec / 1000 /
> 1000));
> -}
> -
> -void spice_timer_cancel(SpiceTimer *timer)
> -{
> - spice_assert(pthread_equal(timer->queue->thread, pthread_self()) != 0);
> -
> - if (!ring_item_is_linked(&timer->active_link)) {
> - spice_assert(!timer->is_active);
> - return;
> - }
> -
> - spice_assert(timer->is_active);
> - ring_remove(&timer->active_link);
> - timer->is_active = FALSE;
> -}
> -
> -void spice_timer_remove(SpiceTimer *timer)
> -{
> - spice_assert(timer->queue);
> - spice_assert(ring_item_is_linked(&timer->link));
> - spice_assert(pthread_equal(timer->queue->thread, pthread_self()) != 0);
> -
> - if (timer->is_active) {
> - spice_assert(ring_item_is_linked(&timer->active_link));
> - ring_remove(&timer->active_link);
> - }
> - ring_remove(&timer->link);
> - free(timer);
> -}
> -
> -unsigned int spice_timer_queue_get_timeout_ms(void)
> -{
> - struct timespec now;
> - int64_t now_ms;
> - RingItem *head;
> - SpiceTimer *head_timer;
> - SpiceTimerQueue *queue = spice_timer_queue_find_with_lock();
> -
> - spice_assert(queue != NULL);
> -
> - if (ring_is_empty(&queue->active_timers)) {
> - return -1;
> - }
> -
> - head = ring_get_head(&queue->active_timers);
> - head_timer = SPICE_CONTAINEROF(head, SpiceTimer, active_link);
> -
> - clock_gettime(CLOCK_MONOTONIC, &now);
> - now_ms = ((int64_t)now.tv_sec * 1000) + (now.tv_nsec / 1000 / 1000);
> -
> - return MAX(0, ((int64_t)head_timer->expiry_time - now_ms));
> -}
> -
> -
> -void spice_timer_queue_cb(void)
> -{
> - struct timespec now;
> - uint64_t now_ms;
> - RingItem *head;
> - SpiceTimerQueue *queue = spice_timer_queue_find_with_lock();
> -
> - spice_assert(queue != NULL);
> -
> - if (ring_is_empty(&queue->active_timers)) {
> - return;
> - }
> -
> - clock_gettime(CLOCK_MONOTONIC, &now);
> - now_ms = ((uint64_t)now.tv_sec * 1000) + (now.tv_nsec / 1000 / 1000);
> -
> - while ((head = ring_get_head(&queue->active_timers))) {
> - SpiceTimer *timer = SPICE_CONTAINEROF(head, SpiceTimer, active_link);
> -
> - if (timer->expiry_time > now_ms) {
> - break;
> - } else {
> - /* Remove active timer before calling the timer function.
> - * Timer function could delete the timer making the timer
> - * pointer point to freed data.
> - */
> - spice_timer_cancel(timer);
> - timer->func(timer->opaque);
> - /* timer could now be invalid ! */
> - }
> - }
> -}
> diff --git a/server/spice_timer_queue.h b/server/spice_timer_queue.h
> deleted file mode 100644
> index a84f6cd..0000000
> --- a/server/spice_timer_queue.h
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -/*
> - Copyright (C) 2013 Red Hat, Inc.
> -
> - This library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - This library is distributed in the hope that it will be useful,
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with this library; if not, see <http://www.gnu.org/licenses/
> >.
> -*/
> -
> -#ifndef _H_SPICE_TIMER_QUEUE
> -#define _H_SPICE_TIMER_QUEUE
> -
> -#include <stdint.h>
> -#include "spice.h"
> -
> -typedef struct SpiceTimerQueue SpiceTimerQueue;
> -
> -/* create/destroy a timer queue for the current thread.
> - * In order to execute the timers functions, spice_timer_queue_cb should be
> called
> - * periodically, according to spice_timer_queue_get_timeout_ms */
> -int spice_timer_queue_create(void);
> -void spice_timer_queue_destroy(void);
> -
> -SpiceTimer *spice_timer_queue_add(SpiceTimerFunc func, void *opaque);
> -void spice_timer_set(SpiceTimer *timer, uint32_t ms);
> -void spice_timer_cancel(SpiceTimer *timer);
> -void spice_timer_remove(SpiceTimer *timer);
> -
> -/* returns the time left till the earliest timer in the queue expires.
> - * returns (unsigned)-1 if there are no active timers */
> -unsigned int spice_timer_queue_get_timeout_ms(void);
> -/* call the timeout callbacks of all the expired timers */
> -void spice_timer_queue_cb(void);
> -
> -#endif
Reviewed-by: Jonathon Jongsma <jjonsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel