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@xxxxxxxxxx> --- 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel