Re: [PATCH linux vdagent v3 3/9] Look up and store xrandr output in display map

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2019-01-07 at 15:50 -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 | 47 +++++++++++++++++++----------------------
>  src/vdagent/x11.c       | 20 ++++++------------
>  3 files changed, 30 insertions(+), 39 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 4fed458..4daacc6 100644
> --- a/src/vdagent/x11-randr.c
> +++ b/src/vdagent/x11-randr.c
> @@ -728,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)
> @@ -753,31 +748,33 @@ void vdagent_x11_handle_graphics_device_info(struct vdagent_x11 *x11, uint8_t *d
>              break;
>          }
>  
> -        GraphicsDisplayInfo *value = g_malloc(sizeof(GraphicsDisplayInfo));
> -
> -        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);
> -        }
> +        // make sure the string is terminated:
> +        device_display_info->device_address[device_display_info->device_address_len] = '\0';
>  
> -        strncpy(value->device_address,
> -                (char*) device_display_info->device_address,
> -                device_address_len);
> -        value->device_address[device_address_len] = '\0';  // make sure the string is terminated
> -        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)) {

By adding this function call which logs quite a bit, the indented
logging below no longer works well, as it is interrupted by the
unindented log lines from this function.

We can drop the indent but it was quite helpful to visually separate
the logged block, now with quite a bit more log lines it will be much
harder to read. Not sure what else to do though.

Cheers,
Lukas

> +            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);
>  
> -        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 2473383..a456678 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);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]