[Spice-commits] 11 commits - server/cursor-channel.c server/display-channel.c server/display-channel.h server/display-channel-private.h server/red-parse-qxl.c server/red-parse-qxl.h server/red-worker.c server/tests

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Thu Dec 6 13:04:47 UTC 2018


 server/cursor-channel.c          |    9 -
 server/display-channel-private.h |    7 +
 server/display-channel.c         |   21 +--
 server/display-channel.h         |    2 
 server/red-parse-qxl.c           |  221 ++++++++++++++++++++++++++++++++++-----
 server/red-parse-qxl.h           |   51 ++++-----
 server/red-worker.c              |   84 ++++++--------
 server/tests/test-qxl-parsing.c  |   53 +++++----
 8 files changed, 310 insertions(+), 138 deletions(-)

New commits:
commit 1c32e680721eb91c3f7df86536a3fe250e0fd89c
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:13 2018 +0100

    qxl: Release QXL resources in red_put_surface_cmd
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/display-channel.c b/server/display-channel.c
index 91ef7221..e68ed10f 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -286,7 +286,6 @@ static void stop_streams(DisplayChannel *display)
 void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id)
 {
     RedSurface *surface = &display->priv->surfaces[surface_id];
-    QXLInstance *qxl = display->priv->qxl;
     DisplayChannelClient *dcc;
 
     if (--surface->refs != 0) {
@@ -301,12 +300,10 @@ void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id)
 
     surface->context.canvas->ops->destroy(surface->context.canvas);
     if (surface->create_cmd != NULL) {
-        red_qxl_release_resource(qxl, surface->create_cmd->release_info_ext);
         red_surface_cmd_unref(surface->create_cmd);
         surface->create_cmd = NULL;
     }
     if (surface->destroy_cmd != NULL) {
-        red_qxl_release_resource(qxl, surface->destroy_cmd->release_info_ext);
         red_surface_cmd_unref(surface->destroy_cmd);
         surface->destroy_cmd = NULL;
     }
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index c1812af1..21214f4e 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1444,8 +1444,9 @@ bool red_validate_surface(uint32_t width, uint32_t height,
     return true;
 }
 
-static bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
+static bool red_get_surface_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
                                 RedSurfaceCmd *red, QXLPHYSICAL addr)
+
 {
     QXLSurfaceCmd *qxl;
     uint64_t size;
@@ -1454,6 +1455,7 @@ static bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
     if (qxl == NULL) {
         return false;
     }
+    red->qxl = qxl_instance;
     red->release_info_ext.info      = &qxl->release_info;
     red->release_info_ext.group_id  = group_id;
 
@@ -1486,7 +1488,9 @@ static bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
 
 static void red_put_surface_cmd(RedSurfaceCmd *red)
 {
-    /* nothing yet */
+    if (red->qxl) {
+        red_qxl_release_resource(red->qxl, red->release_info_ext);
+    }
 }
 
 RedSurfaceCmd *red_surface_cmd_new(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
@@ -1498,8 +1502,8 @@ RedSurfaceCmd *red_surface_cmd_new(QXLInstance *qxl_instance, RedMemSlotInfo *sl
 
     cmd->refs = 1;
 
-    if (!red_get_surface_cmd(slots, group_id, cmd, addr)) {
-        g_free(cmd);
+    if (!red_get_surface_cmd(qxl_instance, slots, group_id, cmd, addr)) {
+        red_surface_cmd_unref(cmd);
         return NULL;
     }
 
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 7abd3847..61c71d6e 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -85,6 +85,7 @@ typedef struct RedSurfaceCreate {
 } RedSurfaceCreate;
 
 typedef struct RedSurfaceCmd {
+    QXLInstance *qxl;
     QXLReleaseInfoExt release_info_ext;
     int refs;
     uint32_t surface_id;
commit 473656d58eaf6864887cfe97e3db7ac82058e99a
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:12 2018 +0100

    display-channel: Store full RedSurfaceCmd, not just QXLReleaseInfoExt
    
    Now that we have a refcounted RedSurfaceCmd, we can store the command
    itself in DisplayChannel rather than copying QXLReleaseInfoExt. This
    will let us move the release of the QXL guest resources in red-parse-qxl
    in the next commit.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index 27f0a019..58179531 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -52,7 +52,12 @@ typedef struct RedSurface {
     QRegion draw_dirty_region;
 
     //fix me - better handling here
-    QXLReleaseInfoExt create, destroy;
+    /* 'create_cmd' holds surface data through a pointer to guest memory, it
+     * must be valid as long as the surface is valid */
+    RedSurfaceCmd *create_cmd;
+    /* QEMU expects the guest data for the command to be valid as long as the
+     * surface is valid */
+    RedSurfaceCmd *destroy_cmd;
 } RedSurface;
 
 typedef struct MonitorsConfig {
diff --git a/server/display-channel.c b/server/display-channel.c
index 118f795f..91ef7221 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -300,11 +300,15 @@ void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id)
     spice_assert(surface->context.canvas);
 
     surface->context.canvas->ops->destroy(surface->context.canvas);
-    if (surface->create.info) {
-        red_qxl_release_resource(qxl, surface->create);
+    if (surface->create_cmd != NULL) {
+        red_qxl_release_resource(qxl, surface->create_cmd->release_info_ext);
+        red_surface_cmd_unref(surface->create_cmd);
+        surface->create_cmd = NULL;
     }
-    if (surface->destroy.info) {
-        red_qxl_release_resource(qxl, surface->destroy);
+    if (surface->destroy_cmd != NULL) {
+        red_qxl_release_resource(qxl, surface->destroy_cmd->release_info_ext);
+        red_surface_cmd_unref(surface->destroy_cmd);
+        surface->destroy_cmd = NULL;
     }
 
     region_destroy(&surface->draw_dirty_region);
@@ -2161,8 +2165,8 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id
         }
         memset(data, 0, height*abs(stride));
     }
-    surface->create.info = NULL;
-    surface->destroy.info = NULL;
+    g_warn_if_fail(surface->create_cmd == NULL);
+    g_warn_if_fail(surface->destroy_cmd == NULL);
     ring_init(&surface->current);
     ring_init(&surface->current_list);
     ring_init(&surface->depend_on_me);
@@ -2306,7 +2310,7 @@ display_channel_constructed(GObject *object)
 }
 
 void display_channel_process_surface_cmd(DisplayChannel *display,
-                                         const RedSurfaceCmd *surface_cmd,
+                                         RedSurfaceCmd *surface_cmd,
                                          int loadvm)
 {
     uint32_t surface_id;
@@ -2342,7 +2346,7 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
                                        reloaded_surface,
                                        // reloaded surfaces will be sent on demand
                                        !reloaded_surface);
-        surface->create = surface_cmd->release_info_ext;
+        surface->create_cmd = red_surface_cmd_ref(surface_cmd);
         break;
     }
     case QXL_SURFACE_CMD_DESTROY:
@@ -2350,7 +2354,7 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
             spice_warning("avoiding destroying a surface twice");
             break;
         }
-        surface->destroy = surface_cmd->release_info_ext;
+        surface->destroy_cmd = red_surface_cmd_ref(surface_cmd);
         display_channel_destroy_surface(display, surface_id);
         break;
     default:
diff --git a/server/display-channel.h b/server/display-channel.h
index 455e224f..5fcf7035 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -147,7 +147,7 @@ void                       display_channel_process_draw              (DisplayCha
                                                                       RedDrawable *red_drawable,
                                                                       uint32_t process_commands_generation);
 void                       display_channel_process_surface_cmd       (DisplayChannel *display,
-                                                                      const RedSurfaceCmd *surface_cmd,
+                                                                      RedSurfaceCmd *surface_cmd,
                                                                       int loadvm);
 void                       display_channel_update_compression        (DisplayChannel *display,
                                                                       DisplayChannelClient *dcc);
commit 8669c933fff70ce2c75498c4ef04f6bb7bd20d36
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:11 2018 +0100

    qxl: Add red_surface_cmd_{new, ref, unref} helpers
    
    Currently, RedWorker is using stack-allocated variables for RedSurfaceCmd.
    Surface commands are rare enough that we can dynamically allocate them
    instead, and make the API in red-parse-qxl.h consistent with how other
    QXL commands are handled.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index efdd06e2..c1812af1 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1444,8 +1444,8 @@ bool red_validate_surface(uint32_t width, uint32_t height,
     return true;
 }
 
-bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
-                         RedSurfaceCmd *red, QXLPHYSICAL addr)
+static bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
+                                RedSurfaceCmd *red, QXLPHYSICAL addr)
 {
     QXLSurfaceCmd *qxl;
     uint64_t size;
@@ -1484,11 +1484,43 @@ bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
     return true;
 }
 
