[Spice-devel] [PATCH spice 3/8] Send the graphics device info to the vd_agent
Lukáš Hrázký
lhrazky at redhat.com
Wed Jan 9 11:53:38 UTC 2019
On Wed, 2019-01-09 at 02:50 -0500, Frediano Ziglio wrote:
> >
> > Sends the device address and device display IDs to the vdagent. The
> > message is sent either in reaction to the SPICE_MSGC_MAIN_AGENT_START
> > message or when the graphics device info changes.
> >
> > TODO: doesn't resend the message on agent reconnect, FIXIT.
>
> Why is this a problem? What if the agent get restarted?
Ah, sorry, forgot this TODO here. It is the agent reconnect issue we
discussed (off-list). I'll remove this TODO, we are taking care of it.
> >
> > Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> > ---
> > server/red-qxl.c | 20 +++++++++++++
> > server/red-qxl.h | 3 ++
> > server/reds-private.h | 1 +
> > server/reds.c | 67 +++++++++++++++++++++++++++++++++++++++++++
> > server/reds.h | 1 +
> > 5 files changed, 92 insertions(+)
> >
> > diff --git a/server/red-qxl.c b/server/red-qxl.c
> > index 6ffd8286..ebc14a46 100644
> > --- a/server/red-qxl.c
> > +++ b/server/red-qxl.c
> > @@ -889,6 +889,26 @@ void spice_qxl_set_device_info(QXLInstance *instance,
> >
> > instance->st->monitors_count = device_display_id_count;
> > instance->st->max_monitors = device_display_id_count;
> > +
> > + reds_send_device_display_info(red_qxl_get_server(instance->st));
> > +}
> > +
> > +const char* red_qxl_get_device_address(const QXLInstance *qxl)
> > +{
> > + const QXLState *qxl_state = qxl->st;
> > + return qxl_state->device_address;
> > +}
> > +
> > +const uint32_t* red_qxl_get_device_display_ids(const QXLInstance *qxl)
> > +{
> > + const QXLState *qxl_state = qxl->st;
> > + return qxl_state->device_display_ids;
> > +}
> > +
> > +size_t red_qxl_get_monitors_count(const QXLInstance *qxl)
> > +{
> > + const QXLState *qxl_state = qxl->st;
> > + return qxl_state->monitors_count;
> > }
> >
> > void red_qxl_init(RedsState *reds, QXLInstance *qxl)
> > diff --git a/server/red-qxl.h b/server/red-qxl.h
> > index 6014d32a..94753948 100644
> > --- a/server/red-qxl.h
> > +++ b/server/red-qxl.h
> > @@ -40,6 +40,9 @@ void red_qxl_put_gl_scanout(QXLInstance *qxl,
> > SpiceMsgDisplayGlScanoutUnix *scan
> > void red_qxl_gl_draw_async_complete(QXLInstance *qxl);
> > int red_qxl_check_qxl_version(QXLInstance *qxl, int major, int minor);
> > SpiceServer* red_qxl_get_server(QXLState *qxl);
> > +const char* red_qxl_get_device_address(const QXLInstance *qxl);
> > +const uint32_t* red_qxl_get_device_display_ids(const QXLInstance *qxl);
> > +size_t red_qxl_get_monitors_count(const QXLInstance *qxl);
> >
> > /* Wrappers around QXLInterface vfuncs */
> > void red_qxl_get_init_info(QXLInstance *qxl, QXLDevInitInfo *info);
> > diff --git a/server/reds-private.h b/server/reds-private.h
> > index 920edc5c..9dbc7fa9 100644
> > --- a/server/reds-private.h
> > +++ b/server/reds-private.h
> > @@ -81,6 +81,7 @@ struct RedsState {
> > SpiceWatch *secure_listen_watch;
> > RedCharDeviceVDIPort *agent_dev;
> > int pending_mouse_event;
> > + bool pending_device_display_info_message;
> > GList *clients;
> > MainChannel *main_channel;
> > InputsChannel *inputs_channel;
> > diff --git a/server/reds.c b/server/reds.c
> > index cdbb94cb..b85758b6 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -257,6 +257,7 @@ typedef struct __attribute__ ((__packed__)) VDInternalBuf
> > {
> > VDAgentMessage header;
> > union {
> > VDAgentMouseState mouse_state;
> > + VDAgentGraphicsDeviceInfo graphics_device_info;
> > }
> > u;
> > } VDInternalBuf;
> > @@ -894,6 +895,65 @@ static RedPipeItem
> > *vdi_port_read_one_msg_from_device(RedCharDevice *self,
> > return NULL;
> > }
> >
> > +void reds_send_device_display_info(RedsState *reds)
> > +{
> > + if (!reds->agent_dev->priv->agent_attached) {
> > + return;
> > + }
>
> I would expect a check for agent capabilities, isn't it necessary?
> Looking at agent code the message won't be ignored, at least it will
> produce an unwanted warning.
It does produce a warning, which I find harmless and better than
introducing a capability, which makes the code more complex and really
has no practical benefit.
Cheers,
Lukas
> > + g_debug("Sending device display info to the agent:");
> > +
> > + size_t message_size = sizeof(VDAgentGraphicsDeviceInfo);
> > + QXLInstance *qxl;
> > + FOREACH_QXL_INSTANCE(reds, qxl) {
> > + message_size +=
> > + (sizeof(VDAgentDeviceDisplayInfo) +
> > strlen(red_qxl_get_device_address(qxl)) + 1) *
> > + red_qxl_get_monitors_count(qxl);
> > + }
> > +
> > + RedCharDeviceWriteBuffer *char_dev_buf =
> > vdagent_new_write_buffer(reds->agent_dev,
> > + VD_AGENT_GRAPHICS_DEVICE_INFO,
> > + message_size,
> > + true);
> > +
> > + if (!char_dev_buf) {
> > + reds->pending_device_display_info_message = true;
> > + return;
> > + }
> > +
> > + VDInternalBuf *internal_buf = (VDInternalBuf *)char_dev_buf->buf;
> > + VDAgentGraphicsDeviceInfo *graphics_device_info =
> > &internal_buf->u.graphics_device_info;
> > + graphics_device_info->count = 0;
> > +
> > + VDAgentDeviceDisplayInfo *device_display_info =
> > graphics_device_info->device_info;
> > + 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;
> > + device_display_info->monitor_id = i;
> > + device_display_info->device_display_id =
> > red_qxl_get_device_display_ids(qxl)[i];
> > +
> > + strcpy((char*) device_display_info->device_address,
> > red_qxl_get_device_address(qxl));
> > +
> > + 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",
> > + 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);
> > +
> > + graphics_device_info->count++;
> > + }
> > + }
> > +
> > + reds->pending_device_display_info_message = false;
> > +
> > + red_char_device_write_buffer_add(RED_CHAR_DEVICE(reds->agent_dev),
> > char_dev_buf);
> > +}
> > +
> > /* after calling this, we unref the message, and the ref is in the instance
> > side */
> > static void vdi_port_send_msg_to_client(RedCharDevice *self,
> > RedPipeItem *msg,
> > @@ -925,6 +985,11 @@ static void vdi_port_on_free_self_token(RedCharDevice
> > *self)
> > spice_debug("pending mouse event");
> > reds_handle_agent_mouse_event(reds,
> > inputs_channel_get_mouse_state(reds->inputs_channel));
> > }
> > +
> > + if (reds->pending_device_display_info_message) {
> > + spice_debug("pending device display info message");
> > + reds_send_device_display_info(reds);
> > + }
> > }
> >
> > static void vdi_port_remove_client(RedCharDevice *self,
> > @@ -1062,6 +1127,8 @@ void reds_on_main_agent_start(RedsState *reds,
> > MainChannelClient *mcc, uint32_t
> > num_tokens);
> > }
> >
> > + reds_send_device_display_info(reds);
> > +
> > agent_msg_filter_config(&reds->agent_dev->priv->write_filter,
> > reds->config->agent_copypaste,
> > reds->config->agent_file_xfer,
> > reds_use_client_monitors_config(reds));
> > diff --git a/server/reds.h b/server/reds.h
> > index 9f17a5ec..8260ab5c 100644
> > --- a/server/reds.h
> > +++ b/server/reds.h
> > @@ -50,6 +50,7 @@ gboolean reds_config_get_agent_mouse(const RedsState
> > *reds); // used by inputs_c
> > int reds_has_vdagent(RedsState *reds); // used by inputs channel
> > bool reds_config_get_playback_compression(RedsState *reds); // used by
> > playback channel
> >
> > +void reds_send_device_display_info(RedsState *reds);
> > void reds_handle_agent_mouse_event(RedsState *reds, const VDAgentMouseState
> > *mouse_state); // used by inputs_channel
> >
> > GArray* reds_get_renderers(RedsState *reds);
>
> Frediano
More information about the Spice-devel
mailing list