[Spice-devel] [PATCH 1/2] server: removing local cursor, this solves RHBZ #714801
Alon Levy
alevy at redhat.com
Tue Jul 5 07:26:23 PDT 2011
On Mon, Jul 04, 2011 at 11:33:57AM +0300, Yonit Halperin wrote:
> When the worker was stoped, the cursor was copied from guest ram to the host ram,
> and its corresponding qxl command was released.
> This is unecessary, since the qxl ram still exists (we keep references
> to the surfaces in the same manner).
> It also led to BSOD on guest upon migration: the device tracks cursor set commands and it stores
> a reference to the last one. Then, it replays it to the destination server when migrating to it.
> However, the command the qxl replayed has already been released from the pci by the original
> worker, upon STOP.
ACK both.
> ---
> server/red_worker.c | 128 ++++++---------------------------------------------
> 1 files changed, 14 insertions(+), 114 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index bee86b9..1726b53 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -239,7 +239,6 @@ enum {
> PIPE_ITEM_TYPE_INVAL_ONE,
> PIPE_ITEM_TYPE_CURSOR,
> PIPE_ITEM_TYPE_MIGRATE,
> - PIPE_ITEM_TYPE_LOCAL_CURSOR,
> PIPE_ITEM_TYPE_CURSOR_INIT,
> PIPE_ITEM_TYPE_IMAGE,
> PIPE_ITEM_TYPE_STREAM_CREATE,
> @@ -300,17 +299,10 @@ typedef struct SurfaceDestroyItem {
> PipeItem pipe_item;
> } SurfaceDestroyItem;
>
> -enum {
> - CURSOR_TYPE_INVALID,
> - CURSOR_TYPE_DEV,
> - CURSOR_TYPE_LOCAL,
> -};
> -
> typedef struct CursorItem {
> PipeItem pipe_data;
> uint32_t group_id;
> int refs;
> - int type;
> RedCursorCmd *red_cursor;
> } CursorItem;
>
> @@ -4065,11 +4057,6 @@ static void red_release_cursor(RedWorker *worker, CursorItem *cursor)
> QXLReleaseInfoExt release_info_ext;
> RedCursorCmd *cursor_cmd;
>
> - if (cursor->type == CURSOR_TYPE_LOCAL) {
> - free(cursor);
> - return;
> - }
> -
> cursor_cmd = cursor->red_cursor;
> release_info_ext.group_id = cursor->group_id;
> release_info_ext.info = cursor_cmd->release_info;
> @@ -4125,7 +4112,6 @@ static CursorItem *get_cursor_item(RedWorker *worker, RedCursorCmd *cmd, uint32_
> cursor_item->refs = 1;
> red_channel_pipe_item_init(&worker->cursor_channel->common.base,
> &cursor_item->pipe_data, PIPE_ITEM_TYPE_CURSOR);
> - cursor_item->type = CURSOR_TYPE_INVALID;
> cursor_item->group_id = group_id;
> cursor_item->red_cursor = cmd;
>
> @@ -4140,7 +4126,6 @@ static void qxl_process_cursor(RedWorker *worker, RedCursorCmd *cursor_cmd, uint
> switch (cursor_cmd->type) {
> case QXL_CURSOR_SET:
> worker->cursor_visible = cursor_cmd->u.set.visible;
> - item->type = CURSOR_TYPE_DEV;
> red_set_cursor(worker, item);
> break;
> case QXL_CURSOR_MOVE:
> @@ -5883,6 +5868,7 @@ static void fill_attr(DisplayChannel *display_channel, SpiceMarshaller *m, Spice
>
> static void fill_cursor(CursorChannel *cursor_channel, SpiceCursor *red_cursor, CursorItem *cursor, AddBufInfo *addbuf)
> {
> + RedCursorCmd *cursor_cmd;
> addbuf->data = NULL;
>
> if (!cursor) {
> @@ -5890,35 +5876,23 @@ static void fill_cursor(CursorChannel *cursor_channel, SpiceCursor *red_cursor,
> return;
> }
>
> - if (cursor->type == CURSOR_TYPE_DEV) {
> - RedCursorCmd *cursor_cmd;
> -
> - cursor_cmd = cursor->red_cursor;
> - *red_cursor = cursor_cmd->u.set.shape;
> + cursor_cmd = cursor->red_cursor;
> + *red_cursor = cursor_cmd->u.set.shape;
>
> - if (red_cursor->header.unique) {
> - if (red_cursor_cache_find(cursor_channel, red_cursor->header.unique)) {
> - red_cursor->flags |= SPICE_CURSOR_FLAGS_FROM_CACHE;
> - return;
> - }
> - if (red_cursor_cache_add(cursor_channel, red_cursor->header.unique, 1)) {
> - red_cursor->flags |= SPICE_CURSOR_FLAGS_CACHE_ME;
> - }
> + if (red_cursor->header.unique) {
> + if (red_cursor_cache_find(cursor_channel, red_cursor->header.unique)) {
> + red_cursor->flags |= SPICE_CURSOR_FLAGS_FROM_CACHE;
> + return;
> }
> -
> - if (red_cursor->data_size) {
> - addbuf->type = BUF_TYPE_RAW;
> - addbuf->data = red_cursor->data;
> - addbuf->size = red_cursor->data_size;
> + if (red_cursor_cache_add(cursor_channel, red_cursor->header.unique, 1)) {
> + red_cursor->flags |= SPICE_CURSOR_FLAGS_CACHE_ME;
> }
> - } else {
> - LocalCursor *local_cursor;
> - ASSERT(cursor->type == CURSOR_TYPE_LOCAL);
> - local_cursor = (LocalCursor *)cursor;
> - *red_cursor = local_cursor->red_cursor;
> + }
> +
> + if (red_cursor->data_size) {
> addbuf->type = BUF_TYPE_RAW;
> - addbuf->data = local_cursor->red_cursor.data;
> - addbuf->size = local_cursor->data_size;
> + addbuf->data = red_cursor->data;
> + addbuf->size = red_cursor->data_size;
> }
> }
>
> @@ -7926,27 +7900,6 @@ static void red_marshall_cursor_init(CursorChannel *channel, SpiceMarshaller *ba
> add_buf_from_info(base_marshaller, &info);
> }
>
> -static void red_marshall_local_cursor(CursorChannel *cursor_channel,
> - SpiceMarshaller *base_marshaller, LocalCursor *cursor)
> -{
> - RedChannel *channel;
> - SpiceMsgCursorSet cursor_set;
> - AddBufInfo info;
> - RedWorker *worker;
> -
> - ASSERT(cursor_channel);
> - worker = cursor_channel->common.worker;
> - channel = &cursor_channel->common.base;
> - red_channel_init_send_data(channel, SPICE_MSG_CURSOR_SET, &cursor->base.pipe_data);
> - cursor_set.position = cursor->position;
> - cursor_set.visible = worker->cursor_visible;
> -
> - fill_cursor(cursor_channel, &cursor_set.cursor, &cursor->base, &info);
> - spice_marshall_msg_cursor_set(base_marshaller, &cursor_set);
> - add_buf_from_info(base_marshaller, &info);
> - red_release_cursor(worker, (CursorItem *)cursor);
> -}
> -
> static void cursor_channel_marshall_migrate(CursorChannel *cursor_channel,
> SpiceMarshaller *base_marshaller)
> {
> @@ -8149,9 +8102,6 @@ static void cursor_channel_send_item(RedChannel *channel, PipeItem *pipe_item)
> case PIPE_ITEM_TYPE_CURSOR:
> red_marshall_cursor(cursor_channel, m, (CursorItem *)pipe_item);
> break;
> - case PIPE_ITEM_TYPE_LOCAL_CURSOR:
> - red_marshall_local_cursor(cursor_channel, m, (LocalCursor *)pipe_item);
> - break;
> case PIPE_ITEM_TYPE_INVAL_ONE:
> red_cursor_marshall_inval(cursor_channel, m, (CacheItem *)pipe_item);
> free(pipe_item);
> @@ -9321,55 +9271,6 @@ typedef struct __attribute__ ((__packed__)) CursorData {
> SpiceCursor _cursor;
> } CursorData;
>
> -static LocalCursor *_new_local_cursor(RedChannel *channel,
> - SpiceCursorHeader *header, int data_size, SpicePoint16 position)
> -{
> - LocalCursor *local;
> -
> - local = (LocalCursor *)spice_malloc0(sizeof(LocalCursor) + data_size);
> -
> - red_channel_pipe_item_init(channel, &local->base.pipe_data,
> - PIPE_ITEM_TYPE_LOCAL_CURSOR);
> - local->base.refs = 1;
> - local->base.type = CURSOR_TYPE_LOCAL;
> -
> - local->red_cursor.header = *header;
> - local->red_cursor.header.unique = 0;
> -
> - local->red_cursor.flags = 0;
> - local->red_cursor.data = (uint8_t*)(local+1);
> -
> - local->position = position;
> - local->data_size = data_size;
> - return local;
> -}
> -
> -static void red_cursor_flush(RedWorker *worker)
> -{
> - RedCursorCmd *cursor_cmd;
> - SpiceCursor *cursor;
> - LocalCursor *local;
> -
> - if (!worker->cursor || worker->cursor->type == CURSOR_TYPE_LOCAL) {
> - return;
> - }
> -
> - ASSERT(worker->cursor->type == CURSOR_TYPE_DEV);
> -
> - cursor_cmd = worker->cursor->red_cursor;
> - ASSERT(cursor_cmd->type == QXL_CURSOR_SET);
> - cursor = &cursor_cmd->u.set.shape;
> -
> - local = _new_local_cursor(&worker->cursor_channel->common.base,
> - &cursor->header, cursor->data_size,
> - worker->cursor_position);
> - ASSERT(local);
> - memcpy(local->red_cursor.data, cursor->data, local->data_size);
> -
> - red_set_cursor(worker, &local->base);
> - red_release_cursor(worker, &local->base);
> -}
> -
> static void red_wait_outgoing_item(RedChannel *channel)
> {
> uint64_t end_time;
> @@ -9756,7 +9657,6 @@ static void handle_dev_input(EventListener *listener, uint32_t events)
> red_current_flush(worker, x);
> }
> }
> - red_cursor_flush(worker);
> red_wait_outgoing_item((RedChannel *)worker->display_channel);
> red_wait_outgoing_item((RedChannel *)worker->cursor_channel);
> message = RED_WORKER_MESSAGE_READY;
> --
> 1.7.4.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list