-void red_put_surface_cmd(RedSurfaceCmd *red)
+static void red_put_surface_cmd(RedSurfaceCmd *red)
 {
     /* nothing yet */
 }
 
+RedSurfaceCmd *red_surface_cmd_new(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
+                                   int group_id, QXLPHYSICAL addr)
+{
+    RedSurfaceCmd *cmd;
+
+    cmd = g_new0(RedSurfaceCmd, 1);
+
+    cmd->refs = 1;
+
+    if (!red_get_surface_cmd(slots, group_id, cmd, addr)) {
+        g_free(cmd);
+        return NULL;
+    }
+
+    return cmd;
+}
+
+RedSurfaceCmd *red_surface_cmd_ref(RedSurfaceCmd *cmd)
+{
+    cmd->refs++;
+    return cmd;
+}
+
+void red_surface_cmd_unref(RedSurfaceCmd *cmd)
+{
+    if (--cmd->refs) {
+        return;
+    }
+    red_put_surface_cmd(cmd);
+    g_free(cmd);
+}
+
 static bool red_get_cursor(RedMemSlotInfo *slots, int group_id,
                            SpiceCursor *red, QXLPHYSICAL addr)
 {
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 6fb7b3c5..7abd3847 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -86,6 +86,7 @@ typedef struct RedSurfaceCreate {
 
 typedef struct RedSurfaceCmd {
     QXLReleaseInfoExt release_info_ext;
+    int refs;
     uint32_t surface_id;
     uint8_t type;
     uint32_t flags;
@@ -134,9 +135,10 @@ void red_message_unref(RedMessage *red);
 bool red_validate_surface(uint32_t width, uint32_t height,
                           int32_t stride, uint32_t format);
 
-bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
-                         RedSurfaceCmd *red, QXLPHYSICAL addr);
-void red_put_surface_cmd(RedSurfaceCmd *red);
+RedSurfaceCmd *red_surface_cmd_new(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
+                                   int group_id, QXLPHYSICAL addr);
+RedSurfaceCmd *red_surface_cmd_ref(RedSurfaceCmd *cmd);
+void red_surface_cmd_unref(RedSurfaceCmd *cmd);
 
 RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
                                  int group_id, QXLPHYSICAL addr);
diff --git a/server/red-worker.c b/server/red-worker.c
index 04ad4c9d..f016943a 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -152,16 +152,17 @@ static int red_process_cursor(RedWorker *worker, int *ring_is_empty)
 
 static gboolean red_process_surface_cmd(RedWorker *worker, QXLCommandExt *ext, gboolean loadvm)
 {
-    RedSurfaceCmd surface_cmd;
+    RedSurfaceCmd *surface_cmd;
 
-    if (!red_get_surface_cmd(&worker->mem_slots, ext->group_id, &surface_cmd, ext->cmd.data)) {
-        return FALSE;
+    surface_cmd = red_surface_cmd_new(worker->qxl, &worker->mem_slots,
+                                      ext->group_id, ext->cmd.data);
+    if (surface_cmd == NULL) {
+        return false;
     }
-    display_channel_process_surface_cmd(worker->display_channel, &surface_cmd, loadvm);
-    // display_channel_process_surface_cmd() takes ownership of 'release_info_ext',
-    // we don't need to release it ourselves
-    red_put_surface_cmd(&surface_cmd);
-    return TRUE;
+    display_channel_process_surface_cmd(worker->display_channel, surface_cmd, loadvm);
+    red_surface_cmd_unref(surface_cmd);
+
+    return true;
 }
 
 static int red_process_display(RedWorker *worker, int *ring_is_empty)
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index 2314d768..324f7fdc 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -88,14 +88,16 @@ static void deinit_qxl_surface(QXLSurfaceCmd *qxl)
 static void test_no_issues(void)
 {
     RedMemSlotInfo mem_info;
-    RedSurfaceCmd cmd;
+    RedSurfaceCmd *cmd;
     QXLSurfaceCmd qxl;
 
     init_meminfo(&mem_info);
     init_qxl_surface(&qxl);
 
     /* try to create a surface with no issues, should succeed */
-    g_assert_true(red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)));
+    cmd = red_surface_cmd_new(NULL, &mem_info, 0, to_physical(&qxl));
+    g_assert_nonnull(cmd);
+    red_surface_cmd_unref(cmd);
 
     deinit_qxl_surface(&qxl);
     memslot_info_destroy(&mem_info);
@@ -104,7 +106,7 @@ static void test_no_issues(void)
 static void test_stride_too_small(void)
 {
     RedMemSlotInfo mem_info;
-    RedSurfaceCmd cmd;
+    RedSurfaceCmd *cmd;
     QXLSurfaceCmd qxl;
 
     init_meminfo(&mem_info);
@@ -115,7 +117,8 @@ static void test_stride_too_small(void)
      * This can be used to cause buffer overflows so refuse it.
      */
     qxl.u.surface_create.stride = 256;
-    g_assert_false(red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)));
+    cmd = red_surface_cmd_new(NULL, &mem_info, 0, to_physical(&qxl));
+    g_assert_null(cmd);
 
     deinit_qxl_surface(&qxl);
     memslot_info_destroy(&mem_info);
