[Spice-devel] [PATCH 08/18] Remove global 'dispatchers', 'num_active_workers' variables

Fabiano Fidêncio ffidenci at redhat.com
Mon Feb 15 23:03:26 UTC 2016


On Mon, 2016-02-15 at 16:01 +0000, Frediano Ziglio wrote:
> Since these are server-level variables, move them into RedsState.
> However, num_active_workers was removed because:
> - each dispatcher always has 1 active worker, so we can determine the
>   number of active workers by counting the dispatchers
> - it was never actually set correctly. Even if there was more than
> one
>   worker, this variable was always only set to either 0 or 1.
> 
> This change required moving a bunch of helper code into RedsState as
> well, an providing some RedDispatcher interfaces to access dispatcher
> information from RedsState.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/agent-msg-filter.c |  3 +-
>  server/red-dispatcher.c   | 92 +++++++++++++++++------------------
> ------------
>  server/red-dispatcher.h   | 11 +++---
>  server/reds.c             | 91
> +++++++++++++++++++++++++++++++++++++++++++---
>  server/reds.h             |  5 ++-
>  5 files changed, 131 insertions(+), 71 deletions(-)
> 
> diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c
> index 1c1c01f..069822b 100644
> --- a/server/agent-msg-filter.c
> +++ b/server/agent-msg-filter.c
> @@ -24,6 +24,7 @@
>  #include <string.h>
>  #include "red-common.h"
>  #include "agent-msg-filter.h"
> +#include "reds.h"
>  #include "red-dispatcher.h"
>  
>  void agent_msg_filter_init(struct AgentMsgFilter *filter,
> @@ -92,7 +93,7 @@ data_to_read:
>              }
>              break;
>          case VD_AGENT_MONITORS_CONFIG:
> -            if (red_dispatcher_use_client_monitors_config()) {
> +            if (reds_use_client_monitors_config(reds)) {
>                  filter->result = AGENT_MSG_FILTER_MONITORS_CONFIG;
>              } else {
>                  filter->result = AGENT_MSG_FILTER_OK;
> diff --git a/server/red-dispatcher.c b/server/red-dispatcher.c
> index 40d13fe..c2ca6b6 100644
> --- a/server/red-dispatcher.c
> +++ b/server/red-dispatcher.c
> @@ -54,13 +54,10 @@ struct RedDispatcher {
>      int x_res;
>      int y_res;
>      int use_hardware_cursor;
> -    RedDispatcher *next;
>      QXLDevSurfaceCreate surface_create;
>      unsigned int max_monitors;
>  };
>  
> -static RedDispatcher *dispatchers = NULL;
> -
>  static int red_dispatcher_check_qxl_version(RedDispatcher *rd, int
> major, int minor)
>  {
>      int qxl_major = rd->qxl->st->qif->base.major_version;
> @@ -190,33 +187,6 @@ static void
> red_dispatcher_cursor_migrate(RedChannelClient *rcc)
>                              &payload);
>  }
>  
> -static void reds_update_client_mouse_allowed(RedsState *reds)
> -{
> -    static int allowed = FALSE;
> -    int allow_now = FALSE;
> -    int x_res = 0;
> -    int y_res = 0;
> -
> -    allow_now = TRUE;
> -    RedDispatcher *now = dispatchers;
> -    while (now && allow_now) {
> -        if (now->primary_active) {
> -            allow_now = now->use_hardware_cursor;
> -            if (allow_now) {
> -                x_res = now->x_res;
> -                y_res = now->y_res;
> -            }
> -            break;
> -        }
> -        now = now->next;
> -    }
> -
> -    if (allow_now || allow_now != allowed) {
> -        allowed = allow_now;
> -        reds_set_client_mouse_allowed(reds, allowed, x_res, y_res);
> -    }
> -}
> -
>  static void red_dispatcher_update_area(RedDispatcher *dispatcher,
> uint32_t surface_id,
>                                     QXLRect *qxl_area, QXLRect
> *qxl_dirty_rects,
>                                     uint32_t num_dirty_rects,
> uint32_t clear_dirty_region)
> @@ -233,33 +203,19 @@ static void
> red_dispatcher_update_area(RedDispatcher *dispatcher, uint32_t surfa
>                              &payload);
>  }
>  
> -int red_dispatcher_use_client_monitors_config(void)
> +gboolean red_dispatcher_use_client_monitors_config(RedDispatcher
> *dispatcher)
>  {
> -    RedDispatcher *now = dispatchers;
> -
> -    for (; now ; now = now->next) {
> -        if (!red_dispatcher_check_qxl_version(now, 3, 3) ||
> -            !now->qxl->st->qif->client_monitors_config ||
> -            !now->qxl->st->qif->client_monitors_config(now->qxl,
> NULL)) {
> -            return FALSE;
> -        }
> -    }
> -    return TRUE;
> +    return (red_dispatcher_check_qxl_version(dispatcher, 3, 3) &&
> +        dispatcher->qxl->st->qif->client_monitors_config &&
> +        dispatcher->qxl->st->qif->client_monitors_config(dispatcher-
> >qxl, NULL));
>  }
>  
> -void red_dispatcher_client_monitors_config(VDAgentMonitorsConfig
> *monitors_config)
> +gboolean red_dispatcher_client_monitors_config(RedDispatcher
> *dispatcher,
> +                                               VDAgentMonitorsConfig
> *monitors_config)
>  {
> -    RedDispatcher *now = dispatchers;
> -
> -    while (now) {
> -        if (!now->qxl->st->qif->client_monitors_config ||
> -            !now->qxl->st->qif->client_monitors_config(now->qxl,
> -                                                       monitors_conf
> ig)) {
> -            /* this is a normal condition, some qemu devices might
> not implement it */
> -            spice_debug("QXLInterface::client_monitors_config
> failed");
> -        }
> -        now = now->next;
> -    }
> +    return (dispatcher->qxl->st->qif->client_monitors_config &&
> +        dispatcher->qxl->st->qif->client_monitors_config(dispatcher-
> >qxl,
> +                                                          monitors_c
> onfig));
>  }
>  
>  static AsyncCommand *async_command_alloc(RedDispatcher *dispatcher,
> @@ -682,6 +638,11 @@ static void qxl_worker_loadvm_commands(QXLWorker
> *qxl_worker,
>      red_dispatcher_loadvm_commands((RedDispatcher*)qxl_worker, ext,
> count);
>  }
>  
> +void red_dispatcher_set_mm_time(RedDispatcher *dispatcher, uint32_t
> mm_time)
> +{
> +    dispatcher->qxl->st->qif->set_mm_time(dispatcher->qxl, mm_time);
> +}
> +
>  void red_dispatcher_attach_worker(RedDispatcher *dispatcher)
>  {
>      QXLInstance *qxl = dispatcher->qxl;
> @@ -693,13 +654,10 @@ void
> red_dispatcher_set_compression_level(RedDispatcher *dispatcher, int
> level)
>      dispatcher->qxl->st->qif->set_compression_level(dispatcher->qxl, 
> level);
>  }
>  
> -uint32_t red_dispatcher_qxl_ram_size(void)
> +uint32_t red_dispatcher_qxl_ram_size(RedDispatcher *dispatcher)
>  {
>      QXLDevInitInfo qxl_info;
> -    if (!dispatchers) {
> -        return 0;
> -    }
> -    dispatchers->qxl->st->qif->get_init_info(dispatchers->qxl,
> &qxl_info);
> +    dispatcher->qxl->st->qif->get_init_info(dispatcher->qxl,
> &qxl_info);
>      return qxl_info.qxl_ram_size;
>  }
>  
> @@ -1010,8 +968,6 @@ void red_dispatcher_init(QXLInstance *qxl)
>      red_worker_run(worker);
>  
>      qxl->st->dispatcher = red_dispatcher;
> -    red_dispatcher->next = dispatchers;
> -    dispatchers = red_dispatcher;
>  }
>  
>  struct Dispatcher *red_dispatcher_get_dispatcher(RedDispatcher
> *red_dispatcher)
> @@ -1032,6 +988,22 @@ void red_dispatcher_clear_pending(RedDispatcher
> *red_dispatcher, int pending)
>      clear_bit(pending, &red_dispatcher->pending);
>  }
>  
> +gboolean red_dispatcher_get_primary_active(RedDispatcher
> *dispatcher)
> +{
> +    return dispatcher->primary_active;
> +}
> +
> +gboolean red_dispatcher_get_allow_client_mouse(RedDispatcher
> *dispatcher, gint *x_res, gint *y_res)
> +{
> +    if (dispatcher->use_hardware_cursor) {
> +        if (x_res)
> +            *x_res = dispatcher->x_res;
> +        if (y_res)
> +            *y_res = dispatcher->y_res;
> +    }
> +    return dispatcher->use_hardware_cursor;
> +}
> +
>  void red_dispatcher_on_ic_change(RedDispatcher *dispatcher,
> SpiceImageCompression ic)
>  {
>      RedWorkerMessageSetCompression payload;
> diff --git a/server/red-dispatcher.h b/server/red-dispatcher.h
> index 9614544..1aa63c2 100644
> --- a/server/red-dispatcher.h
> +++ b/server/red-dispatcher.h
> @@ -26,6 +26,7 @@ typedef struct AsyncCommand AsyncCommand;
>  
>  void red_dispatcher_init(QXLInstance *qxl);
>  
> +void red_dispatcher_set_mm_time(RedDispatcher *dispatcher,
> uint32_t);
>  void red_dispatcher_on_ic_change(RedDispatcher *dispatcher,
> SpiceImageCompression ic);
>  void red_dispatcher_on_sv_change(RedDispatcher *dispatcher, int sv);
>  void red_dispatcher_set_mouse_mode(RedDispatcher *dispatcher,
> uint32_t mode);
> @@ -33,13 +34,13 @@ void red_dispatcher_attach_worker(RedDispatcher
> *dispatcher);
>  void red_dispatcher_set_compression_level(RedDispatcher *dispatcher,
> int level);
>  void red_dispatcher_stop(RedDispatcher *dispatcher);
>  void red_dispatcher_start(RedDispatcher *dispatcher);
> -void red_dispatcher_on_vm_stop(void);
> -void red_dispatcher_on_vm_start(void);
> -uint32_t red_dispatcher_qxl_ram_size(void);
> +uint32_t red_dispatcher_qxl_ram_size(RedDispatcher *dispatcher);
>  void red_dispatcher_async_complete(struct RedDispatcher *,
> AsyncCommand *);
>  struct Dispatcher *red_dispatcher_get_dispatcher(struct
> RedDispatcher *);
> -int red_dispatcher_use_client_monitors_config(void);
> -void red_dispatcher_client_monitors_config(VDAgentMonitorsConfig
> *monitors_config);
> +gboolean red_dispatcher_use_client_monitors_config(RedDispatcher
> *dispatcher);
> +gboolean red_dispatcher_client_monitors_config(RedDispatcher
> *dispatcher, VDAgentMonitorsConfig *monitors_config);
> +gboolean red_dispatcher_get_primary_active(RedDispatcher
> *dispatcher);
> +gboolean red_dispatcher_get_allow_client_mouse(RedDispatcher
> *dispatcher, gint *x_res, gint *y_res);
>  
>  typedef uint32_t RedWorkerMessage;
>  
> diff --git a/server/reds.c b/server/reds.c
> index 57b01b0..c30b5a6 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -176,6 +176,9 @@ static void reds_on_ic_change(RedsState *reds);
>  static void reds_on_sv_change(RedsState *reds);
>  static void reds_on_vm_stop(RedsState *reds);
>  static void reds_on_vm_start(RedsState *reds);
> +static void reds_set_mouse_mode(RedsState *reds, uint32_t mode);
> +static uint32_t reds_qxl_ram_size(RedsState *reds);
> +static int calc_compression_level(RedsState *reds);
>  
>  static VDIReadBuf *vdi_port_state_get_read_buf(VDIPortState *state);
>  static VDIReadBuf *vdi_port_read_buf_ref(VDIReadBuf *buf);
> @@ -1034,7 +1037,7 @@ static void
> reds_on_main_agent_monitors_config(RedsState *reds,
>      }
>      monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer +
> sizeof(*msg_header));
>      spice_debug("%s: %d", __func__, monitors_config-
> >num_of_monitors);
> -    red_dispatcher_client_monitors_config(monitors_config);
> +    reds_client_monitors_config(reds, monitors_config);
>      reds_client_monitors_config_cleanup(reds);
>  }
>  
> @@ -1681,7 +1684,7 @@ static void reds_handle_main_link(RedsState
> *reds, RedLinkInfo *link)
>          main_channel_push_init(mcc, g_list_length(reds-
> >dispatchers),
>              reds->mouse_mode, reds->is_client_mouse_allowed,
>              reds_get_mm_time() - MM_TIME_DELTA,
> -            red_dispatcher_qxl_ram_size());
> +            reds_qxl_ram_size(reds));
>          if (reds->spice_name)
>              main_channel_push_name(mcc, reds->spice_name);
>          if (reds->spice_uuid_is_set)
> @@ -1827,7 +1830,7 @@ void
> reds_on_client_semi_seamless_migrate_complete(RedsState *reds,
> RedClient *c
>      main_channel_push_init(mcc, g_list_length(reds->dispatchers),
>                             reds->mouse_mode, reds-
> >is_client_mouse_allowed,
>                             reds_get_mm_time() - MM_TIME_DELTA,
> -                           red_dispatcher_qxl_ram_size());
> +                           reds_qxl_ram_size(reds));
>      reds_link_mig_target_channels(reds, client);
>      main_channel_migrate_dst_complete(mcc);
>  }
> @@ -4063,7 +4066,74 @@ SpiceCoreInterfaceInternal*
> reds_get_core_interface(RedsState *reds)
>      return reds->core;
>  }
>  
> -int calc_compression_level(RedsState *reds)
> +void reds_update_client_mouse_allowed(RedsState *reds)
> +{
> +    static int allowed = FALSE;
> +    int allow_now = FALSE;
> +    int x_res = 0;
> +    int y_res = 0;
> +    GList *l;
> +    int num_active_workers = g_list_length(reds->dispatchers);
> +
> +    if (num_active_workers > 0) {
> +        allow_now = TRUE;
> +        for (l = reds->dispatchers; l != NULL && allow_now; l = l-
> >next) {
> +
> +            RedDispatcher *now = l->data;
> +            if (red_dispatcher_get_primary_active(now)) {
> +                allow_now =
> red_dispatcher_get_allow_client_mouse(now, &x_res, &y_res);
> +                break;
> +            }
> +        }
> +    }
> +
> +    if (allow_now || allow_now != allowed) {
> +        allowed = allow_now;
> +        reds_set_client_mouse_allowed(reds, allowed, x_res, y_res);
> +    }
> +}
> +
> +gboolean reds_use_client_monitors_config(RedsState *reds)
> +{
> +    GList *l;
> +
> +    if (reds->dispatchers == NULL) {
> +        return FALSE;
> +    }
> +
> +    for (l = reds->dispatchers; l != NULL ; l = l->next) {
> +        RedDispatcher *now = l->data;
> +
> +        if (!red_dispatcher_use_client_monitors_config(now))
> +            return FALSE;
> +    }
> +    return TRUE;
> +}
> +
> +void reds_client_monitors_config(RedsState *reds,
> VDAgentMonitorsConfig *monitors_config)
> +{
> +    GList *l;
> +
> +    for (l = reds->dispatchers; l != NULL; l = l->next) {
> +        RedDispatcher *now = l->data;
> +        if (!red_dispatcher_client_monitors_config(now,
> monitors_config)) {
> +            /* this is a normal condition, some qemu devices might
> not implement it */
> +            spice_debug("QXLInterface::client_monitors_config
> failed\n");
> +        }
> +    }
> +}
> +
> +void reds_set_mm_time(RedsState *reds, uint32_t mm_time)
> +{
> +    GList *l;
> +
> +    for (l = reds->dispatchers; l != NULL; l = l->next) {
> +        RedDispatcher *now = l->data;
> +        red_dispatcher_set_mm_time(now, mm_time);
> +    }
> +}
> +
> +static int calc_compression_level(RedsState *reds)
>  {
>      spice_assert(reds_get_streaming_video(reds) !=
> SPICE_STREAM_VIDEO_INVALID);
>  
> @@ -4103,6 +4173,7 @@ void reds_on_vm_stop(RedsState *reds)
>  {
>      GList *l;
>  
> +    spice_debug(NULL);

Is this really part of this patch?
Not sure if it makes sense, but if it does, would be better to have it
squased to the patch that introduced reds_on_vm_stop() function.


>      for (l = reds->dispatchers; l != NULL; l = l->next)
>          red_dispatcher_stop(l->data);
>  }
> @@ -4111,6 +4182,18 @@ void reds_on_vm_start(RedsState *reds)
>  {
>      GList *l;
>  
> +    spice_debug(NULL);

Same here.

>      for (l = reds->dispatchers; l != NULL; l = l->next)
>          red_dispatcher_start(l->data);
>  }
> +
> +uint32_t reds_qxl_ram_size(RedsState *reds)
> +{
> +    RedDispatcher *first;
> +    if (!reds->dispatchers) {
> +        return 0;
> +    }
> +
> +    first = reds->dispatchers->data;
> +    return red_dispatcher_qxl_ram_size(first);
> +}
> diff --git a/server/reds.h b/server/reds.h
> index df34a88..686aaac 100644
> --- a/server/reds.h
> +++ b/server/reds.h
> @@ -111,6 +111,9 @@ uint32_t reds_get_streaming_video(const RedsState
> *reds);
>  spice_wan_compression_t reds_get_jpeg_state(const RedsState *reds);
>  spice_wan_compression_t reds_get_zlib_glz_state(const RedsState
> *reds);
>  SpiceCoreInterfaceInternal* reds_get_core_interface(RedsState
> *reds);
> -int calc_compression_level(RedsState *reds);
> +void reds_update_client_mouse_allowed(RedsState *reds);
> +gboolean reds_use_client_monitors_config(RedsState *reds);
> +void reds_client_monitors_config(RedsState *reds,
> VDAgentMonitorsConfig *monitors_config);
> +void reds_set_mm_time(RedsState *reds, uint32_t mm_time);
>  
>  #endif

Acked-by: Fabiano Fidêncio <fidencio at redhat.com>


More information about the Spice-devel mailing list