This patch try to make sure that CursorChannel and DisplayChannel don't know RedWorker existence. Definition common to CursorChannel and DisplayChannel are moved to a new common-channel.h header. RedWorker pointers are removed from CommonChannel and CommonChannelClient. red_drawable_ref declaration is moved to display-channel.h and accept a DisplayChannel* instead of a RedWorker* (perhaps should be moved to display-channel.c too with red_drawable_new). As a small hack red_worker_new_channel is renamed to common_channel_new accepting a pointer to a structure with finally is a RedWorker*. A good thing you can see is that red_worker_get_qxl and red_worker_get_memslot went away. There are only 4 function declaration for RedWorker (red-worker.h) and I already have a patch to remove the red_worker_get_* ones. Result is that only RedDispatcher know of the existence of RedWorker! Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- server/Makefile.am | 1 + server/common-channel.h | 107 +++++++++++++++++++++++++++++++++++++++++++++++ server/cursor-channel.c | 10 ++--- server/cursor-channel.h | 4 +- server/dcc-encoders.c | 3 +- server/dcc.c | 2 +- server/dcc.h | 4 +- server/display-channel.c | 15 +++---- server/display-channel.h | 12 ++++-- server/red-record-qxl.c | 1 - server/red-replay-qxl.c | 2 +- server/red-worker.c | 44 +++++++------------ server/red-worker.h | 91 +--------------------------------------- 13 files changed, 152 insertions(+), 144 deletions(-) create mode 100644 server/common-channel.h diff --git a/server/Makefile.am b/server/Makefile.am index 92b716f..feb69c5 100644 --- a/server/Makefile.am +++ b/server/Makefile.am @@ -106,6 +106,7 @@ libserver_la_SOURCES = \ red-parse-qxl.h \ red-worker.c \ red-worker.h \ + common-channel.h \ display-channel.c \ display-channel.h \ cursor-channel.c \ diff --git a/server/common-channel.h b/server/common-channel.h new file mode 100644 index 0000000..5bd85d3 --- /dev/null +++ b/server/common-channel.h @@ -0,0 +1,107 @@ +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ +/* + Copyright (C) 2009-2015 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 COMMON_CHANNEL_H_ +#define COMMON_CHANNEL_H_ + +typedef struct CommonChannelClient { + RedChannelClient base; + + uint32_t id; + int is_low_bandwidth; +} CommonChannelClient; + +#define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client)) +#define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30) + +#define CHANNEL_RECEIVE_BUF_SIZE 1024 +typedef struct CommonChannel { + RedChannel base; // Must be the first thing + + QXLInstance *qxl; + uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE]; + uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme. + int during_target_migrate; /* TRUE when the client that is associated with the channel + is during migration. Turned off when the vm is started. + The flag is used to avoid sending messages that are artifacts + of the transition from stopped vm to loaded vm (e.g., recreation + of the primary surface) */ +} CommonChannel; + +#define COMMON_CHANNEL(Channel) ((CommonChannel*)(Channel)) + +enum { + PIPE_ITEM_TYPE_VERB = PIPE_ITEM_TYPE_CHANNEL_BASE, + PIPE_ITEM_TYPE_INVAL_ONE, + + PIPE_ITEM_TYPE_COMMON_LAST +}; + +typedef struct VerbItem { + PipeItem base; + uint16_t verb; +} VerbItem; + +static inline void red_marshall_verb(RedChannelClient *rcc, VerbItem *item) +{ + red_channel_client_init_send_data(rcc, item->verb, NULL); +} + +static inline void red_pipe_add_verb(RedChannelClient* rcc, uint16_t verb) +{ + VerbItem *item = spice_new(VerbItem, 1); + + red_channel_pipe_item_init(rcc->channel, &item->base, PIPE_ITEM_TYPE_VERB); + item->verb = verb; + red_channel_client_pipe_add(rcc, &item->base); +} + +#define LINK_TO_RCC(ptr) SPICE_CONTAINEROF(ptr, RedChannelClient, channel_link) +#define RCC_FOREACH_SAFE(link, next, rcc, channel) \ + SAFE_FOREACH(link, next, channel, &(channel)->clients, rcc, LINK_TO_RCC(link)) + + +static inline void red_pipes_add_verb(RedChannel *channel, uint16_t verb) +{ + RedChannelClient *rcc; + RingItem *link, *next; + + RCC_FOREACH_SAFE(link, next, rcc, channel) { + red_pipe_add_verb(rcc, verb); + } +} + +typedef struct CommonChannelNewParam CommonChannelNewParam; + +CommonChannel *common_channel_new(CommonChannelNewParam *param, int size, + const char *name, + uint32_t channel_type, int migration_flags, + ChannelCbs *channel_cbs, + channel_handle_parsed_proc handle_parsed); + +CommonChannelClient *common_channel_new_client(CommonChannel *common, + int size, + RedClient *client, + RedsStream *stream, + int mig_target, + int monitor_latency, + uint32_t *common_caps, + int num_common_caps, + uint32_t *caps, + int num_caps); + +#endif diff --git a/server/cursor-channel.c b/server/cursor-channel.c index 4c15582..917cf4b 100644 --- a/server/cursor-channel.c +++ b/server/cursor-channel.c @@ -424,7 +424,7 @@ static void cursor_channel_release_item(RedChannelClient *rcc, PipeItem *item, i } } -CursorChannel* cursor_channel_new(RedWorker *worker) +CursorChannel* cursor_channel_new(CommonChannelNewParam *param) { CursorChannel *cursor_channel; CommonChannel *channel = NULL; @@ -436,9 +436,9 @@ CursorChannel* cursor_channel_new(RedWorker *worker) }; spice_info("create cursor channel"); - channel = red_worker_new_channel(worker, sizeof(CursorChannel), "cursor_channel", - SPICE_CHANNEL_CURSOR, 0, - &cbs, red_channel_client_handle_message); + channel = common_channel_new(param, sizeof(CursorChannel), "cursor_channel", + SPICE_CHANNEL_CURSOR, 0, + &cbs, red_channel_client_handle_message); cursor_channel = (CursorChannel *)channel; cursor_channel->cursor_visible = TRUE; @@ -496,7 +496,7 @@ void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd, spice_return_if_fail(cursor); spice_return_if_fail(cursor_cmd); - cursor_item = cursor_item_new(red_worker_get_qxl(cursor->common.worker), + cursor_item = cursor_item_new(cursor->common.qxl, cursor_cmd, group_id); switch (cursor_cmd->type) { diff --git a/server/cursor-channel.h b/server/cursor-channel.h index 104af20..c4381c7 100644 --- a/server/cursor-channel.h +++ b/server/cursor-channel.h @@ -20,7 +20,7 @@ #include "spice.h" #include "reds.h" -#include "red-worker.h" +#include "common-channel.h" #include "red-parse-qxl.h" #include "cache-item.h" #include "stat.h" @@ -30,7 +30,7 @@ typedef struct CursorChannelClient CursorChannelClient; #define CURSOR_CHANNEL_CLIENT(Client) ((CursorChannelClient*)(Client)) -CursorChannel* cursor_channel_new (RedWorker *worker); +CursorChannel* cursor_channel_new (CommonChannelNewParam *param); void cursor_channel_disconnect (CursorChannel *cursor_channel); void cursor_channel_reset (CursorChannel *cursor); void cursor_channel_init (CursorChannel *cursor, CursorChannelClient* client); diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c index 0923c9c..c810b8e 100644 --- a/server/dcc-encoders.c +++ b/server/dcc-encoders.c @@ -456,7 +456,6 @@ void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc, GlzDrawableInstanceItem *instance) { DisplayChannel *display_channel = DCC_TO_DC(dcc); - RedWorker *worker = display_channel->common.worker; RedGlzDrawable *glz_drawable; spice_assert(instance); @@ -484,7 +483,7 @@ void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc, if (drawable) { ring_remove(&glz_drawable->drawable_link); } - red_drawable_unref(worker, glz_drawable->red_drawable, + red_drawable_unref(display_channel, glz_drawable->red_drawable, glz_drawable->group_id); display_channel->glz_drawable_count--; if (ring_item_is_linked(&glz_drawable->link)) { diff --git a/server/dcc.c b/server/dcc.c index bf692f8..3f02a98 100644 --- a/server/dcc.c +++ b/server/dcc.c @@ -1085,7 +1085,7 @@ int dcc_compress_image(DisplayChannelClient *dcc, if (drawable != NULL) { group_id = drawable->group_id; } else { - group_id = red_worker_get_memslot(display_channel->common.worker)->internal_groupslot_id; + group_id = display_channel->groupslot_id; } if (quic_compress) { diff --git a/server/dcc.h b/server/dcc.h index f715792..e4592b4 100644 --- a/server/dcc.h +++ b/server/dcc.h @@ -16,14 +16,14 @@ License along with this library; if not, see <http://www.gnu.org/licenses/>. */ #ifndef DCC_H_ -# define DCC_H_ +#define DCC_H_ -#include "red-worker.h" #include "pixmap-cache.h" #include "cache-item.h" #include "dcc-encoders.h" #include "stream.h" #include "display-limits.h" +#include "common-channel.h" #define PALETTE_CACHE_HASH_SHIFT 8 #define PALETTE_CACHE_HASH_SIZE (1 << PALETTE_CACHE_HASH_SHIFT) diff --git a/server/display-channel.c b/server/display-channel.c index f0d133a..12f44e9 100644 --- a/server/display-channel.c +++ b/server/display-channel.c @@ -241,8 +241,7 @@ static void stop_streams(DisplayChannel *display) void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id) { RedSurface *surface = &display->surfaces[surface_id]; - RedWorker *worker = COMMON_CHANNEL(display)->worker; - QXLInstance *qxl = red_worker_get_qxl(worker); + QXLInstance *qxl = display->common.qxl; DisplayChannelClient *dcc; RingItem *link, *next; @@ -1445,7 +1444,7 @@ void display_channel_drawable_unref(DisplayChannel *display, Drawable *drawable) ring_remove(item); } if (drawable->red_drawable) { - red_drawable_unref(COMMON_CHANNEL(display)->worker, drawable->red_drawable, drawable->group_id); + red_drawable_unref(display, drawable->red_drawable, drawable->group_id); } drawable_free(display, drawable); display->drawable_count--; @@ -2014,8 +2013,8 @@ static SpiceCanvas *image_surfaces_get(SpiceImageSurfaces *surfaces, uint32_t su return display->surfaces[surface_id].context.canvas; } -DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int stream_video, - uint32_t n_surfaces) +DisplayChannel* display_channel_new(CommonChannelNewParam *param, int migrate, int stream_video, + uint32_t n_surfaces, uint32_t groupslot_id) { DisplayChannel *display; ChannelCbs cbs = { @@ -2034,13 +2033,15 @@ DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int stream_v spice_return_val_if_fail(num_renderers > 0, NULL); spice_info("create display channel"); - display = (DisplayChannel *)red_worker_new_channel( - worker, sizeof(*display), "display_channel", + display = (DisplayChannel *)common_channel_new( + param, sizeof(*display), "display_channel", SPICE_CHANNEL_DISPLAY, SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER, &cbs, dcc_handle_message); spice_return_val_if_fail(display, NULL); + display->groupslot_id = groupslot_id; + clockid_t stat_clock = CLOCK_THREAD_CPUTIME_ID; stat_init(&display->add_stat, "add", stat_clock); stat_init(&display->exclude_stat, "exclude", stat_clock); diff --git a/server/display-channel.h b/server/display-channel.h index bf29cd3..73289f2 100644 --- a/server/display-channel.h +++ b/server/display-channel.h @@ -21,7 +21,6 @@ #include <setjmp.h> #include "common/rect.h" -#include "red-worker.h" #include "reds-stream.h" #include "cache-item.h" #include "pixmap-cache.h" @@ -200,6 +199,8 @@ struct DisplayChannel { ImageCache image_cache; RedCompressBuf *free_compress_bufs; + uint32_t groupslot_id; + /* TODO: some day unify this, make it more runtime.. */ stat_info_t add_stat; stat_info_t exclude_stat; @@ -248,11 +249,11 @@ typedef struct UpgradeItem { SpiceClipRects *rects; } UpgradeItem; - -DisplayChannel* display_channel_new (RedWorker *worker, +DisplayChannel* display_channel_new (CommonChannelNewParam *param, int migrate, int stream_video, - uint32_t n_surfaces); + uint32_t n_surfaces, + uint32_t groupslot_id); void display_channel_create_surface (DisplayChannel *display, uint32_t surface_id, uint32_t width, uint32_t height, int32_t stride, uint32_t format, void *line_0, @@ -307,6 +308,9 @@ void display_channel_process_surface_cmd (DisplayCha void display_channel_update_compression (DisplayChannel *display, DisplayChannelClient *dcc); +void red_drawable_unref(DisplayChannel *display, RedDrawable *red_drawable, + uint32_t group_id); + static inline int validate_surface(DisplayChannel *display, uint32_t surface_id) { if SPICE_UNLIKELY(surface_id >= display->n_surfaces) { diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c index 9c9dd62..2791903 100644 --- a/server/red-record-qxl.c +++ b/server/red-record-qxl.c @@ -21,7 +21,6 @@ #include <stdbool.h> #include <inttypes.h> -#include "red-worker.h" #include "red-common.h" #include "memslot.h" #include "red-parse-qxl.h" diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c index 17ee022..0c0515b 100644 --- a/server/red-replay-qxl.c +++ b/server/red-replay-qxl.c @@ -24,7 +24,7 @@ #include <zlib.h> #include <pthread.h> #include "reds.h" -#include "red-worker.h" +#include "red-dispatcher.h" #include "red-common.h" #include "memslot.h" #include "red-parse-qxl.h" diff --git a/server/red-worker.c b/server/red-worker.c index ad8ba1a..c27533f 100644 --- a/server/red-worker.c +++ b/server/red-worker.c @@ -99,20 +99,6 @@ struct RedWorker { FILE *record_fd; }; -QXLInstance* red_worker_get_qxl(RedWorker *worker) -{ - spice_return_val_if_fail(worker != NULL, NULL); - - return worker->qxl; -} - -RedMemSlotInfo* red_worker_get_memslot(RedWorker *worker) -{ - spice_return_val_if_fail(worker != NULL, NULL); - - return &worker->mem_slots; -} - static int display_is_connected(RedWorker *worker) { return (worker->display_channel && red_channel_is_connected( @@ -149,7 +135,7 @@ static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32 } } -void red_drawable_unref(RedWorker *worker, RedDrawable *red_drawable, +void red_drawable_unref(DisplayChannel *display, RedDrawable *red_drawable, uint32_t group_id) { QXLReleaseInfoExt release_info_ext; @@ -157,10 +143,10 @@ void red_drawable_unref(RedWorker *worker, RedDrawable *red_drawable, if (--red_drawable->refs) { return; } - worker->display_channel->red_drawable_count--; + display->red_drawable_count--; release_info_ext.group_id = group_id; release_info_ext.info = red_drawable->release_info; - worker->qxl->st->qif->release_resource(worker->qxl, release_info_ext); + display->common.qxl->st->qif->release_resource(display->common.qxl, release_info_ext); red_put_drawable(red_drawable); free(red_drawable); } @@ -270,7 +256,7 @@ static int red_process_display(RedWorker *worker, uint32_t max_pipe_size, int *r worker->process_display_generation); } // release the red_drawable - red_drawable_unref(worker, red_drawable, ext_cmd.group_id); + red_drawable_unref(worker->display_channel, red_drawable, ext_cmd.group_id); break; } case QXL_CMD_UPDATE: { @@ -606,8 +592,7 @@ CommonChannelClient *common_channel_new_client(CommonChannel *common, return NULL; } CommonChannelClient *common_cc = (CommonChannelClient*)rcc; - common_cc->worker = common->worker; - common_cc->id = common->worker->qxl->id; + common_cc->id = common->qxl->id; common->during_target_migrate = mig_target; // TODO: move wide/narrow ack setting to red_channel. @@ -618,12 +603,13 @@ CommonChannelClient *common_channel_new_client(CommonChannel *common, } -CommonChannel *red_worker_new_channel(RedWorker *worker, int size, - const char *name, - uint32_t channel_type, int migration_flags, - ChannelCbs *channel_cbs, - channel_handle_parsed_proc handle_parsed) +CommonChannel *common_channel_new(CommonChannelNewParam *param, int size, + const char *name, + uint32_t channel_type, int migration_flags, + ChannelCbs *channel_cbs, + channel_handle_parsed_proc handle_parsed) { + RedWorker *worker = (RedWorker *) param; RedChannel *channel = NULL; CommonChannel *common; @@ -648,7 +634,7 @@ CommonChannel *red_worker_new_channel(RedWorker *worker, int size, red_channel_set_stat_node(channel, stat_add_node(worker->stat, name, TRUE)); common = (CommonChannel *)channel; - common->worker = worker; + common->qxl = worker->qxl; return common; } @@ -1597,10 +1583,10 @@ RedWorker* red_worker_new(QXLInstance *qxl, RedDispatcher *red_dispatcher) worker->event_timeout = INF_EVENT_WAIT; - worker->cursor_channel = cursor_channel_new(worker); + worker->cursor_channel = cursor_channel_new((CommonChannelNewParam*) worker); // TODO: handle seemless migration. Temp, setting migrate to FALSE - worker->display_channel = display_channel_new(worker, FALSE, streaming_video, - init_info.n_surfaces); + worker->display_channel = display_channel_new((CommonChannelNewParam*) worker, FALSE, streaming_video, + init_info.n_surfaces, worker->mem_slots.internal_groupslot_id); return worker; } diff --git a/server/red-worker.h b/server/red-worker.h index 91533e1..f6db1d5 100644 --- a/server/red-worker.h +++ b/server/red-worker.h @@ -21,102 +21,13 @@ #include "red-common.h" #include "red-dispatcher.h" #include "red-parse-qxl.h" +#include "common-channel.h" typedef struct RedWorker RedWorker; -typedef struct CommonChannelClient { - RedChannelClient base; - - uint32_t id; - RedWorker *worker; - int is_low_bandwidth; -} CommonChannelClient; - -#define COMMON_CHANNEL_CLIENT(Client) ((CommonChannelClient*)(Client)) -#define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30) - -#define CHANNEL_RECEIVE_BUF_SIZE 1024 -typedef struct CommonChannel { - RedChannel base; // Must be the first thing - - struct RedWorker *worker; - uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE]; - uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme. - int during_target_migrate; /* TRUE when the client that is associated with the channel - is during migration. Turned off when the vm is started. - The flag is used to avoid sending messages that are artifacts - of the transition from stopped vm to loaded vm (e.g., recreation - of the primary surface) */ -} CommonChannel; - -#define COMMON_CHANNEL(Channel) ((CommonChannel*)(Channel)) - -enum { - PIPE_ITEM_TYPE_VERB = PIPE_ITEM_TYPE_CHANNEL_BASE, - PIPE_ITEM_TYPE_INVAL_ONE, - - PIPE_ITEM_TYPE_COMMON_LAST -}; - -typedef struct VerbItem { - PipeItem base; - uint16_t verb; -} VerbItem; - -static inline void red_marshall_verb(RedChannelClient *rcc, VerbItem *item) -{ - red_channel_client_init_send_data(rcc, item->verb, NULL); -} - -static inline void red_pipe_add_verb(RedChannelClient* rcc, uint16_t verb) -{ - VerbItem *item = spice_new(VerbItem, 1); - - red_channel_pipe_item_init(rcc->channel, &item->base, PIPE_ITEM_TYPE_VERB); - item->verb = verb; - red_channel_client_pipe_add(rcc, &item->base); -} - -#define LINK_TO_RCC(ptr) SPICE_CONTAINEROF(ptr, RedChannelClient, channel_link) -#define RCC_FOREACH_SAFE(link, next, rcc, channel) \ - SAFE_FOREACH(link, next, channel, &(channel)->clients, rcc, LINK_TO_RCC(link)) - - -static inline void red_pipes_add_verb(RedChannel *channel, uint16_t verb) -{ - RedChannelClient *rcc; - RingItem *link, *next; - - RCC_FOREACH_SAFE(link, next, rcc, channel) { - red_pipe_add_verb(rcc, 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); -RedMemSlotInfo* red_worker_get_memslot(RedWorker *worker); - -void red_drawable_unref(RedWorker *worker, RedDrawable *red_drawable, - uint32_t group_id); - -CommonChannel *red_worker_new_channel(RedWorker *worker, int size, - const char *name, - uint32_t channel_type, int migration_flags, - ChannelCbs *channel_cbs, - channel_handle_parsed_proc handle_parsed); - -CommonChannelClient *common_channel_new_client(CommonChannel *common, - int size, - RedClient *client, - RedsStream *stream, - int mig_target, - int monitor_latency, - uint32_t *common_caps, - int num_common_caps, - uint32_t *caps, - int num_caps); #endif -- 2.4.3 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel