[Spice-devel] [PATCH spice-server] Release cursor as soon as possible
Frediano Ziglio
fziglio at redhat.com
Wed Sep 6 12:41:21 UTC 2017
Cursor resources (basically the shape of it) was retained till
it was used however it was copied so there were no reason to not release
this resource.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
server/cursor-channel.c | 14 ++++----------
server/cursor-channel.h | 4 ++--
server/red-parse-qxl.c | 10 +++++++---
server/red-parse-qxl.h | 4 ++--
server/red-worker.c | 5 +++--
server/tests/test-qxl-parsing.c | 26 +++++++++++++++++++++++---
6 files changed, 41 insertions(+), 22 deletions(-)
This patch was discussed time ago ending up with this
https://lists.freedesktop.org/archives/spice-devel/2017-March/036210.html
after 6 months no patch has been proposed.
diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index 8db3e86bc..a844100a0 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -30,7 +30,6 @@
typedef struct RedCursorPipeItem {
RedPipeItem base;
- QXLInstance *qxl;
RedCursorCmd *red_cursor;
} RedCursorPipeItem;
@@ -55,7 +54,7 @@ G_DEFINE_TYPE(CursorChannel, cursor_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
static void cursor_pipe_item_free(RedPipeItem *pipe_item);
-static RedCursorPipeItem *cursor_pipe_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
+static RedCursorPipeItem *cursor_pipe_item_new(RedCursorCmd *cmd)
{
RedCursorPipeItem *item = g_new0(RedCursorPipeItem, 1);
@@ -63,7 +62,6 @@ static RedCursorPipeItem *cursor_pipe_item_new(QXLInstance *qxl, RedCursorCmd *c
red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR,
cursor_pipe_item_free);
- item->qxl = qxl;
item->red_cursor = cmd;
return item;
@@ -75,7 +73,6 @@ static void cursor_pipe_item_free(RedPipeItem *base)
RedCursorPipeItem *pipe_item = SPICE_UPCAST(RedCursorPipeItem, base);
cursor_cmd = pipe_item->red_cursor;
- red_qxl_release_resource(pipe_item->qxl, cursor_cmd->release_info_ext);
red_put_cursor_cmd(cursor_cmd);
free(cursor_cmd);
@@ -235,7 +232,7 @@ static void cursor_channel_send_item(RedChannelClient *rcc, RedPipeItem *pipe_it
red_channel_client_begin_send_message(rcc);
}
-CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
+CursorChannel* cursor_channel_new(RedsState *server, int id,
const SpiceCoreInterfaceInternal *core)
{
spice_debug("create cursor channel");
@@ -243,9 +240,8 @@ CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
"spice-server", server,
"core-interface", core,
"channel-type", SPICE_CHANNEL_CURSOR,
- "id", qxl->id,
+ "id", id,
"migration-flags", 0,
- "qxl", qxl,
"handle-acks", TRUE,
NULL);
}
@@ -254,13 +250,11 @@ void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
{
RedCursorPipeItem *cursor_pipe_item;
bool cursor_show = false;
- QXLInstance *qxl;
spice_return_if_fail(cursor);
spice_return_if_fail(cursor_cmd);
- qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(cursor));
- cursor_pipe_item = cursor_pipe_item_new(qxl, cursor_cmd);
+ cursor_pipe_item = cursor_pipe_item_new(cursor_cmd);
switch (cursor_cmd->type) {
case QXL_CURSOR_SET:
diff --git a/server/cursor-channel.h b/server/cursor-channel.h
index f279aafcf..50cf71f16 100644
--- a/server/cursor-channel.h
+++ b/server/cursor-channel.h
@@ -56,8 +56,8 @@ GType cursor_channel_get_type(void) G_GNUC_CONST;
* provided as helper functions and should only be called from the
* CursorChannel thread.
*/
-CursorChannel* cursor_channel_new (RedsState *server, QXLInstance *qxl,
- const SpiceCoreInterfaceInternal *core);
+CursorChannel* cursor_channel_new(RedsState *server, int id,
+ const SpiceCoreInterfaceInternal *core);
void cursor_channel_reset (CursorChannel *cursor);
void cursor_channel_do_init (CursorChannel *cursor);
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 33f36923a..fda4ff26d 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -26,6 +26,7 @@
#include "red-common.h"
#include "memslot.h"
#include "red-parse-qxl.h"
+#include "red-qxl.h"
/* Max size in bytes for any data field used in a QXL command.
* This will for example be useful to prevent the guest from saturating the
@@ -1461,8 +1462,10 @@ static void red_put_cursor(SpiceCursor *red)
}
bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
- RedCursorCmd *red, QXLPHYSICAL addr)
+ RedCursorCmd *red, QXLPHYSICAL addr,
+ QXLInstance *qxl_instance)
{
+ QXLReleaseInfoExt release_info_ext;
QXLCursorCmd *qxl;
int error;
@@ -1470,8 +1473,8 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
if (error) {
return false;
}
- red->release_info_ext.info = &qxl->release_info;
- red->release_info_ext.group_id = group_id;
+ release_info_ext.info = &qxl->release_info;
+ release_info_ext.group_id = group_id;
red->type = qxl->type;
switch (red->type) {
@@ -1487,6 +1490,7 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
red->u.trail.frequency = qxl->u.trail.frequency;
break;
}
+ red_qxl_release_resource(qxl_instance, release_info_ext);
return true;
}
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 4a576ca07..c73b5a99b 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -99,7 +99,6 @@ typedef struct RedSurfaceCmd {
} RedSurfaceCmd;
typedef struct RedCursorCmd {
- QXLReleaseInfoExt release_info_ext;
uint8_t type;
union {
struct {
@@ -138,7 +137,8 @@ bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
void red_put_surface_cmd(RedSurfaceCmd *red);
bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
- RedCursorCmd *red, QXLPHYSICAL addr);
+ RedCursorCmd *red, QXLPHYSICAL addr,
+ QXLInstance *qxl);
void red_put_cursor_cmd(RedCursorCmd *red);
#endif /* RED_PARSE_QXL_H_ */
diff --git a/server/red-worker.c b/server/red-worker.c
index 675c232e7..fe6eda7a7 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -108,7 +108,8 @@ static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *e
RedCursorCmd *cursor_cmd;
cursor_cmd = spice_new0(RedCursorCmd, 1);
- if (!red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data)) {
+ if (!red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data,
+ worker->qxl)) {
free(cursor_cmd);
return FALSE;
}
@@ -1342,7 +1343,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
worker->event_timeout = INF_EVENT_WAIT;
- worker->cursor_channel = cursor_channel_new(reds, qxl,
+ worker->cursor_channel = cursor_channel_new(reds, qxl->id,
&worker->core);
channel = RED_CHANNEL(worker->cursor_channel);
red_channel_init_stat_node(channel, &worker->stat, "cursor_channel");
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index 9c0c3b1c6..aa567a2ac 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -64,6 +64,23 @@ static void init_meminfo(RedMemSlotInfo *mem_info)
memslot_info_add_slot(mem_info, 0, 0, 0 /* delta */, 0 /* start */, ~0ul /* end */, 0 /* generation */);
}
+static void release_resource(SPICE_GNUC_UNUSED QXLInstance *qin,
+ SPICE_GNUC_UNUSED struct QXLReleaseInfoExt release_info)
+{
+}
+
+static QXLInterface display_sif = {
+ .base = {
+ .type = SPICE_INTERFACE_QXL,
+ .description = "test",
+ .major_version = SPICE_INTERFACE_QXL_MAJOR,
+ .minor_version = SPICE_INTERFACE_QXL_MINOR
+ },
+ .release_resource = release_resource,
+};
+
+static QXLInstance display;
+
static void init_qxl_surface(QXLSurfaceCmd *qxl)
{
void *surface_mem;
@@ -167,7 +184,7 @@ 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_assert_true(red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd), &display));
free(red_cursor_cmd.u.set.shape.data);
free(cursor);
memslot_info_destroy(&mem_info);
@@ -201,7 +218,7 @@ 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))) {
+ if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd), &display)) {
/* function does not return errors so there should be no data */
g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
@@ -243,7 +260,7 @@ 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))) {
+ if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd), &display)) {
/* function does not return errors so there should be no data */
g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
@@ -260,6 +277,9 @@ static void test_circular_small_chunks(void)
int main(int argc, char *argv[])
{
+ display.base.sif = &display_sif.base;
+ display.id = 0;
+
g_test_init(&argc, &argv, NULL);
/* try to create a surface with no issues, should succeed */
--
2.13.5
More information about the Spice-devel
mailing list