[Spice-devel] [RFC/POC PATCH spice 06/16] Use VDAgentMonitorsConfigV2 that contains the output_id field

[Lukáš Hrázký]

The VDAgentMonitorsConfig message is sent from the client to the
server and possibly passed on to the vd_agent. Use the version 2 (V2
suffix) that contains the guest (xrandr) output ID of the monitor. The
output ID is only useful if the message is passed to the vd_agent, as it
is an ID in the guest context.

Passing the output ID is not a common case at this time, as with recent
enough Qemu the configuration is done through the QXL interface and
never passed to the vd_agent (that will likely change with multimonitor
streaming, though). Unfortunately, the VDAgentMonitorsConfig message is
also used on the QXL interface, so we need to copy the content into it
from the VDAgentMonitorsConfigV2.
---
 server/dcc-send.c |  1 +
 server/reds.c     | 42 +++++++++++++++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/server/dcc-send.c b/server/dcc-send.c
index 211c69d7..866e95c3 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -2296,6 +2296,7 @@ static void marshall_monitors_config(RedChannelClient *rcc, SpiceMarshaller *bas
         }
         msg->heads[count].id = monitors_config->heads[i].id;
         msg->heads[count].surface_id = monitors_config->heads[i].surface_id;
+        msg->heads[count].output_id = 0; // 0 means output_id is unset, which is the case when not streaming
         msg->heads[count].width = monitors_config->heads[i].width;
         msg->heads[count].height = monitors_config->heads[i].height;
         msg->heads[count].x = monitors_config->heads[i].x;
diff --git a/server/reds.c b/server/reds.c
index 935448d8..f1a60637 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -78,7 +78,7 @@
 
 #define REDS_MAX_STAT_NODES 100
 
-static void reds_client_monitors_config(RedsState *reds, VDAgentMonitorsConfig *monitors_config);
+static void reds_client_monitors_config(RedsState *reds, VDAgentMonitorsConfigV2 *monitors_config);
 static gboolean reds_use_client_monitors_config(RedsState *reds);
 static void reds_set_video_codecs(RedsState *reds, GArray *video_codecs);
 
@@ -1120,10 +1120,10 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
 {
     const unsigned int MAX_MONITORS = 256;
     const unsigned int MAX_MONITOR_CONFIG_SIZE =
-       sizeof(VDAgentMonitorsConfig) + MAX_MONITORS * sizeof(VDAgentMonConfig);
+       sizeof(VDAgentMonitorsConfigV2) + MAX_MONITORS * sizeof(VDAgentMonConfigV2);
 
     VDAgentMessage *msg_header;
-    VDAgentMonitorsConfig *monitors_config;
+    VDAgentMonitorsConfigV2 *monitors_config;
     SpiceBuffer *cmc = &reds->client_monitors_config;
     uint32_t max_monitors;
 
@@ -1145,13 +1145,13 @@ static void reds_on_main_agent_monitors_config(RedsState *reds,
         spice_debug("not enough data yet. %zd", cmc->offset);
         return;
     }
-    if (msg_header->size < sizeof(VDAgentMonitorsConfig)) {
+    if (msg_header->size < sizeof(VDAgentMonitorsConfigV2)) {
         goto overflow;
     }
-    monitors_config = (VDAgentMonitorsConfig *)(cmc->buffer + sizeof(*msg_header));
+    monitors_config = (VDAgentMonitorsConfigV2 *)(cmc->buffer + sizeof(*msg_header));
     // limit the monitor number to avoid buffer overflows
-    max_monitors = (msg_header->size - sizeof(VDAgentMonitorsConfig)) /
-                   sizeof(VDAgentMonConfig);
+    max_monitors = (msg_header->size - sizeof(VDAgentMonitorsConfigV2)) /
+                   sizeof(VDAgentMonConfigV2);
     if (monitors_config->num_of_monitors > max_monitors) {
         goto overflow;
     }
@@ -4312,16 +4312,40 @@ static gboolean reds_use_client_monitors_config(RedsState *reds)
     return TRUE;
 }
 
-static void reds_client_monitors_config(RedsState *reds, VDAgentMonitorsConfig *monitors_config)
+static VDAgentMonitorsConfig *copy_monitors_config(VDAgentMonitorsConfigV2 *monitors_config)
+{
+    VDAgentMonitorsConfig *v1_mc = (VDAgentMonitorsConfig *) g_malloc(sizeof(VDAgentMonitorsConfig)
+        + monitors_config->num_of_monitors * sizeof(VDAgentMonConfig));
+
+    v1_mc->num_of_monitors = monitors_config->num_of_monitors;
+    v1_mc->flags = monitors_config->flags;
+
+    for (size_t i = 0; i < monitors_config->num_of_monitors; ++i) {
+        v1_mc->monitors[i].height = monitors_config->monitors[i].height;
+        v1_mc->monitors[i].width = monitors_config->monitors[i].width;
+        v1_mc->monitors[i].depth = monitors_config->monitors[i].depth;
+        v1_mc->monitors[i].x = monitors_config->monitors[i].x;
+        v1_mc->monitors[i].y = monitors_config->monitors[i].y;
+    }
+
+    return v1_mc;
+}
+
+static void reds_client_monitors_config(RedsState *reds, VDAgentMonitorsConfigV2 *monitors_config)
 {
     QXLInstance *qxl;
 
+    // VDAgentMonitorsConfig (v1) struct is used on the QXL interface, copy the data into it
+    VDAgentMonitorsConfig *v1_mc = copy_monitors_config(monitors_config);
+
     FOREACH_QXL_INSTANCE(reds, qxl) {
-        if (!red_qxl_client_monitors_config(qxl, monitors_config)) {
+        if (!red_qxl_client_monitors_config(qxl, v1_mc)) {
             /* this is a normal condition, some qemu devices might not implement it */
             spice_debug("QXLInterface::client_monitors_config failed\n");
         }
     }
+
+    g_free(v1_mc);
 }
 
 static int calc_compression_level(RedsState *reds)
-- 
2.17.1