[Spice-devel] [PATCH 1/2] server: removing local cursor, this solves RHBZ #714801

Yonit Halperin yhalperi at redhat.com
Mon Jul 4 01:33:57 PDT 2011


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.
---
 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



More information about the Spice-devel mailing list