[Spice-devel] [spice-server] cursor: Delay release of QXL guest cursor resources

[Christophe Fergeau]

There's an implicit API/ABI contract between QEMU and SPICE that SPICE
will keep the guest QXL resources alive as long as QEMU can hold a
pointer to them. This implicit contract was broken in 1c6e7cf7 "Release
cursor as soon as possible", causing crashes at migration time.
While the proper fix would be in QEMU so that spice-server does not need
to have that kind of knowledge regarding QEMU internal implementation,
this commit reverts to the pre-1c6e7cf7 behaviour to avoid a regression
while QEMU is being fixed.

https://bugzilla.redhat.com/show_bug.cgi?id=1540919

Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
---
 server/cursor-channel.c    | 16 +++++++++++++---
 server/cursor-channel.h    |  5 ++++-
 server/red-stream-device.c |  4 ++--
 server/red-worker.c        | 10 ++++++++--
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index 522261e3f..a7bee9815 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -31,6 +31,8 @@
 typedef struct RedCursorPipeItem {
     RedPipeItem base;
     RedCursorCmd *red_cursor;
+    ReleaseCommandCb release_command_cb;
+    gpointer user_data;
 } RedCursorPipeItem;
 
 struct CursorChannel
@@ -54,7 +56,9 @@ G_DEFINE_TYPE(CursorChannel, cursor_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
 
 static void cursor_pipe_item_free(RedPipeItem *pipe_item);
 
-static RedCursorPipeItem *cursor_pipe_item_new(RedCursorCmd *cmd)
+static RedCursorPipeItem *cursor_pipe_item_new(RedCursorCmd *cmd,
+                                               ReleaseCommandCb release_command_cb,
+                                               gpointer user_data)
 {
     RedCursorPipeItem *item = g_new0(RedCursorPipeItem, 1);
 
@@ -63,6 +67,8 @@ static RedCursorPipeItem *cursor_pipe_item_new(RedCursorCmd *cmd)
     red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR,
                             cursor_pipe_item_free);
     item->red_cursor = cmd;
+    item->release_command_cb = release_command_cb;
+    item->user_data = user_data;
 
     return item;
 }
@@ -73,6 +79,9 @@ static void cursor_pipe_item_free(RedPipeItem *base)
     RedCursorPipeItem *pipe_item = SPICE_UPCAST(RedCursorPipeItem, base);
 
     cursor_cmd = pipe_item->red_cursor;
+    if (pipe_item->release_command_cb) {
+        pipe_item->release_command_cb(cursor_cmd, pipe_item->user_data);
+    }
     red_put_cursor_cmd(cursor_cmd);
     g_free(cursor_cmd);
 
@@ -246,7 +255,8 @@ CursorChannel* cursor_channel_new(RedsState *server, int id,
                         NULL);
 }
 
-void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
+void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd,
+                                ReleaseCommandCb release_command_cb, gpointer user_data)
 {
     RedCursorPipeItem *cursor_pipe_item;
     bool cursor_show = false;
@@ -254,7 +264,7 @@ void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
     spice_return_if_fail(cursor);
     spice_return_if_fail(cursor_cmd);
 
-    cursor_pipe_item = cursor_pipe_item_new(cursor_cmd);
+    cursor_pipe_item = cursor_pipe_item_new(cursor_cmd, release_command_cb, user_data);
 
     switch (cursor_cmd->type) {
     case QXL_CURSOR_SET:
diff --git a/server/cursor-channel.h b/server/cursor-channel.h
index 603c2c0ac..e41e52438 100644
--- a/server/cursor-channel.h
+++ b/server/cursor-channel.h
@@ -44,6 +44,8 @@ typedef struct CursorChannelClass CursorChannelClass;
 
 GType cursor_channel_get_type(void) G_GNUC_CONST;
 
+typedef void (*ReleaseCommandCb)(RedCursorCmd *command, gpointer user_data);
+
 /**
  * Create CursorChannel.
  * Since CursorChannel is intended to be run in a separate thread,
@@ -61,7 +63,8 @@ CursorChannel* cursor_channel_new(RedsState *server, int id,
 
 void                 cursor_channel_reset       (CursorChannel *cursor);
 void                 cursor_channel_do_init     (CursorChannel *cursor);
-void                 cursor_channel_process_cmd (CursorChannel *cursor, RedCursorCmd *cursor_cmd);
+void                 cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd,
+                                                ReleaseCommandCb release_command_cb, gpointer user_data);
 void                 cursor_channel_set_mouse_mode(CursorChannel *cursor, uint32_t mode);
 
 /**
diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index e151de367..36f8b4b61 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -386,7 +386,7 @@ handle_msg_cursor_set(StreamDevice *dev, SpiceCharDeviceInstance *sin)
     if (!cmd) {
         return handle_msg_invalid(dev, sin, NULL);
     }
-    cursor_channel_process_cmd(dev->cursor_channel, cmd);
+    cursor_channel_process_cmd(dev->cursor_channel, cmd, NULL, NULL);
 
     return true;
 }
@@ -413,7 +413,7 @@ handle_msg_cursor_move(StreamDevice *dev, SpiceCharDeviceInstance *sin)
     cmd->u.position.x = move->x;
     cmd->u.position.y = move->y;
 
-    cursor_channel_process_cmd(dev->cursor_channel, cmd);
+    cursor_channel_process_cmd(dev->cursor_channel, cmd, NULL, NULL);
 
     return true;
 }
diff --git a/server/red-worker.c b/server/red-worker.c
index 387f500e8..a549becf0 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -109,6 +109,13 @@ void red_drawable_unref(RedDrawable *red_drawable)
     g_free(red_drawable);
 }
 
+static void release_cursor_cmd_cb(RedCursorCmd *cursor_cmd, gpointer user_data)
+{
+    QXLInstance *qxl = user_data;
+
+    red_qxl_release_resource(qxl, cursor_cmd->release_info_ext);
+}
+
 static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *ext)
 {
     RedCursorCmd *cursor_cmd;
@@ -118,8 +125,7 @@ static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *e
         g_free(cursor_cmd);
         return FALSE;
     }
-    red_qxl_release_resource(worker->qxl, cursor_cmd->release_info_ext);
-    cursor_channel_process_cmd(worker->cursor_channel, cursor_cmd);
+    cursor_channel_process_cmd(worker->cursor_channel, cursor_cmd, release_cursor_cmd_cb, worker->qxl);
     return TRUE;
 }
 
-- 
2.14.3