[Spice-devel] [PATCH spice-server v2 5/8] Add CommonGraphicsChannelPrivate struct
Frediano Ziglio
fziglio at redhat.com
Mon Oct 10 16:20:12 UTC 2016
>
> From: Jonathon Jongsma <jjongsma at redhat.com>
>
> Encapsulate private data for CommonGraphicsChannel and prepare for
> GObject conversion.
This object has all the fields with accessors... not a really private
I would say. Is not possible to implement in a different way using
this structure in CursorChannelPrivate and DisplayChannelPrivate ?
> ---
> server/common-graphics-channel.c | 34 ++++++++++++++++++++++++++++++++--
> server/common-graphics-channel.h | 14 ++++++--------
> server/cursor-channel-client.c | 2 +-
> server/cursor-channel.c | 8 +++++---
> server/dcc-send.c | 2 +-
> server/dcc.c | 8 ++++----
> server/display-channel.c | 6 +++---
> server/red-worker.c | 7 ++++---
> 8 files changed, 56 insertions(+), 25 deletions(-)
>
> diff --git a/server/common-graphics-channel.c
> b/server/common-graphics-channel.c
> index bcf7279..6871540 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -27,6 +27,20 @@
> #include "dcc.h"
> #include "main-channel-client.h"
>
> +#define CHANNEL_RECEIVE_BUF_SIZE 1024
> +
> +struct CommonGraphicsChannelPrivate
> +{
> + QXLInstance *qxl;
> + uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> + uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme.
No reason to introduced again this unused field.
> + 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) */
> +};
> +
> static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type,
> uint32_t size)
> {
> RedChannel *channel = red_channel_client_get_channel(rcc);
> @@ -41,7 +55,7 @@ static uint8_t *common_alloc_recv_buf(RedChannelClient
> *rcc, uint16_t type, uint
> spice_critical("unexpected message size %u (max is %d)", size,
> CHANNEL_RECEIVE_BUF_SIZE);
> return NULL;
> }
> - return common->recv_buf;
> + return common->priv->recv_buf;
> }
>
> static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type,
> uint32_t size,
> @@ -123,7 +137,23 @@ CommonGraphicsChannel*
> common_graphics_channel_new(RedsState *server,
> spice_return_val_if_fail(channel, NULL);
>
> common = COMMON_GRAPHICS_CHANNEL(channel);
> - common->qxl = qxl;
> + common->priv = g_new0(CommonGraphicsChannelPrivate, 1);
> + common->priv->qxl = qxl;
> return common;
> }
>
new and no free, leak... but only till next patch so it's not a big
issue.
> +void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value)
> +{
> + self->priv->during_target_migrate = value;
> +}
> +
> +gboolean
> common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel
> *self)
> +{
> + return self->priv->during_target_migrate;
> +}
> +
This field should really not be in this "private" structure
> +QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self)
> +{
> + return self->priv->qxl;
> +}
> +
> diff --git a/server/common-graphics-channel.h
> b/server/common-graphics-channel.h
> index 7b2aeff..c6c3f48 100644
> --- a/server/common-graphics-channel.h
> +++ b/server/common-graphics-channel.h
> @@ -25,21 +25,19 @@ int common_channel_config_socket(RedChannelClient *rcc);
>
> #define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
>
> -#define CHANNEL_RECEIVE_BUF_SIZE 1024
> +typedef struct CommonGraphicsChannelPrivate CommonGraphicsChannelPrivate;
> typedef struct CommonGraphicsChannel {
> RedChannel base; // Must be the first thing
>
> - QXLInstance *qxl;
> - uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> - 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) */
> + CommonGraphicsChannelPrivate *priv;
> } CommonGraphicsChannel;
>
> #define COMMON_GRAPHICS_CHANNEL(Channel) ((CommonGraphicsChannel*)(Channel))
>
> +void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value);
> +gboolean
> common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel
> *self);
> +QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self);
> +
> enum {
> RED_PIPE_ITEM_TYPE_VERB = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
> RED_PIPE_ITEM_TYPE_INVAL_ONE,
> diff --git a/server/cursor-channel-client.c b/server/cursor-channel-client.c
> index 90778ed..b7ab2e5 100644
> --- a/server/cursor-channel-client.c
> +++ b/server/cursor-channel-client.c
> @@ -124,7 +124,7 @@ CursorChannelClient*
> cursor_channel_client_new(CursorChannel *cursor, RedClient
> "common-caps", common_caps_array,
> "caps", caps_array,
> NULL);
> - COMMON_GRAPHICS_CHANNEL(cursor)->during_target_migrate = mig_target;
> +
> common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor),
> mig_target);
>
> if (caps_array)
> g_array_unref(caps_array);
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index e84b593..f796c8c 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -338,11 +338,13 @@ void cursor_channel_process_cmd(CursorChannel *cursor,
> RedCursorCmd *cursor_cmd)
> {
> CursorItem *cursor_item;
> int cursor_show = FALSE;
> + QXLInstance *qxl = NULL;
>
> spice_return_if_fail(cursor);
> spice_return_if_fail(cursor_cmd);
>
> - cursor_item = cursor_item_new(cursor->common.qxl, cursor_cmd);
> + qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(cursor));
> + cursor_item = cursor_item_new(qxl, cursor_cmd);
>
> switch (cursor_cmd->type) {
> case QXL_CURSOR_SET:
> @@ -390,7 +392,7 @@ void cursor_channel_reset(CursorChannel *cursor)
>
> if (red_channel_is_connected(channel)) {
> red_channel_pipes_add_type(channel,
> RED_PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE);
> - if (!cursor->common.during_target_migrate) {
> + if
> (!common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor)))
> {
> red_pipes_add_verb(channel, SPICE_MSG_CURSOR_RESET);
> }
> if (!red_channel_wait_all_sent(&cursor->common.base,
> @@ -406,7 +408,7 @@ static void cursor_channel_init_client(CursorChannel
> *cursor, CursorChannelClien
> spice_return_if_fail(cursor);
>
> if (!red_channel_is_connected(&cursor->common.base)
> - || COMMON_GRAPHICS_CHANNEL(cursor)->during_target_migrate) {
> + ||
> common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor)))
> {
> spice_debug("during_target_migrate: skip init");
> return;
> }
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index c49adab..e33f428 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -2311,7 +2311,7 @@ static void marshall_gl_scanout(RedChannelClient *rcc,
> {
> DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(rcc);
> DisplayChannel *display_channel = DCC_TO_DC(dcc);
> - QXLInstance* qxl = display_channel->common.qxl;
> + QXLInstance* qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display_channel));
>
> SpiceMsgDisplayGlScanoutUnix *scanout = red_qxl_get_gl_scanout(qxl);
> if (scanout != NULL) {
> diff --git a/server/dcc.c b/server/dcc.c
> index 3ded1b9..021c241 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -284,7 +284,7 @@ void dcc_create_surface(DisplayChannelClient *dcc, int
> surface_id)
> flags = is_primary_surface(DCC_TO_DC(dcc), surface_id) ?
> SPICE_SURFACE_FLAGS_PRIMARY : 0;
>
> /* don't send redundant create surface commands to client */
> - if (!dcc || display->common.during_target_migrate ||
> + if (!dcc ||
> common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
> ||
> dcc->priv->surface_client_created[surface_id]) {
> return;
> }
> @@ -514,8 +514,8 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
> "zlib-glz-state", zlib_glz_state,
> NULL);
> spice_info("New display (client %p) dcc %p stream %p", client, dcc,
> stream);
> - display->common.during_target_migrate = mig_target;
> - dcc->priv->id = display->common.qxl->id;
> +
> common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display),
> mig_target);
> + dcc->priv->id =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display))->id;
>
> if (common_caps_array)
> g_array_unref(common_caps_array);
> @@ -749,7 +749,7 @@ void dcc_destroy_surface(DisplayChannelClient *dcc,
> uint32_t surface_id)
> display = DCC_TO_DC(dcc);
> channel = RED_CHANNEL(display);
>
> - if (COMMON_GRAPHICS_CHANNEL(display)->during_target_migrate ||
> + if
> (common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
> ||
> !dcc->priv->surface_client_created[surface_id]) {
> return;
> }
> diff --git a/server/display-channel.c b/server/display-channel.c
> index b9e2b93..69edd35 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -172,7 +172,7 @@ static void stop_streams(DisplayChannel *display)
> void display_channel_surface_unref(DisplayChannel *display, uint32_t
> surface_id)
> {
> RedSurface *surface = &display->priv->surfaces[surface_id];
> - QXLInstance *qxl = display->common.qxl;
> + QXLInstance *qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display));
> DisplayChannelClient *dcc;
> GListIter iter;
>
> @@ -1831,7 +1831,7 @@ void display_channel_create_surface(DisplayChannel
> *display, uint32_t surface_id
>
> if (display->priv->renderer == RED_RENDERER_INVALID) {
> int i;
> - QXLInstance *qxl = display->common.qxl;
> + QXLInstance *qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display));
> RedsState *reds = red_qxl_get_server(qxl->st);
> GArray *renderers = reds_get_renderers(reds);
> for (i = 0; i < renderers->len; i++) {
> @@ -2037,7 +2037,7 @@ void display_channel_gl_scanout(DisplayChannel
> *display)
>
> static void set_gl_draw_async_count(DisplayChannel *display, int num)
> {
> - QXLInstance *qxl = display->common.qxl;
> + QXLInstance *qxl =
> common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display));
>
> display->priv->gl_draw_async_count = num;
>
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 2dfa8e4..8da154a 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -533,7 +533,8 @@ static void dev_create_primary_surface(RedWorker *worker,
> uint32_t surface_id,
> line_0, surface.flags &
> QXL_SURF_FLAG_KEEP_DATA, TRUE);
> display_channel_set_monitors_config_to_primary(display);
>
> - if (display_is_connected(worker) &&
> !worker->display_channel->common.during_target_migrate) {
> + if (display_is_connected(worker) &&
> +
> !common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(worker->display_channel)))
> {
> /* guest created primary, so it will (hopefully) send a
> monitors_config
> * now, don't send our own temporary one */
> if (!worker->driver_cap_monitors_config) {
> @@ -638,10 +639,10 @@ static void handle_dev_start(void *opaque, void
> *payload)
>
> spice_assert(!worker->running);
> if (worker->cursor_channel) {
> -
> COMMON_GRAPHICS_CHANNEL(worker->cursor_channel)->during_target_migrate
> = FALSE;
> +
> common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(worker->cursor_channel),
> FALSE);
> }
> if (worker->display_channel) {
> - worker->display_channel->common.during_target_migrate = FALSE;
> +
> common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(worker->display_channel),
> FALSE);
> display_channel_wait_for_migrate_data(worker->display_channel);
> }
> worker->running = TRUE;
Frediano
More information about the Spice-devel
mailing list