@@ -124,7 +127,7 @@ static void test_stride_too_small(void)
 static void test_too_big_image(void)
 {
     RedMemSlotInfo mem_info;
-    RedSurfaceCmd cmd;
+    RedSurfaceCmd *cmd;
     QXLSurfaceCmd qxl;
 
     init_meminfo(&mem_info);
@@ -140,7 +143,8 @@ static void test_too_big_image(void)
     qxl.u.surface_create.stride = 0x08000004 * 4;
     qxl.u.surface_create.width = 0x08000004;
     qxl.u.surface_create.height = 0x40000020;
-    g_assert_false(red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)));
+    cmd = red_surface_cmd_new(NULL, &mem_info, 0, to_physical(&qxl));
+    g_assert_null(cmd);
 
     deinit_qxl_surface(&qxl);
     memslot_info_destroy(&mem_info);
commit decfe65d733d4d14cebb5876e7f84ce706b4a843
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:10 2018 +0100

    qxl: Add red_update_cmd_{new, ref, unref} helpers
    
    Currently, RedUpdateCmd are allocated on the stack, and then
    initialized/uninitialized with red_{get,put}_update_cmd
    This makes the API inconsistent with what is being done for RedDrawable,
    RedCursor and RedMessage. QXLUpdateCmd are not occurring very often,
    we can dynamically allocate them instead, and get a consistent API.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 79b3cee9..efdd06e2 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1268,9 +1268,8 @@ void red_drawable_unref(RedDrawable *red_drawable)
     g_free(red_drawable);
 }
 
-bool red_get_update_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
-                        int group_id, RedUpdateCmd *red,
-                        QXLPHYSICAL addr)
+static bool red_get_update_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
+                               RedUpdateCmd *red, QXLPHYSICAL addr)
 {
     QXLUpdateCmd *qxl;
 
@@ -1288,13 +1287,45 @@ bool red_get_update_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
     return true;
 }
 
-void red_put_update_cmd(RedUpdateCmd *red)
+static void red_put_update_cmd(RedUpdateCmd *red)
 {
     if (red->qxl != NULL) {
         red_qxl_release_resource(red->qxl, red->release_info_ext);
     }
 }
 
