[Spice-devel] [PATCH spice-server] Release cursor as soon as possible

Frediano Ziglio fziglio at redhat.com
Tue Feb 28 15:20:09 UTC 2017


Cursor resources (basically the shape of it) was retained till
it was used however it was copied so there were no reason to not release
this resource.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/cursor-channel.c         | 14 ++++----------
 server/cursor-channel.h         |  4 ++--
 server/red-parse-qxl.c          | 10 +++++++---
 server/red-parse-qxl.h          |  4 ++--
 server/red-worker.c             |  5 +++--
 server/tests/test-qxl-parsing.c | 26 +++++++++++++++++++++++---
 6 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index 4fe3f8d..14540b8 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -29,7 +29,6 @@
 #include "red-qxl.h"
 
 typedef struct CursorItem {
-    QXLInstance *qxl;
     int refs;
     RedCursorCmd *red_cursor;
 } CursorItem;
@@ -62,14 +61,13 @@ G_DEFINE_TYPE(CursorChannel, cursor_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
 
 static void cursor_pipe_item_free(RedPipeItem *pipe_item);
 
-static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
+static CursorItem *cursor_item_new(RedCursorCmd *cmd)
 {
     CursorItem *cursor_item;
 
     spice_return_val_if_fail(cmd != NULL, NULL);
 
     cursor_item = g_new0(CursorItem, 1);
-    cursor_item->qxl = qxl;
     cursor_item->refs = 1;
     cursor_item->red_cursor = cmd;
 
@@ -96,7 +94,6 @@ static void cursor_item_unref(CursorItem *item)
         return;
 
     cursor_cmd = item->red_cursor;
-    red_qxl_release_resource(item->qxl, cursor_cmd->release_info_ext);
     red_put_cursor_cmd(cursor_cmd);
     free(cursor_cmd);
 
@@ -291,7 +288,7 @@ static void cursor_channel_send_item(RedChannelClient *rcc, RedPipeItem *pipe_it
     red_channel_client_begin_send_message(rcc);
 }
 
-CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
+CursorChannel* cursor_channel_new(RedsState *server, int id,
                                   const SpiceCoreInterfaceInternal *core)
 {
     spice_info("create cursor channel");
@@ -299,9 +296,8 @@ CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
                         "spice-server", server,
                         "core-interface", core,
                         "channel-type", SPICE_CHANNEL_CURSOR,
-                        "id", qxl->id,
+                        "id", id,
                         "migration-flags", 0,
-                        "qxl", qxl,
                         "handle-acks", TRUE,
                         NULL);
 }
@@ -310,13 +306,11 @@ void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
 {
     CursorItem *cursor_item;
     int cursor_show = FALSE;
-    QXLInstance *qxl;
 
     spice_return_if_fail(cursor);
     spice_return_if_fail(cursor_cmd);
 
-    qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(cursor));
-    cursor_item = cursor_item_new(qxl, cursor_cmd);
+    cursor_item = cursor_item_new(cursor_cmd);
 
     switch (cursor_cmd->type) {
     case QXL_CURSOR_SET:
diff --git a/server/cursor-channel.h b/server/cursor-channel.h
index ec9d44f..8f2afb6 100644
--- a/server/cursor-channel.h
+++ b/server/cursor-channel.h
@@ -55,8 +55,8 @@ GType cursor_channel_get_type(void) G_GNUC_CONST;
  * provided as helper functions and should only be called from the
  * CursorChannel thread.
  */
-CursorChannel*       cursor_channel_new         (RedsState *server, QXLInstance *qxl,
-                                                 const SpiceCoreInterfaceInternal *core);
+CursorChannel* cursor_channel_new(RedsState *server, int id,
+                                  const SpiceCoreInterfaceInternal *core);
 
 /**
  * Cause the channel to disconnect all clients
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 89cb120..5ed36df 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -27,6 +27,7 @@
 #include "red-common.h"
 #include "memslot.h"
 #include "red-parse-qxl.h"
+#include "red-qxl.h"
 
 /* Max size in bytes for any data field used in a QXL command.
  * This will for example be useful to prevent the guest from saturating the
@@ -1470,8 +1471,10 @@ static void red_put_cursor(SpiceCursor *red)
 }
 
 int red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
-                       RedCursorCmd *red, QXLPHYSICAL addr)
+                       RedCursorCmd *red, QXLPHYSICAL addr,
+                       QXLInstance *qxl_instance)
 {
+    QXLReleaseInfoExt release_info_ext;
     QXLCursorCmd *qxl;
     int error;
 
@@ -1479,8 +1482,8 @@ int red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
     if (error) {
         return error;
     }
-    red->release_info_ext.info      = &qxl->release_info;
-    red->release_info_ext.group_id  = group_id;
+    release_info_ext.info      = &qxl->release_info;
+    release_info_ext.group_id  = group_id;
 
     red->type = qxl->type;
     switch (red->type) {
@@ -1497,6 +1500,7 @@ int red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
         red->u.trail.frequency = qxl->u.trail.frequency;
         break;
     }
+    red_qxl_release_resource(qxl_instance, release_info_ext);
     return error;
 }
 
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 86a2d93..9aa1077 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -99,7 +99,6 @@ typedef struct RedSurfaceCmd {
 } RedSurfaceCmd;
 
 typedef struct RedCursorCmd {
-    QXLReleaseInfoExt release_info_ext;
     uint8_t type;
     union {
         struct {
@@ -138,7 +137,8 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
 void red_put_surface_cmd(RedSurfaceCmd *red);
 
 int red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
-                       RedCursorCmd *red, QXLPHYSICAL addr);
+                       RedCursorCmd *red, QXLPHYSICAL addr,
+                       QXLInstance *qxl);
 void red_put_cursor_cmd(RedCursorCmd *red);
 
 #endif
diff --git a/server/red-worker.c b/server/red-worker.c
index e5adbaa..20b15bc 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -107,7 +107,8 @@ static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *e
     RedCursorCmd *cursor_cmd;
 
     cursor_cmd = spice_new0(RedCursorCmd, 1);
-    if (red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data)) {
+    if (red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data,
+                           worker->qxl)) {
         free(cursor_cmd);
         return FALSE;
     }
@@ -1364,7 +1365,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
 
     worker->event_timeout = INF_EVENT_WAIT;
 
-    worker->cursor_channel = cursor_channel_new(reds, qxl,
+    worker->cursor_channel = cursor_channel_new(reds, qxl->id,
                                                 &worker->core);
     channel = RED_CHANNEL(worker->cursor_channel);
     red_channel_init_stat_node(channel, &worker->stat, "cursor_channel");
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index dd99150..6d49aab 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -71,8 +71,28 @@ create_chunk(size_t prefix, uint32_t size, QXLDataChunk* prev, int fill)
     return ptr;
 }
 
+static void release_resource(SPICE_GNUC_UNUSED QXLInstance *qin,
+                             SPICE_GNUC_UNUSED struct QXLReleaseInfoExt release_info)
+{
+}
+
+static QXLInterface display_sif = {
+    .base = {
+        .type = SPICE_INTERFACE_QXL,
+        .description = "test",
+        .major_version = SPICE_INTERFACE_QXL_MAJOR,
+        .minor_version = SPICE_INTERFACE_QXL_MINOR
+    },
+    .release_resource = release_resource,
+};
+
+static QXLInstance display;
+
 int main(int argc, char **argv)
 {
+    display.base.sif = &display_sif.base;
+    display.id = 0;
+
     RedMemSlotInfo mem_info;
     memslot_info_init(&mem_info, 1 /* groups */, 1 /* slots */, 1, 1, 0);
     memslot_info_add_slot(&mem_info, 0, 0, 0 /* delta */, 0 /* start */, ~0ul /* end */, 0 /* generation */);
@@ -138,7 +158,7 @@ int main(int argc, char **argv)
 
     cursor_cmd.u.set.shape = to_physical(cursor);
 
-    if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd)))
+    if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd), &display))
         failure();
     free(red_cursor_cmd.u.set.shape.data);
     free(cursor);
@@ -160,7 +180,7 @@ int main(int argc, char **argv)
     cursor_cmd.u.set.shape = to_physical(cursor);
 
     memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
-    if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd))) {
+    if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd), &display)) {
         /* function does not return errors so there should be no data */
         assert(red_cursor_cmd.type == QXL_CURSOR_SET);
         assert(red_cursor_cmd.u.set.position.x == 0);
@@ -187,7 +207,7 @@ int main(int argc, char **argv)
     cursor_cmd.u.set.shape = to_physical(cursor);
 
     memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
-    if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd))) {
+    if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd), &display)) {
         /* function does not return errors so there should be no data */
         assert(red_cursor_cmd.type == QXL_CURSOR_SET);
         assert(red_cursor_cmd.u.set.position.x == 0);
-- 
2.9.3



More information about the Spice-devel mailing list