[Spice-commits] 3 commits - server/red_worker.c

Yonit Halperin yhalperi at kemper.freedesktop.org
Mon Nov 12 08:50:21 PST 2012


 server/red_worker.c |  112 +++++++++++++++++++++++++++-------------------------
 1 file changed, 60 insertions(+), 52 deletions(-)

New commits:
commit 45a09e4113d0ce2a74a9b791c4bb2271f3423b07
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Tue Nov 6 20:11:17 2012 +0200

    red_worker.c: fix calling set_client_capabilities when it is unsupported by qemu
    
    The erroneous call was in handle_dev_start.
    This patch also fixes not calling set_client_capabilities when the
    qxl major_version is > 3.

diff --git a/server/red_worker.c b/server/red_worker.c
index 8638a13..18ac949 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -10323,6 +10323,12 @@ static void guest_set_client_capabilities(RedWorker *worker)
         SPICE_DISPLAY_CAP_A8_SURFACE,
     };
 
+    if (worker->qxl->st->qif->base.major_version < 3 ||
+        (worker->qxl->st->qif->base.major_version == 3 &&
+        worker->qxl->st->qif->base.minor_version < 2) ||
+        !worker->qxl->st->qif->set_client_capabilities) {
+        return;
+    }
 #define SET_CAP(a,c)                                                    \
         ((a)[(c) / 8] |= (1 << ((c) % 8)))
 
@@ -10405,11 +10411,7 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
     spice_info("jpeg %s", display_channel->enable_jpeg ? "enabled" : "disabled");
     spice_info("zlib-over-glz %s", display_channel->enable_zlib_glz_wrap ? "enabled" : "disabled");
 
-    if (worker->qxl->st->qif->base.major_version == 3 &&
-        worker->qxl->st->qif->base.minor_version >= 2 &&
-        worker->qxl->st->qif->set_client_capabilities) {
-        guest_set_client_capabilities(worker);
-    }
+    guest_set_client_capabilities(worker);
     
     // todo: tune level according to bandwidth
     display_channel->zlib_level = ZLIB_DEFAULT_COMPRESSION_LEVEL;
@@ -11303,11 +11305,7 @@ void handle_dev_display_disconnect(void *opaque, void *payload)
     spice_info("disconnect display client");
     spice_assert(rcc);
 
-    if (worker->qxl->st->qif->base.major_version == 3 &&
-        worker->qxl->st->qif->base.minor_version >= 2 &&
-        worker->qxl->st->qif->set_client_capabilities) {
-        guest_set_client_capabilities(worker);
-    }
+    guest_set_client_capabilities(worker);
 
     red_channel_client_disconnect(rcc);
 }
commit 9a7a645ce201ec8bb161dc648ae7eccf9e9969f0
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Mon Nov 5 23:28:19 2012 +0200

    display seamless migration: no need to trace the generation of the primary surface
    
    We no longer process destroy_primary or destroy_surfaces while waiting
    for migration data.

