[Spice-devel] [PATCH linux vdagent v4 2/9] Look up and store xrandr output in display map

Lukáš Hrázký lhrazky at redhat.com
Fri Jan 18 12:44:02 UTC 2019


On Thu, 2019-01-17 at 16:14 -0600, Jonathon Jongsma wrote:
> Instead of storing each device address and device display ID in the hash
> table, simply use the lookup_xrandr_output_for_device_info() function to
> look up the ID of the xrandr output and store that in the hash table.
> ---
>  src/vdagent/x11-priv.h  |  2 +-
>  src/vdagent/x11-randr.c | 53 +++++++++++++++++++----------------------
>  src/vdagent/x11.c       | 20 ++++++----------
>  3 files changed, 32 insertions(+), 43 deletions(-)
> 
> diff --git a/src/vdagent/x11-priv.h b/src/vdagent/x11-priv.h
> index 0e954cf..e487aa2 100644
> --- a/src/vdagent/x11-priv.h
> +++ b/src/vdagent/x11-priv.h
> @@ -139,7 +139,7 @@ struct vdagent_x11 {
>      int xrandr_minor;
>      int has_xinerama;
>      int dont_send_guest_xorg_res;
> -    GHashTable *graphics_display_infos;
> +    GHashTable *guest_output_map;
>  };
>  
>  extern int (*vdagent_x11_prev_error_handler)(Display *, XErrorEvent *);
> diff --git a/src/vdagent/x11-randr.c b/src/vdagent/x11-randr.c
> index 405fca9..e65d8d0 100644
> --- a/src/vdagent/x11-randr.c
> +++ b/src/vdagent/x11-randr.c
> @@ -31,6 +31,7 @@
>  
>  #include <X11/extensions/Xinerama.h>
>  
> +#include "device-info.h"
>  #include "vdagentd-proto.h"
>  #include "x11.h"
>  #include "x11-priv.h"
> @@ -727,11 +728,6 @@ static void dump_monitors_config(struct vdagent_x11 *x11,
>      }
>  }
>  
> -typedef struct GraphicsDisplayInfo {
> -    char device_address[256];
> -    uint32_t device_display_id;
> -} GraphicsDisplayInfo;
> -
>  // handle the device info message from the server. This will allow us to
>  // maintain a mapping from spice display id to xrandr output
>  void vdagent_x11_handle_graphics_device_info(struct vdagent_x11 *x11, uint8_t *data, size_t size)
> @@ -752,39 +748,38 @@ void vdagent_x11_handle_graphics_device_info(struct vdagent_x11 *x11, uint8_t *d
>              break;
>          }
>  
> -        GraphicsDisplayInfo *value = g_malloc(sizeof(GraphicsDisplayInfo));
> -        value->device_address[0] = '\0';
> -
> -        size_t device_address_len = device_display_info->device_address_len;
> -        if (device_address_len > sizeof(value->device_address)) {
> -            syslog(LOG_ERR, "Received a device address longer than %lu, "
> -                   "will be truncated!", device_address_len);
> -            device_address_len = sizeof(value->device_address);
> -        }
> -
> -        strncpy(value->device_address,
> -                (char*) device_display_info->device_address,
> -                device_address_len);
> -
> -        if (device_address_len > 0) {
> -            value->device_address[device_address_len - 1] = '\0';  // make sure the string is terminated
> +        // make sure the string is terminated:
> +        if (device_display_info->device_address_len > 0) {
> +            device_display_info->device_address[device_display_info->device_address_len - 1] = '\0';
>          } else {
>              syslog(LOG_WARNING, "Zero length device_address received for channel_id: %u, monitor_id: %u",
>                     device_display_info->channel_id, device_display_info->monitor_id);
>          }
>  
> -        value->device_display_id = device_display_info->device_display_id;
> +        RROutput x_output;
> +        if (lookup_xrandr_output_for_device_info(device_display_info, x11->display, x11->randr.res, &x_output)) {
> +            gint64 *value = g_new(gint64, 1);
> +            *value = x_output;
>  
> -        syslog(LOG_INFO, "   channel_id: %u monitor_id: %u device_address: %s, "
> -               "device_display_id: %u",
> -               device_display_info->channel_id,
> -               device_display_info->monitor_id,
> -               value->device_address,
> -               value->device_display_id);
> +            syslog(LOG_INFO, "channel_id: %u monitor_id: %u device_address: %s, "
> +                   "device_display_id: %u xrandr output ID: %lu",
> +                   device_display_info->channel_id,
> +                   device_display_info->monitor_id,
> +                   device_display_info->device_address,
> +                   device_display_info->device_display_id,
> +                   x_output);

Since you've dropped the indent, I'd prefix the logline with some
context, like "Adding draphics device info ..." or something.

> -        g_hash_table_insert(x11->graphics_display_infos,
> +            g_hash_table_insert(x11->guest_output_map,
>                  GUINT_TO_POINTER(device_display_info->channel_id + device_display_info->monitor_id),
>                  value);
> +        } else {
> +            syslog(LOG_INFO, "channel_id: %u monitor_id: %u device_address: %s, "
> +                   "device_display_id: %u xrandr output ID NOT FOUND",
> +                   device_display_info->channel_id,
> +                   device_display_info->monitor_id,
> +                   device_display_info->device_address,
> +                   device_display_info->device_display_id);
> +        }
>  
>          device_display_info = (VDAgentDeviceDisplayInfo*) ((char*) device_display_info +
>              sizeof(VDAgentDeviceDisplayInfo) + device_display_info->device_address_len);
> diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> index ab822f8..484be5e 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -196,12 +196,6 @@ static gchar *vdagent_x11_get_wm_name(struct vdagent_x11 *x11)
>  #endif
>  }
>  
> -static void graphics_display_info_destroy(gpointer gdi)
> -{
> -    g_free(gdi);
> -}
> -
> -
>  struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
>      int debug, int sync)
>  {
> @@ -218,6 +212,12 @@ struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
>      x11->vdagentd = vdagentd;
>      x11->debug = debug;
>  
> +    x11->guest_output_map = g_hash_table_new_full(&g_direct_hash,
> +                                                  &g_direct_equal,
> +                                                  NULL,
> +                                                  &g_free);
> +
> +
>      x11->display = XOpenDisplay(NULL);
>      if (!x11->display) {
>          syslog(LOG_ERR, "could not connect to X-server");
> @@ -322,12 +322,6 @@ struct vdagent_x11 *vdagent_x11_create(struct udscs_connection *vdagentd,
>                 __func__, net_wm_name, vdagent_x11_has_icons_on_desktop(x11));
>      g_free(net_wm_name);
>  
> -    x11->graphics_display_infos = g_hash_table_new_full(&g_direct_hash,
> -                                                  &g_direct_equal,
> -                                                  NULL,
> -                                                  &graphics_display_info_destroy);
> -
> -
>      /* Flush output buffers and consume any pending events */
>      vdagent_x11_do_read(x11);
>  
> @@ -349,7 +343,7 @@ void vdagent_x11_destroy(struct vdagent_x11 *x11, int vdagentd_disconnected)
>      }
>  #endif
>  
> -    g_hash_table_destroy(x11->graphics_display_infos);
> +    g_hash_table_destroy(x11->guest_output_map);
>      XCloseDisplay(x11->display);
>      g_free(x11->randr.failed_conf);
>      g_free(x11);

Reviewed-by: Lukáš Hrázký <lhrazky at redhat.com>


More information about the Spice-devel mailing list