+RedUpdateCmd *red_update_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                                 int group_id, QXLPHYSICAL addr)
+{
+    RedUpdateCmd *red;
+
+    red = g_new0(RedUpdateCmd, 1);
+
+    red->refs = 1;
+
+    if (!red_get_update_cmd(qxl, slots, group_id, red, addr)) {
+        red_update_cmd_unref(red);
+        return NULL;
+    }
+
+    return red;
+}
+
+RedUpdateCmd *red_update_cmd_ref(RedUpdateCmd *red)
+{
+    red->refs++;
+    return red;
+}
+
+void red_update_cmd_unref(RedUpdateCmd *red)
+{
+    if (--red->refs) {
+        return;
+    }
+    red_put_update_cmd(red);
+    g_free(red);
+}
+
 static bool red_get_message(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
                             RedMessage *red, QXLPHYSICAL addr)
 {
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 22323ffa..6fb7b3c5 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -62,6 +62,7 @@ typedef struct RedDrawable {
 typedef struct RedUpdateCmd {
     QXLInstance *qxl;
     QXLReleaseInfoExt release_info_ext;
+    int refs;
     SpiceRect area;
     uint32_t update_id;
     uint32_t surface_id;
@@ -120,9 +121,10 @@ RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
 RedDrawable *red_drawable_ref(RedDrawable *drawable);
 void red_drawable_unref(RedDrawable *red_drawable);
 
-bool red_get_update_cmd(QXLInstance *qxl, RedMemSlotInfo *slots, int group_id,
-                        RedUpdateCmd *red, QXLPHYSICAL addr);
-void red_put_update_cmd(RedUpdateCmd *red);
+RedUpdateCmd *red_update_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                                 int group_id, QXLPHYSICAL addr);
+RedUpdateCmd *red_update_cmd_ref(RedUpdateCmd *red);
+void red_update_cmd_unref(RedUpdateCmd *red);
 
 RedMessage *red_message_new(QXLInstance *qxl, RedMemSlotInfo *slots,
                             int group_id, QXLPHYSICAL addr);
diff --git a/server/red-worker.c b/server/red-worker.c
index c9dac36b..04ad4c9d 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -213,19 +213,20 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
             break;
         }
         case QXL_CMD_UPDATE: {
-            RedUpdateCmd update;
+            RedUpdateCmd *update;
 
-            if (!red_get_update_cmd(worker->qxl, &worker->mem_slots, ext_cmd.group_id,
-                                    &update, ext_cmd.cmd.data)) {
+            update = red_update_cmd_new(worker->qxl, &worker->mem_slots,
+                                        ext_cmd.group_id, ext_cmd.cmd.data);
+            if (update == NULL) {
                 break;
             }
-            if (!display_channel_validate_surface(worker->display_channel, update.surface_id)) {
+            if (!display_channel_validate_surface(worker->display_channel, update->surface_id)) {
                 spice_warning("Invalid surface in QXL_CMD_UPDATE");
             } else {
-                display_channel_draw(worker->display_channel, &update.area, update.surface_id);
-                red_qxl_notify_update(worker->qxl, update.update_id);
+                display_channel_draw(worker->display_channel, &update->area, update->surface_id);
+                red_qxl_notify_update(worker->qxl, update->update_id);
             }
-            red_put_update_cmd(&update);
+            red_update_cmd_unref(update);
             break;
         }
         case QXL_CMD_MESSAGE: {
commit 74663be7fc57111c974adb4c7f4b95c33710ad39
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:09 2018 +0100

    qxl: Release QXL resources in red_put_update_cmd
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 09b01ac4..79b3cee9 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1268,8 +1268,9 @@ void red_drawable_unref(RedDrawable *red_drawable)
     g_free(red_drawable);
 }
 
-bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
-                        RedUpdateCmd *red, QXLPHYSICAL addr)
+bool red_get_update_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
+                        int group_id, RedUpdateCmd *red,
+                        QXLPHYSICAL addr)
 {
     QXLUpdateCmd *qxl;
 
@@ -1277,10 +1278,10 @@ bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
     if (qxl == NULL) {
         return false;
     }
+    red->qxl = qxl_instance;
     red->release_info_ext.info     = &qxl->release_info;
     red->release_info_ext.group_id = group_id;
 
-
     red_get_rect_ptr(&red->area, &qxl->area);
     red->update_id  = qxl->update_id;
     red->surface_id = qxl->surface_id;
@@ -1289,7 +1290,9 @@ bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
 
 void red_put_update_cmd(RedUpdateCmd *red)
 {
-    /* nothing yet */
+    if (red->qxl != NULL) {
+        red_qxl_release_resource(red->qxl, red->release_info_ext);
+    }
 }
 
 static bool red_get_message(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 8a0e57e8..22323ffa 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -60,6 +60,7 @@ typedef struct RedDrawable {
 } RedDrawable;
 
 typedef struct RedUpdateCmd {
+    QXLInstance *qxl;
     QXLReleaseInfoExt release_info_ext;
     SpiceRect area;
     uint32_t update_id;
@@ -119,7 +120,7 @@ RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
 RedDrawable *red_drawable_ref(RedDrawable *drawable);
 void red_drawable_unref(RedDrawable *red_drawable);
 
-bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
+bool red_get_update_cmd(QXLInstance *qxl, RedMemSlotInfo *slots, int group_id,
                         RedUpdateCmd *red, QXLPHYSICAL addr);
 void red_put_update_cmd(RedUpdateCmd *red);
 
diff --git a/server/red-worker.c b/server/red-worker.c
index 056e4238..c9dac36b 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -215,7 +215,7 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
         case QXL_CMD_UPDATE: {
             RedUpdateCmd update;
 
-            if (!red_get_update_cmd(&worker->mem_slots, ext_cmd.group_id,
+            if (!red_get_update_cmd(worker->qxl, &worker->mem_slots, ext_cmd.group_id,
                                     &update, ext_cmd.cmd.data)) {
                 break;
             }
@@ -225,7 +225,6 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
                 display_channel_draw(worker->display_channel, &update.area, update.surface_id);
                 red_qxl_notify_update(worker->qxl, update.update_id);
             }
-            red_qxl_release_resource(worker->qxl, update.release_info_ext);
             red_put_update_cmd(&update);
             break;
         }
commit 5840ba2a8940f2e32cdd9a1103151d1f6ad63c59
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:08 2018 +0100

    qxl: Add red_message_{new, ref, unref} helpers
    
    Currently, RedMessage are allocated on the stack, and then
    initialized/uninitialized with red_{get,put}_message
    This makes the API inconsistent with what is being done for RedDrawable
    and RedCursor. Since QXLMessage is just a (mostly unused/unsecure) debugging tool,
    we can dynamically allocate it instead, and get a consistent API.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index a7a8cd8d..09b01ac4 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1292,8 +1292,8 @@ void red_put_update_cmd(RedUpdateCmd *red)
     /* nothing yet */
 }
 
-bool red_get_message(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
-                     RedMessage *red, QXLPHYSICAL addr)
+static bool red_get_message(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
+                            RedMessage *red, QXLPHYSICAL addr)
 {
     QXLMessage *qxl;
     int memslot_id;
@@ -1325,13 +1325,45 @@ bool red_get_message(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group
     return true;
 }
 
-void red_put_message(RedMessage *red)
+static void red_put_message(RedMessage *red)
 {
     if (red->qxl != NULL) {
         red_qxl_release_resource(red->qxl, red->release_info_ext);
     }
 }
 
+RedMessage *red_message_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                            int group_id, QXLPHYSICAL addr)
+{
+    RedMessage *red;
+
+    red = g_new0(RedMessage, 1);
+
+    red->refs = 1;
+
+    if (!red_get_message(qxl, slots, group_id, red, addr)) {
+        red_message_unref(red);
+        return NULL;
+    }
+
+    return red;
+}
+
+RedMessage *red_message_ref(RedMessage *red)
+{
+    red->refs++;
+    return red;
+}
+
+void red_message_unref(RedMessage *red)
+{
+    if (--red->refs) {
+        return;
+    }
+    red_put_message(red);
+    g_free(red);
+}
+
 static unsigned int surface_format_to_bpp(uint32_t format)
 {
     switch (format) {
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index ecf7b157..8a0e57e8 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -69,6 +69,7 @@ typedef struct RedUpdateCmd {
 typedef struct RedMessage {
     QXLInstance *qxl;
     QXLReleaseInfoExt release_info_ext;
+    int refs;
     int len;
     uint8_t *data;
 } RedMessage;
@@ -122,9 +123,10 @@ bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
                         RedUpdateCmd *red, QXLPHYSICAL addr);
 void red_put_update_cmd(RedUpdateCmd *red);
 
-bool red_get_message(QXLInstance *qxl, RedMemSlotInfo *slots, int group_id,
-                     RedMessage *red, QXLPHYSICAL addr);
-void red_put_message(RedMessage *red);
+RedMessage *red_message_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                            int group_id, QXLPHYSICAL addr);
+RedMessage *red_message_ref(RedMessage *red);
+void red_message_unref(RedMessage *red);
 
 bool red_validate_surface(uint32_t width, uint32_t height,
                           int32_t stride, uint32_t format);
diff --git a/server/red-worker.c b/server/red-worker.c
index 942c29f1..056e4238 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -230,16 +230,17 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
             break;
         }
         case QXL_CMD_MESSAGE: {
-            RedMessage message;
+            RedMessage *message;
 
-            if (!red_get_message(worker->qxl, &worker->mem_slots, ext_cmd.group_id,
-                                 &message, ext_cmd.cmd.data)) {
+            message = red_message_new(worker->qxl, &worker->mem_slots,
+                                      ext_cmd.group_id, ext_cmd.cmd.data);
+            if (message == NULL) {
                 break;
             }
 #ifdef DEBUG
             spice_warning("MESSAGE: %.*s", message.len, message.data);
 #endif
-            red_put_message(&message);
+            red_message_unref(message);
             break;
         }
         case QXL_CMD_SURFACE:
commit 60c425011ca9a67ad9e3a958c3b3625d9cedaf34
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:07 2018 +0100

    qxl: Release QXL resource in red_put_message
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index db7b51b7..a7a8cd8d 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1292,7 +1292,7 @@ void red_put_update_cmd(RedUpdateCmd *red)
     /* nothing yet */
 }
 
-bool red_get_message(RedMemSlotInfo *slots, int group_id,
+bool red_get_message(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
                      RedMessage *red, QXLPHYSICAL addr)
 {
     QXLMessage *qxl;
@@ -1310,6 +1310,7 @@ bool red_get_message(RedMemSlotInfo *slots, int group_id,
     if (qxl == NULL) {
         return false;
     }
+    red->qxl = qxl_instance;
     red->release_info_ext.info      = &qxl->release_info;
     red->release_info_ext.group_id  = group_id;
     red->data                       = qxl->data;
@@ -1326,7 +1327,9 @@ bool red_get_message(RedMemSlotInfo *slots, int group_id,
 
 void red_put_message(RedMessage *red)
 {
-    /* nothing yet */
+    if (red->qxl != NULL) {
+        red_qxl_release_resource(red->qxl, red->release_info_ext);
+    }
 }
 
 static unsigned int surface_format_to_bpp(uint32_t format)
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index cb1c1b9f..ecf7b157 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -67,6 +67,7 @@ typedef struct RedUpdateCmd {
 } RedUpdateCmd;
 
 typedef struct RedMessage {
+    QXLInstance *qxl;
     QXLReleaseInfoExt release_info_ext;
     int len;
     uint8_t *data;
@@ -121,7 +122,7 @@ bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
                         RedUpdateCmd *red, QXLPHYSICAL addr);
 void red_put_update_cmd(RedUpdateCmd *red);
 
-bool red_get_message(RedMemSlotInfo *slots, int group_id,
+bool red_get_message(QXLInstance *qxl, RedMemSlotInfo *slots, int group_id,
                      RedMessage *red, QXLPHYSICAL addr);
 void red_put_message(RedMessage *red);
 
diff --git a/server/red-worker.c b/server/red-worker.c
index 266fb04a..942c29f1 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -232,14 +232,13 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
         case QXL_CMD_MESSAGE: {
             RedMessage message;
 
-            if (!red_get_message(&worker->mem_slots, ext_cmd.group_id,
+            if (!red_get_message(worker->qxl, &worker->mem_slots, ext_cmd.group_id,
                                  &message, ext_cmd.cmd.data)) {
                 break;
             }
 #ifdef DEBUG
             spice_warning("MESSAGE: %.*s", message.len, message.data);
 #endif
-            red_qxl_release_resource(worker->qxl, message.release_info_ext);
             red_put_message(&message);
             break;
         }
commit fb3c602d7564721dc3ec8d0bee4166822bb06295
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:06 2018 +0100

    qxl: Add red_cursor_cmd_{new, ref, unref} helpers
    
    Currently, the cursor channel is allocating RedCursorCmd instances itself, and then
    calling into red-parse-qxl.h to initialize it, and doing something
    similar when releasing the data. This commit moves this common code to
    red-parse-qxl.[ch]
    The ref/unref are not strictly needed, red_cursor_cmd_free() would
    currently be enough, but this makes the API consistent with
    red_drawable_{new,ref,unref}.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index ada1d62a..9dd69fa2 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -26,7 +26,6 @@
 #include "cursor-channel.h"
 #include "cursor-channel-client.h"
 #include "reds.h"
-#include "red-qxl.h"
 
 typedef struct RedCursorPipeItem {
     RedPipeItem base;
@@ -62,20 +61,16 @@ 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->red_cursor = red_cursor_cmd_ref(cmd);
 
     return item;
 }
 
 static void cursor_pipe_item_free(RedPipeItem *base)
 {
-    RedCursorCmd *cursor_cmd;
     RedCursorPipeItem *pipe_item = SPICE_UPCAST(RedCursorPipeItem, base);
 
-    cursor_cmd = pipe_item->red_cursor;
-    red_put_cursor_cmd(cursor_cmd);
-    g_free(cursor_cmd);
-
+    red_cursor_cmd_unref(pipe_item->red_cursor);
     g_free(pipe_item);
 }
 
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 229c8669..db7b51b7 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1465,8 +1465,9 @@ static void red_put_cursor(SpiceCursor *red)
     g_free(red->data);
 }
 
-bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
-                        RedCursorCmd *red, QXLPHYSICAL addr)
+static bool red_get_cursor_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
+                               int group_id, RedCursorCmd *red,
+                               QXLPHYSICAL addr)
 {
     QXLCursorCmd *qxl;
 
@@ -1474,6 +1475,7 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
     if (qxl == NULL) {
         return false;
     }
+    red->qxl = qxl_instance;
     red->release_info_ext.info      = &qxl->release_info;
     red->release_info_ext.group_id  = group_id;
 
@@ -1494,7 +1496,24 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
     return true;
 }
 
-void red_put_cursor_cmd(RedCursorCmd *red)
+RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                                 int group_id, QXLPHYSICAL addr)
+{
+    RedCursorCmd *cmd;
+
+    cmd = g_new0(RedCursorCmd, 1);
+
+    cmd->refs = 1;
+
+    if (!red_get_cursor_cmd(qxl, slots, group_id, cmd, addr)) {
+        red_cursor_cmd_unref(cmd);
+        return NULL;
+    }
+
+    return cmd;
+}
+
+static void red_put_cursor_cmd(RedCursorCmd *red)
 {
     switch (red->type) {
     case QXL_CURSOR_SET:
@@ -1505,3 +1524,18 @@ void red_put_cursor_cmd(RedCursorCmd *red)
         red_qxl_release_resource(red->qxl, red->release_info_ext);
     }
 }
+
+RedCursorCmd *red_cursor_cmd_ref(RedCursorCmd *red)
+{
+    red->refs++;
+    return red;
+}
+
+void red_cursor_cmd_unref(RedCursorCmd *red)
+{
+    if (--red->refs) {
+        return;
+    }
+    red_put_cursor_cmd(red);
+    g_free(red);
+}
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index cbb88019..cb1c1b9f 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -93,6 +93,7 @@ typedef struct RedSurfaceCmd {
 typedef struct RedCursorCmd {
     QXLInstance *qxl;
     QXLReleaseInfoExt release_info_ext;
+    int refs;
     uint8_t type;
     union {
         struct {
@@ -131,8 +132,9 @@ bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
                          RedSurfaceCmd *red, QXLPHYSICAL addr);
 void red_put_surface_cmd(RedSurfaceCmd *red);
 
-bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
-                        RedCursorCmd *red, QXLPHYSICAL addr);
-void red_put_cursor_cmd(RedCursorCmd *red);
+RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                                 int group_id, QXLPHYSICAL addr);
+RedCursorCmd *red_cursor_cmd_ref(RedCursorCmd *cmd);
+void red_cursor_cmd_unref(RedCursorCmd *cmd);
 
 #endif /* RED_PARSE_QXL_H_ */
diff --git a/server/red-worker.c b/server/red-worker.c
index 9168553d..266fb04a 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -96,13 +96,15 @@ static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *e
 {
     RedCursorCmd *cursor_cmd;
 
-    cursor_cmd = g_new0(RedCursorCmd, 1);
-    if (!red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data)) {
-        g_free(cursor_cmd);
+    cursor_cmd = red_cursor_cmd_new(worker->qxl, &worker->mem_slots,
+                                    ext->group_id, ext->cmd.data);
+    if (cursor_cmd == NULL) {
         return FALSE;
     }
-    cursor_cmd->qxl = worker->qxl;
+
     cursor_channel_process_cmd(worker->cursor_channel, cursor_cmd);
+    red_cursor_cmd_unref(cursor_cmd);
+
     return TRUE;
 }
 
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index 47139a48..2314d768 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -149,7 +149,7 @@ static void test_too_big_image(void)
 static void test_cursor_command(void)
 {
     RedMemSlotInfo mem_info;
-    RedCursorCmd red_cursor_cmd;
+    RedCursorCmd *red_cursor_cmd;
     QXLCursorCmd cursor_cmd;
     QXLCursor *cursor;
 
@@ -167,8 +167,9 @@ static void test_cursor_command(void)
 
     cursor_cmd.u.set.shape = to_physical(cursor);
 
-    g_assert_true(red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd)));
-    g_free(red_cursor_cmd.u.set.shape.data);
+    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd));
+    g_assert_nonnull(red_cursor_cmd);
+    red_cursor_cmd_unref(red_cursor_cmd);
     g_free(cursor);
     memslot_info_destroy(&mem_info);
 }
@@ -176,7 +177,7 @@ static void test_cursor_command(void)
 static void test_circular_empty_chunks(void)
 {
     RedMemSlotInfo mem_info;
-    RedCursorCmd red_cursor_cmd;
+    RedCursorCmd *red_cursor_cmd;
     QXLCursorCmd cursor_cmd;
     QXLCursor *cursor;
     QXLDataChunk *chunks[2];
@@ -200,13 +201,14 @@ static void test_circular_empty_chunks(void)
 
     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))) {
+    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd));
+    if (red_cursor_cmd != NULL) {
         /* function does not return errors so there should be no data */
-        g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.y, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.shape.data_size, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->type, ==, QXL_CURSOR_SET);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.x, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.y, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.shape.data_size, ==, 0);
+        red_cursor_cmd_unref(red_cursor_cmd);
     }
     g_test_assert_expected_messages();
 
