[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