[Spice-devel] [PATCH spice 5/8 v3] Send the graphics device info from streaming agent to the vd_agent

[Jonathon Jongsma]

On Wed, 2019-01-16 at 13:52 +0100, Lukáš Hrázký wrote:
> Adds the graphics device info from the streaming device(s) to the
> VDAgentGraphicsDeviceInfo message sent to the vd_agent.
> 
> Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> ---
>  server/red-stream-device.c | 18 +++++++++++
>  server/red-stream-device.h |  7 +++++
>  server/reds.c              | 63
> ++++++++++++++++++++++++++++++++++++--
>  3 files changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index 6c90d77d..e7ed9f76 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -342,6 +342,8 @@ handle_msg_device_display_info(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
>              dev->device_display_info.device_address,
>              dev->device_display_info.device_display_id);
>  
> +    reds_send_device_display_info(red_char_device_get_server(RED_CHA
> R_DEVICE(dev)));
> +
>      return true;
>  }
>  
> @@ -799,6 +801,22 @@ stream_device_init(StreamDevice *dev)
>      dev->msg_len = sizeof(*dev->msg);
>  }
>  
> +StreamDeviceDisplayInfo
> *stream_device_get_device_display_info(StreamDevice *dev)
> +{
> +    return &dev->device_display_info;
> +}
> +
> +int32_t stream_device_get_stream_channel_id(StreamDevice *dev)
> +{
> +    if (dev->stream_channel == NULL) {
> +        return -1;
> +    }
> +
> +    int32_t channel_id;
> +    g_object_get(dev->stream_channel, "id", &channel_id, NULL);
> +    return channel_id;
> +}
> +

Did you consider using a similar interface to the one used for the QXL
displays? e.g. _get_device_address() / _get_device_display_ids() / etc?

I know that the stream device currently (and probably always) only has
a single display per channel, but part of me wishes the interfaces had
some consistency.  Not a big issue, just curious.

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


>  static StreamDevice *
>  stream_device_new(SpiceCharDeviceInstance *sin, RedsState *reds)
>  {
> diff --git a/server/red-stream-device.h b/server/red-stream-device.h
> index 996be016..d7ab5e41 100644
> --- a/server/red-stream-device.h
> +++ b/server/red-stream-device.h
> @@ -56,6 +56,13 @@ StreamDevice *stream_device_connect(RedsState
> *reds, SpiceCharDeviceInstance *si
>   */
>  void stream_device_create_channel(StreamDevice *dev);
>  
> +StreamDeviceDisplayInfo
> *stream_device_get_device_display_info(StreamDevice *dev);
> +
> +/**
> + * Returns -1 if the StreamDevice doesn't have a channel yet.
> + */
> +int32_t stream_device_get_stream_channel_id(StreamDevice *dev);
> +
>  G_END_DECLS
>  
>  #endif /* STREAM_DEVICE_H */
> diff --git a/server/reds.c b/server/reds.c
> index 88a82419..22e81d73 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -902,14 +902,33 @@ void reds_send_device_display_info(RedsState
> *reds)
>      }
>      g_debug("Sending device display info to the agent:");
>  
> -    size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
>      QXLInstance *qxl;
> +    RedCharDevice *dev;
> +
> +    size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
> +
> +    // size for the qxl device info
>      FOREACH_QXL_INSTANCE(reds, qxl) {
>          message_size +=
>              (sizeof(VDAgentDeviceDisplayInfo) +
> strlen(red_qxl_get_device_address(qxl)) + 1) *
>              red_qxl_get_monitors_count(qxl);
>      }
>  
> +    // size for the stream device info
> +    GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
> +        if (IS_STREAM_DEVICE(dev)) {
> +            size_t device_address_len =
> +                strlen(stream_device_get_device_display_info(STREAM_
> DEVICE(dev))->device_address);
> +
> +            if (device_address_len == 0) {
> +                // the device info wasn't set (yet), don't send it
> +                continue;
> +            }
> +
> +            message_size += (sizeof(VDAgentDeviceDisplayInfo) +
> device_address_len + 1);
> +        }
> +    }
> +
>      RedCharDeviceWriteBuffer *char_dev_buf =
> vdagent_new_write_buffer(reds->agent_dev,
>                                           VD_AGENT_GRAPHICS_DEVICE_IN
> FO,
>                                           message_size,
> @@ -925,6 +944,8 @@ void reds_send_device_display_info(RedsState
> *reds)
>      graphics_device_info->count = 0;
>  
>      VDAgentDeviceDisplayInfo *device_display_info =
> graphics_device_info->display_info;
> +
> +    // add the qxl devices to the message
>      FOREACH_QXL_INSTANCE(reds, qxl) {
>          for (size_t i = 0; i < red_qxl_get_monitors_count(qxl); ++i)
> {
>              device_display_info->channel_id = qxl->id;
> @@ -936,7 +957,45 @@ void reds_send_device_display_info(RedsState
> *reds)
>              device_display_info->device_address_len =
>                  strlen((char*) device_display_info->device_address)
> + 1;
>  
> -            g_debug("   channel_id: %u monitor_id: %u,
> device_address: %s, device_display_id: %u",
> +            g_debug("   (qxl)    channel_id: %u monitor_id: %u,
> device_address: %s, device_display_id: %u",
> +                    device_display_info->channel_id,
> +                    device_display_info->monitor_id,
> +                    device_display_info->device_address,12th
> Congressional District of Pennsylvania
> +                    device_display_info->device_display_id);
> +
> +            device_display_info = (VDAgentDeviceDisplayInfo*)
> ((char*) device_display_info +
> +                    sizeof(VDAgentDeviceDisplayInfo) +
> device_display_info->device_address_len);
> +
> +            graphics_device_info->count++;
> +        }
> +    }
> +
> +    // add the stream devices to the message
> +    GLIST_FOREACH(reds->char_devices, RedCharDevice, dev) {
> +        if (IS_STREAM_DEVICE(dev)) {
> +            StreamDevice *stream_dev = STREAM_DEVICE(dev);
> +            StreamDeviceDisplayInfo *info =
> stream_device_get_device_display_info(stream_dev);
> +            size_t device_address_len = strlen(info-
> >device_address);
> +
> +            if (device_address_len == 0) {
> +                // the device info wasn't set (yet), don't send it
> +                continue;
> +            }
> +
> +            int32_t channel_id =
> stream_device_get_stream_channel_id(stream_dev);
> +            if (channel_id == -1) {
> +                g_warning("DeviceDisplayInfo set but no stream
> channel exists");
> +                continue;
> +            }
> +
> +            device_display_info->channel_id = channel_id;
> +            device_display_info->monitor_id = info->stream_id;
> +            device_display_info->device_display_id = info-
> >device_display_id;
> +
> +            strcpy((char*) device_display_info->device_address,
> info->device_address);
> +            device_display_info->device_address_len =
> device_address_len + 1;
> +
> +            g_debug("   (stream) channel_id: %u monitor_id: %u,
> device_address: %s, device_display_id: %u",
>                      device_display_info->channel_id,
>                      device_display_info->monitor_id,
>                      device_display_info->device_address,