@@ -218,7 +220,7 @@ static void test_circular_empty_chunks(void)
 static void test_circular_small_chunks(void)
 {
     RedMemSlotInfo mem_info;
-    RedCursorCmd red_cursor_cmd;
+    RedCursorCmd *red_cursor_cmd;
     QXLCursorCmd cursor_cmd;
     QXLCursor *cursor;
     QXLDataChunk *chunks[2];
@@ -242,13 +244,14 @@ static void test_circular_small_chunks(void)
 
     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))) {
+    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd));
+    if (red_cursor_cmd != NULL) {
         /* function does not return errors so there should be no data */
-        g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.y, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.shape.data_size, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->type, ==, QXL_CURSOR_SET);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.x, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.y, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.shape.data_size, ==, 0);
+        red_cursor_cmd_unref(red_cursor_cmd);
     }
     g_test_assert_expected_messages();
 
commit f928763db637d19f0b5e7c59e09f4ab85d52105e
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:05 2018 +0100

    qxl: Fix guest resources release in red_put_drawable()
    
    At the moment, we'll unconditionally release the guest QXL resources in
    red_put_drawable() even if red_get_drawable() failed and did not
    initialize drawable->release_info_ext properly.
    This commit only sets RedDrawable::qxl once the guest resource have been
    successfully retrieved, and only free the guest QXL resources when
    RedDrawable::qxl is set.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index a5b6dfe4..229c8669 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1008,7 +1008,7 @@ static void red_put_clip(SpiceClip *red)
     }
 }
 