diff --git a/server/red_worker.c b/server/red_worker.c
index 4636647..8638a13 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -921,7 +921,6 @@ typedef struct RedWorker {
     RedSurface surfaces[NUM_SURFACES];
     uint32_t n_surfaces;
     SpiceImageSurfaces image_surfaces;
-    uint32_t primary_surface_generation;
 
     MonitorsConfig *monitors_config;
 
@@ -9766,23 +9765,12 @@ static uint64_t display_channel_handle_migrate_data_get_serial(
     return migrate_data->message_serial;
 }
 
-static int display_channel_client_restore_surface(DisplayChannelClient *dcc, uint32_t surface_id)
+static void display_channel_client_restore_surface(DisplayChannelClient *dcc, uint32_t surface_id)
 {
-    if (surface_id == 0) {
-        if (dcc->common.worker->primary_surface_generation <= 1) {
-            dcc->surface_client_created[surface_id] = TRUE;
-            return TRUE;
-        } else {
-            /* red_create_surface_item already updated the client */
-            return FALSE;
-        }
-    } else {
-        /* we don't process commands till we receive the migration data, thus,
-         * we should have not created any off-screen surface */
-        spice_assert(!dcc->surface_client_created[surface_id]);
-        dcc->surface_client_created[surface_id] = TRUE;
-        return TRUE;
-    }
+    /* we don't process commands till we receive the migration data, thus,
+     * we should have not sent any surface to the client. */
+    spice_assert(!dcc->surface_client_created[surface_id]);
+    dcc->surface_client_created[surface_id] = TRUE;
 }
 
 static void display_channel_client_restore_surfaces_lossless(DisplayChannelClient *dcc,
@@ -9806,21 +9794,19 @@ static void display_channel_client_restore_surfaces_lossy(DisplayChannelClient *
     spice_debug(NULL);
     for (i = 0; i < mig_surfaces->num_surfaces; i++) {
         uint32_t surface_id = mig_surfaces->surfaces[i].id;
+        SpiceMigrateDataRect *mig_lossy_rect;
+        SpiceRect lossy_rect;
 
-        if (display_channel_client_restore_surface(dcc, surface_id)) {
-            SpiceMigrateDataRect *mig_lossy_rect;
-            SpiceRect lossy_rect;
-
-            spice_assert(dcc->surface_client_created[surface_id]);
+        display_channel_client_restore_surface(dcc, surface_id);
+        spice_assert(dcc->surface_client_created[surface_id]);
 
-            mig_lossy_rect = &mig_surfaces->surfaces[i].lossy_rect;
-            lossy_rect.left = mig_lossy_rect->left;
-            lossy_rect.top = mig_lossy_rect->top;
-            lossy_rect.right = mig_lossy_rect->right;
-            lossy_rect.bottom = mig_lossy_rect->bottom;
-            region_init(&dcc->surface_client_lossy_region[surface_id]);
-            region_add(&dcc->surface_client_lossy_region[surface_id], &lossy_rect);
-        }
+        mig_lossy_rect = &mig_surfaces->surfaces[i].lossy_rect;
+        lossy_rect.left = mig_lossy_rect->left;
+        lossy_rect.top = mig_lossy_rect->top;
+        lossy_rect.right = mig_lossy_rect->right;
+        lossy_rect.bottom = mig_lossy_rect->bottom;
+        region_init(&dcc->surface_client_lossy_region[surface_id]);
+        region_add(&dcc->surface_client_lossy_region[surface_id], &lossy_rect);
     }
 }
 static int display_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t size,
@@ -11041,11 +11027,6 @@ static void dev_create_primary_surface(RedWorker *worker, uint32_t surface_id,
         line_0 -= (int32_t)(surface.stride * (surface.height -1));
     }
 
-    if (!worker->display_channel->common.during_target_migrate) {
-        worker->primary_surface_generation++;
-    } else {
-         worker->primary_surface_generation = 1;
-    }
     red_create_surface(worker, 0, surface.width, surface.height, surface.stride, surface.format,
                        line_0, surface.flags & QXL_SURF_FLAG_KEEP_DATA, TRUE);
     set_monitors_config_to_primary(worker);
commit 8918664cc3f63f3266490b2188ad537c282f021c
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Mon Nov 5 21:18:56 2012 +0200

    display seamless migration: don't process both cmd ring and dispatcher queue till migration data is received
    
    fix: rhbz#866929
    
    At migration destination side, we need to restore the client's surfaces
    state, before sending surfaces related messages.
    Before this patch, we stopped the processing of only the cmd ring, till migration data
    arrived.
    However, some QXL_IOs require reading and rendering the cmd ring (e.g.,
    update_area). Moreover, when the device is reset, after destroying all
    surfaces, we assert (in qemu) if the cmd ring is not empty (see
    rhbz#866929).
    This fix makes the red_worker thread wait till the migration data arrives
    (or till a timeout), and not process any input from the device after the
    vm is started.

diff --git a/server/red_worker.c b/server/red_worker.c
index 9edd5d4..4636647 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -102,6 +102,7 @@
 #define CHANNEL_PUSH_SLEEP_DURATION 10000 //micro
 
 #define DISPLAY_CLIENT_TIMEOUT 15000000000ULL //nano
+#define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT 10000000000ULL //nano, 10 sec
 #define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro
 
 #define DISPLAY_FREE_LIST_DEFAULT_SIZE 128
@@ -4892,13 +4893,7 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
     int n = 0;
     uint64_t start = red_now();
 
-    /* we don't process the command ring if we are at the migration target and we
-     * are expecting migration data. The migration data includes the list
-     * of surfaces that are held by the client. We need to have it before
-     * processing commands in order not to render and send surfaces unnecessarily
-     */
-    if (!worker->running || (worker->display_channel &&
-        red_channel_waits_for_migrate_data(&worker->display_channel->common.base))) {
+    if (!worker->running) {
         *ring_is_empty = TRUE;
         return n;
     }
@@ -11162,6 +11157,37 @@ void handle_dev_stop(void *opaque, void *payload)
     red_wait_outgoing_items(&worker->cursor_channel->common.base);
 }
 
+static int display_channel_wait_for_migrate_data(DisplayChannel *display)
+{
+    uint64_t end_time = red_now() + DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT;
+    RedChannel *channel = &display->common.base;
+    RedChannelClient *rcc;
+
+    spice_debug(NULL);
+    spice_assert(channel->clients_num == 1);
+
+    rcc = SPICE_CONTAINEROF(ring_get_head(&channel->clients), RedChannelClient, channel_link);
+    spice_assert(red_channel_client_waits_for_migrate_data(rcc));
+
+    for (;;) {
+        red_channel_client_receive(rcc);
+        if (!red_channel_client_is_connected(rcc)) {
+            break;
+        }
+
+        if (!red_channel_client_waits_for_migrate_data(rcc)) {
+            return TRUE;
+        }
+        if (red_now() > end_time) {
+            spice_warning("timeout");
+            red_channel_client_disconnect(rcc);
+            break;
+        }
+        usleep(DISPLAY_CLIENT_RETRY_INTERVAL);
+    }
+    return FALSE;
+}
+
 void handle_dev_start(void *opaque, void *payload)
 {
     RedWorker *worker = opaque;
@@ -11172,6 +11198,9 @@ void handle_dev_start(void *opaque, void *payload)
     }
     if (worker->display_channel) {
         worker->display_channel->common.during_target_migrate = FALSE;
+        if (red_channel_waits_for_migrate_data(&worker->display_channel->common.base)) {
+            display_channel_wait_for_migrate_data(worker->display_channel);
+        }
     }
     worker->running = TRUE;
     guest_set_client_capabilities(worker);


More information about the Spice-commits mailing list