[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