-static bool red_get_native_drawable(RedMemSlotInfo *slots, int group_id,
+static bool red_get_native_drawable(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
                                     RedDrawable *red, QXLPHYSICAL addr, uint32_t flags)
 {
     QXLDrawable *qxl;
@@ -1018,6 +1018,7 @@ static bool red_get_native_drawable(RedMemSlotInfo *slots, int group_id,
     if (qxl == NULL) {
         return false;
     }
+    red->qxl = qxl_instance;
     red->release_info_ext.info     = &qxl->release_info;
     red->release_info_ext.group_id = group_id;
 
@@ -1088,7 +1089,7 @@ static bool red_get_native_drawable(RedMemSlotInfo *slots, int group_id,
     return true;
 }
 
-static bool red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
+static bool red_get_compat_drawable(QXLInstance *qxl_instance, RedMemSlotInfo *slots, int group_id,
                                     RedDrawable *red, QXLPHYSICAL addr, uint32_t flags)
 {
     QXLCompatDrawable *qxl;
@@ -1097,6 +1098,7 @@ static bool red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
     if (qxl == NULL) {
         return false;
     }
+    red->qxl = qxl_instance;
     red->release_info_ext.info     = &qxl->release_info;
     red->release_info_ext.group_id = group_id;
 
@@ -1170,15 +1172,15 @@ static bool red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
     return true;
 }
 
-static bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
+static bool red_get_drawable(QXLInstance *qxl, RedMemSlotInfo *slots, int group_id,
                              RedDrawable *red, QXLPHYSICAL addr, uint32_t flags)
 {
     bool ret;
 
     if (flags & QXL_COMMAND_FLAG_COMPAT) {
-        ret = red_get_compat_drawable(slots, group_id, red, addr, flags);
+        ret = red_get_compat_drawable(qxl, slots, group_id, red, addr, flags);
     } else {
-        ret = red_get_native_drawable(slots, group_id, red, addr, flags);
+        ret = red_get_native_drawable(qxl, slots, group_id, red, addr, flags);
     }
     return ret;
 }
@@ -1230,6 +1232,9 @@ static void red_put_drawable(RedDrawable *red)
         red_put_whiteness(&red->u.whiteness);
         break;
     }
+    if (red->qxl != NULL) {
+        red_qxl_release_resource(red->qxl, red->release_info_ext);
+    }
 }
 
 RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
@@ -1239,9 +1244,8 @@ RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
     RedDrawable *red = g_new0(RedDrawable, 1);
 
     red->refs = 1;
-    red->qxl = qxl;
 
-    if (!red_get_drawable(slots, group_id, red, addr, flags)) {
+    if (!red_get_drawable(qxl, slots, group_id, red, addr, flags)) {
        red_drawable_unref(red);
        return NULL;
     }
@@ -1260,7 +1264,6 @@ void red_drawable_unref(RedDrawable *red_drawable)
     if (--red_drawable->refs) {
         return;
     }
-    red_qxl_release_resource(red_drawable->qxl, red_drawable->release_info_ext);
     red_put_drawable(red_drawable);
     g_free(red_drawable);
 }
commit 179134a4e86717198f3e6f88194f833f4943306e
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:04 2018 +0100

    qxl: Make red_{get, put}_drawable static
    
    Rather than needing to call red_drawable_new() and then initialize it
    with red_get_drawable(), we can improve slightly red_drawable new so
    that red_drawable_{new,ref,unref} is all which is used by code out of
    red-parse-qxl.c.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 120b9e76..a5b6dfe4 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1170,8 +1170,8 @@ static bool red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
     return true;
 }
 
-bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
-                      RedDrawable *red, QXLPHYSICAL addr, uint32_t flags)
+static bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
+                             RedDrawable *red, QXLPHYSICAL addr, uint32_t flags)
 {
     bool ret;
 
@@ -1183,7 +1183,7 @@ bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
     return ret;
 }
 
-void red_put_drawable(RedDrawable *red)
+static void red_put_drawable(RedDrawable *red)
 {
     red_put_clip(&red->clip);
     if (red->self_bitmap_image) {
@@ -1232,13 +1232,20 @@ void red_put_drawable(RedDrawable *red)
     }
 }
 
-RedDrawable *red_drawable_new(QXLInstance *qxl)
+RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                              int group_id, QXLPHYSICAL addr,
+                              uint32_t flags)
 {
-    RedDrawable * red = g_new0(RedDrawable, 1);
+    RedDrawable *red = g_new0(RedDrawable, 1);
 
     red->refs = 1;
     red->qxl = qxl;
 
+    if (!red_get_drawable(slots, group_id, red, addr, flags)) {
+       red_drawable_unref(red);
+       return NULL;
+    }
+
     return red;
 }
 
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 3b815a86..cbb88019 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -110,14 +110,12 @@ typedef struct RedCursorCmd {
 
 void red_get_rect_ptr(SpiceRect *red, const QXLRect *qxl);
 
-RedDrawable *red_drawable_new(QXLInstance *qxl);
+RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                              int group_id, QXLPHYSICAL addr,
+                              uint32_t flags);
 RedDrawable *red_drawable_ref(RedDrawable *drawable);
 void red_drawable_unref(RedDrawable *red_drawable);
 
-bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
-                      RedDrawable *red, QXLPHYSICAL addr, uint32_t flags);
-void red_put_drawable(RedDrawable *red);
-
 bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
                         RedUpdateCmd *red, QXLPHYSICAL addr);
 void red_put_update_cmd(RedUpdateCmd *red);
diff --git a/server/red-worker.c b/server/red-worker.c
index d9ae792a..9168553d 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -198,15 +198,16 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
         worker->display_poll_tries = 0;
         switch (ext_cmd.cmd.type) {
         case QXL_CMD_DRAW: {
-            RedDrawable *red_drawable = red_drawable_new(worker->qxl); // returns with 1 ref
+            RedDrawable *red_drawable;
+            red_drawable = red_drawable_new(worker->qxl, &worker->mem_slots,
+                                            ext_cmd.group_id, ext_cmd.cmd.data,
+                                            ext_cmd.flags); // returns with 1 ref
 
-            if (red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
-                                 red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
+            if (red_drawable != NULL) {
                 display_channel_process_draw(worker->display_channel, red_drawable,
                                              worker->process_display_generation);
+                red_drawable_unref(red_drawable);
             }
-            // release the red_drawable
-            red_drawable_unref(red_drawable);
             break;
         }
         case QXL_CMD_UPDATE: {
commit 7f8228b54047693077d0723a0aea549ccdfb6d23
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Nov 29 13:50:03 2018 +0100

    qxl: Move red_drawable_unref/red_drawable_new
    
    RedDrawable really is a RedDrawCmd which is parsed by red-parse-qxl.h
    Moreover, red_drawable_ref() is already defined inline in
    red-parse-qxl.h, and red_drawable_unref() is declared there too even if
    its code is still in red-worker.c
    This commit moves them close to the other functions creating/unref'ing
    QXL commands parsed by red-parse-qxl.h.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 86abe3ca..120b9e76 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1232,6 +1232,32 @@ void red_put_drawable(RedDrawable *red)
     }
 }
 
+RedDrawable *red_drawable_new(QXLInstance *qxl)
+{
+    RedDrawable * red = g_new0(RedDrawable, 1);
+
+    red->refs = 1;
+    red->qxl = qxl;
+
+    return red;
+}
+
+RedDrawable *red_drawable_ref(RedDrawable *drawable)
+{
+    drawable->refs++;
+    return drawable;
+}
+
+void red_drawable_unref(RedDrawable *red_drawable)
+{
+    if (--red_drawable->refs) {
+        return;
+    }
+    red_qxl_release_resource(red_drawable->qxl, red_drawable->release_info_ext);
+    red_put_drawable(red_drawable);
+    g_free(red_drawable);
+}
+
 bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
                         RedUpdateCmd *red, QXLPHYSICAL addr)
 {
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index a33f36ad..3b815a86 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -59,14 +59,6 @@ typedef struct RedDrawable {
     } u;
 } RedDrawable;
 
-static inline RedDrawable *red_drawable_ref(RedDrawable *drawable)
-{
-    drawable->refs++;
-    return drawable;
-}
-
-void red_drawable_unref(RedDrawable *red_drawable);
-
 typedef struct RedUpdateCmd {
     QXLReleaseInfoExt release_info_ext;
     SpiceRect area;
@@ -118,6 +110,10 @@ typedef struct RedCursorCmd {
 
 void red_get_rect_ptr(SpiceRect *red, const QXLRect *qxl);
 
+RedDrawable *red_drawable_new(QXLInstance *qxl);
+RedDrawable *red_drawable_ref(RedDrawable *drawable);
+void red_drawable_unref(RedDrawable *red_drawable);
+
 bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
                       RedDrawable *red, QXLPHYSICAL addr, uint32_t flags);
 void red_put_drawable(RedDrawable *red);
diff --git a/server/red-worker.c b/server/red-worker.c
index ccab9d96..d9ae792a 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -92,16 +92,6 @@ struct RedWorker {
     GMainLoop *loop;
 };
 
-void red_drawable_unref(RedDrawable *red_drawable)
-{
-    if (--red_drawable->refs) {
-        return;
-    }
-    red_qxl_release_resource(red_drawable->qxl, red_drawable->release_info_ext);
-    red_put_drawable(red_drawable);
-    g_free(red_drawable);
-}
-
 static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *ext)
 {
     RedCursorCmd *cursor_cmd;
@@ -158,16 +148,6 @@ static int red_process_cursor(RedWorker *worker, int *ring_is_empty)
     return n;
 }
 
-static RedDrawable *red_drawable_new(QXLInstance *qxl)
-{
-    RedDrawable * red = g_new0(RedDrawable, 1);
-
-    red->refs = 1;
-    red->qxl = qxl;
-
-    return red;
-}
-
 static gboolean red_process_surface_cmd(RedWorker *worker, QXLCommandExt *ext, gboolean loadvm)
 {
     RedSurfaceCmd surface_cmd;


More information about the Spice-commits mailing list