[Spice-devel] [PATCH 12/18] worker: move dcc_handle_migrate_data

Fabiano Fidêncio fabiano at fidencio.org
Fri Nov 20 08:19:17 PST 2015


On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>
> ---
>  server/dcc.c        | 135 ++++++++++++++++++++++++++++++++++++++++++
>  server/dcc.h        |   5 ++
>  server/red_worker.c | 165 ++--------------------------------------------------
>  3 files changed, 145 insertions(+), 160 deletions(-)
>
> diff --git a/server/dcc.c b/server/dcc.c
> index a20e27e..fbaf386 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -1255,3 +1255,138 @@ int dcc_handle_message(RedChannelClient *rcc, uint32_t size, uint16_t type, void
>          return red_channel_client_handle_message(rcc, size, type, msg);
>      }
>  }
> +
> +static int dcc_handle_migrate_glz_dictionary(DisplayChannelClient *dcc,
> +                                             SpiceMigrateDataDisplay *migrate)
> +{
> +    spice_return_val_if_fail(!dcc->glz_dict, FALSE);
> +
> +    ring_init(&dcc->glz_drawables);
> +    ring_init(&dcc->glz_drawables_inst_to_free);
> +    pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
> +    dcc->glz_dict = dcc_restore_glz_dictionary(dcc,
> +                                               migrate->glz_dict_id,
> +                                               &migrate->glz_dict_data);
> +    return dcc->glz_dict != NULL;
> +}
> +
> +static int restore_surface(DisplayChannelClient *dcc, uint32_t surface_id)
> +{
> +    /* we don't process commands till we receive the migration data, thus,
> +     * we should have not sent any surface to the client. */
> +    if (dcc->surface_client_created[surface_id]) {
> +        spice_warning("surface %u is already marked as client_created", surface_id);
> +        return FALSE;
> +    }
> +    dcc->surface_client_created[surface_id] = TRUE;
> +    return TRUE;
> +}
> +
> +static int restore_surfaces_lossless(DisplayChannelClient *dcc,
> +                                         MigrateDisplaySurfacesAtClientLossless *mig_surfaces)
> +{
> +    uint32_t i;
> +
> +    spice_debug(NULL);
> +    for (i = 0; i < mig_surfaces->num_surfaces; i++) {
> +        uint32_t surface_id = mig_surfaces->surfaces[i].id;
> +
> +        if (!restore_surface(dcc, surface_id))
> +            return FALSE;
> +    }
> +    return TRUE;
> +}
> +
> +static int restore_surfaces_lossy(DisplayChannelClient *dcc,
> +                                  MigrateDisplaySurfacesAtClientLossy *mig_surfaces)
> +{
> +    uint32_t i;
> +
> +    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 (!restore_surface(dcc, surface_id))
> +            return FALSE;
> +
> +        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);
> +    }
> +    return TRUE;
> +}
> +
> +int dcc_handle_migrate_data(DisplayChannelClient *dcc, uint32_t size, void *message)
> +{
> +    DisplayChannel *display = DCC_TO_DC(dcc);
> +    int surfaces_restored = FALSE;
> +    SpiceMigrateDataHeader *header = (SpiceMigrateDataHeader *)message;
> +    SpiceMigrateDataDisplay *migrate_data = (SpiceMigrateDataDisplay *)(header + 1);
> +    uint8_t *surfaces;
> +    int i;
> +
> +    spice_return_val_if_fail(
> +        size >= (sizeof(*migrate_data) + sizeof(SpiceMigrateDataHeader)), FALSE);
> +    spice_return_val_if_fail(
> +         migration_protocol_validate_header(header,
> +             SPICE_MIGRATE_DATA_DISPLAY_MAGIC, SPICE_MIGRATE_DATA_DISPLAY_VERSION), FALSE);
> +
> +    /* size is set to -1 in order to keep the cache frozen until the original
> +     * channel client that froze the cache on the src size receives the migrate
> +     * data and unfreezes the cache by setting its size > 0 and by triggering
> +     * pixmap_cache_reset */
> +    dcc->pixmap_cache = pixmap_cache_get(RED_CHANNEL_CLIENT(dcc)->client,
> +                                         migrate_data->pixmap_cache_id, -1);
> +    spice_return_val_if_fail(dcc->pixmap_cache, FALSE);
> +
> +    pthread_mutex_lock(&dcc->pixmap_cache->lock);
> +    for (i = 0; i < MAX_CACHE_CLIENTS; i++) {
> +        dcc->pixmap_cache->sync[i] = MAX(dcc->pixmap_cache->sync[i],
> +                                         migrate_data->pixmap_cache_clients[i]);
> +    }
> +    pthread_mutex_unlock(&dcc->pixmap_cache->lock);
> +
> +    if (migrate_data->pixmap_cache_freezer) {
> +        /* activating the cache. The cache will start to be active after
> +         * pixmap_cache_reset is called, when handling PIPE_ITEM_TYPE_PIXMAP_RESET */
> +        dcc->pixmap_cache->size = migrate_data->pixmap_cache_size;
> +        red_channel_client_pipe_add_type(RED_CHANNEL_CLIENT(dcc), PIPE_ITEM_TYPE_PIXMAP_RESET);
> +    }
> +
> +    if (dcc_handle_migrate_glz_dictionary(dcc, migrate_data)) {
> +        dcc->glz =
> +            glz_encoder_create(dcc->common.id, dcc->glz_dict->dict, &dcc->glz_data.usr);
> +    } else {
> +        spice_critical("restoring global lz dictionary failed");
> +    }
> +
> +    dcc->common.is_low_bandwidth = migrate_data->low_bandwidth_setting;
> +
> +    if (migrate_data->low_bandwidth_setting) {
> +        red_channel_client_ack_set_client_window(RED_CHANNEL_CLIENT(dcc), WIDE_CLIENT_ACK_WINDOW);
> +        if (dcc->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
> +            display->enable_jpeg = TRUE;
> +        }
> +        if (dcc->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
> +            display->enable_zlib_glz_wrap = TRUE;
> +        }
> +    }
> +
> +    surfaces = (uint8_t *)message + migrate_data->surfaces_at_client_ptr;
> +    surfaces_restored = display->enable_jpeg ?
> +        restore_surfaces_lossy(dcc, (MigrateDisplaySurfacesAtClientLossy *)surfaces) :
> +        restore_surfaces_lossless(dcc, (MigrateDisplaySurfacesAtClientLossless*)surfaces);
> +
> +    spice_return_val_if_fail(surfaces_restored, FALSE);
> +
> +    red_channel_client_pipe_add_type(RED_CHANNEL_CLIENT(dcc), PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
> +    /* enable sending messages */
> +    red_channel_client_ack_zero_messages_window(RED_CHANNEL_CLIENT(dcc));
> +    return TRUE;
> +}
> diff --git a/server/dcc.h b/server/dcc.h
> index 2757d16..9da85ab 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -38,6 +38,9 @@
>  /* Each drawable can refer to at most 3 images: src, brush and mask */
>  #define MAX_DRAWABLE_PIXMAP_CACHE_ITEMS 3
>
> +#define WIDE_CLIENT_ACK_WINDOW 40
> +#define NARROW_CLIENT_ACK_WINDOW 20
> +
>  typedef struct WaitForChannels {
>      SpiceMsgWaitForChannels header;
>      SpiceWaitForChannel buf[MAX_CACHE_CLIENTS];
> @@ -161,6 +164,8 @@ void                       dcc_start                                 (DisplayCha
>  int                        dcc_handle_message                        (RedChannelClient *rcc,
>                                                                        uint32_t size,
>                                                                        uint16_t type, void *msg);
> +int                        dcc_handle_migrate_data                   (DisplayChannelClient *dcc,
> +                                                                      uint32_t size, void *message);
>  void                       dcc_push_monitors_config                  (DisplayChannelClient *dcc);
>  void                       dcc_destroy_surface                       (DisplayChannelClient *dcc,
>                                                                        uint32_t surface_id);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index adc9cb3..10b6c00 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -85,9 +85,6 @@ struct SpiceWatch {
>
>  #define MAX_PIPE_SIZE 50
>
> -#define WIDE_CLIENT_ACK_WINDOW 40
> -#define NARROW_CLIENT_ACK_WINDOW 20
> -
>  typedef struct UpgradeItem {
>      PipeItem base;
>      int refs;
> @@ -4338,20 +4335,6 @@ static inline void flush_all_qxl_commands(RedWorker *worker)
>      flush_cursor_commands(worker);
>  }
>
> -static int dcc_handle_migrate_glz_dictionary(DisplayChannelClient *dcc,
> -                                             SpiceMigrateDataDisplay *migrate)
> -{
> -    spice_return_val_if_fail(!dcc->glz_dict, FALSE);
> -
> -    ring_init(&dcc->glz_drawables);
> -    ring_init(&dcc->glz_drawables_inst_to_free);
> -    pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
> -    dcc->glz_dict = dcc_restore_glz_dictionary(dcc,
> -                                               migrate->glz_dict_id,
> -                                               &migrate->glz_dict_data);
> -    return dcc->glz_dict != NULL;
> -}
> -
>  static int display_channel_handle_migrate_mark(RedChannelClient *rcc)
>  {
>      DisplayChannel *display_channel = SPICE_CONTAINEROF(rcc->channel, DisplayChannel, common.base);
> @@ -4361,8 +4344,7 @@ static int display_channel_handle_migrate_mark(RedChannelClient *rcc)
>      return TRUE;
>  }
>
> -static uint64_t display_channel_handle_migrate_data_get_serial(
> -                RedChannelClient *rcc, uint32_t size, void *message)
> +static uint64_t handle_migrate_data_get_serial(RedChannelClient *rcc, uint32_t size, void *message)

This rename is not related to this patch ...

>  {
>      SpiceMigrateDataDisplay *migrate_data;
>
> @@ -4371,146 +4353,9 @@ 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)
> -{
> -    /* we don't process commands till we receive the migration data, thus,
> -     * we should have not sent any surface to the client. */
> -    if (dcc->surface_client_created[surface_id]) {
> -        spice_warning("surface %u is already marked as client_created", surface_id);
> -        return FALSE;
> -    }
> -    dcc->surface_client_created[surface_id] = TRUE;
> -    return TRUE;
> -}
> -
> -static int display_channel_client_restore_surfaces_lossless(DisplayChannelClient *dcc,
> -                                                            MigrateDisplaySurfacesAtClientLossless *mig_surfaces)
> -{
> -    uint32_t i;
> -
> -    spice_debug(NULL);
> -    for (i = 0; i < mig_surfaces->num_surfaces; i++) {
> -        uint32_t surface_id = mig_surfaces->surfaces[i].id;
> -
> -        if (!display_channel_client_restore_surface(dcc, surface_id)) {
> -            return FALSE;
> -        }
> -    }
> -    return TRUE;
> -}
> -
> -static int display_channel_client_restore_surfaces_lossy(DisplayChannelClient *dcc,
> -                                                          MigrateDisplaySurfacesAtClientLossy *mig_surfaces)
> -{
> -    uint32_t i;
> -
> -    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)) {
> -            return FALSE;
> -        }
> -        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);
> -    }
> -    return TRUE;
> -}
> -static int display_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t size,
> -                                               void *message)
> +static int handle_migrate_data(RedChannelClient *rcc, uint32_t size, void *message)

Same here ...

>  {
> -    SpiceMigrateDataHeader *header;
> -    SpiceMigrateDataDisplay *migrate_data;
> -    DisplayChannel *display_channel = SPICE_CONTAINEROF(rcc->channel, DisplayChannel, common.base);
> -    DisplayChannelClient *dcc = RCC_TO_DCC(rcc);
> -    uint8_t *surfaces;
> -    int surfaces_restored = FALSE;
> -    int i;
> -
> -    spice_debug(NULL);
> -    if (size < sizeof(*migrate_data) + sizeof(SpiceMigrateDataHeader)) {
> -        spice_error("bad message size");
> -        return FALSE;
> -    }
> -    header = (SpiceMigrateDataHeader *)message;
> -    migrate_data = (SpiceMigrateDataDisplay *)(header + 1);
> -    if (!migration_protocol_validate_header(header,
> -                                            SPICE_MIGRATE_DATA_DISPLAY_MAGIC,
> -                                            SPICE_MIGRATE_DATA_DISPLAY_VERSION)) {
> -        spice_error("bad header");
> -        return FALSE;
> -    }
> -    /* size is set to -1 in order to keep the cache frozen until the original
> -     * channel client that froze the cache on the src size receives the migrate
> -     * data and unfreezes the cache by setting its size > 0 and by triggering
> -     * pixmap_cache_reset */
> -    dcc->pixmap_cache = pixmap_cache_get(RED_CHANNEL_CLIENT(dcc)->client,
> -                                         migrate_data->pixmap_cache_id, -1);
> -    if (!dcc->pixmap_cache) {
> -        return FALSE;
> -    }
> -    pthread_mutex_lock(&dcc->pixmap_cache->lock);
> -    for (i = 0; i < MAX_CACHE_CLIENTS; i++) {
> -        dcc->pixmap_cache->sync[i] = MAX(dcc->pixmap_cache->sync[i],
> -                                         migrate_data->pixmap_cache_clients[i]);
> -    }
> -    pthread_mutex_unlock(&dcc->pixmap_cache->lock);
> -
> -    if (migrate_data->pixmap_cache_freezer) {
> -        /* activating the cache. The cache will start to be active after
> -         * pixmap_cache_reset is called, when handling PIPE_ITEM_TYPE_PIXMAP_RESET */
> -        dcc->pixmap_cache->size = migrate_data->pixmap_cache_size;
> -        red_channel_client_pipe_add_type(rcc,
> -                                         PIPE_ITEM_TYPE_PIXMAP_RESET);
> -    }
> -
> -    if (dcc_handle_migrate_glz_dictionary(dcc, migrate_data)) {
> -        dcc->glz = glz_encoder_create(dcc->common.id,
> -                                      dcc->glz_dict->dict, &dcc->glz_data.usr);
> -        if (!dcc->glz) {
> -            spice_critical("create global lz failed");
> -        }
> -    } else {
> -        spice_critical("restoring global lz dictionary failed");
> -    }
> -
> -    dcc->common.is_low_bandwidth = migrate_data->low_bandwidth_setting;
> -
> -    if (migrate_data->low_bandwidth_setting) {
> -        red_channel_client_ack_set_client_window(rcc, WIDE_CLIENT_ACK_WINDOW);
> -        if (dcc->jpeg_state == SPICE_WAN_COMPRESSION_AUTO) {
> -            display_channel->enable_jpeg = TRUE;
> -        }
> -        if (dcc->zlib_glz_state == SPICE_WAN_COMPRESSION_AUTO) {
> -            display_channel->enable_zlib_glz_wrap = TRUE;
> -        }
> -    }
> -
> -    surfaces = (uint8_t *)message + migrate_data->surfaces_at_client_ptr;
> -    if (display_channel->enable_jpeg) {
> -        surfaces_restored = display_channel_client_restore_surfaces_lossy(dcc,
> -                                (MigrateDisplaySurfacesAtClientLossy *)surfaces);
> -    } else {
> -        surfaces_restored = display_channel_client_restore_surfaces_lossless(dcc,
> -                                (MigrateDisplaySurfacesAtClientLossless*)surfaces);
> -    }
> -
> -    if (!surfaces_restored) {
> -        return FALSE;
> -    }
> -    red_channel_client_pipe_add_type(rcc, PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
> -    /* enable sending messages */
> -    red_channel_client_ack_zero_messages_window(rcc);
> -    return TRUE;
> +    return dcc_handle_migrate_data(RCC_TO_DCC(rcc), size, message);
>  }
>
>  static int common_channel_config_socket(RedChannelClient *rcc)
> @@ -4847,8 +4692,8 @@ static void display_channel_create(RedWorker *worker, int migrate, int stream_vi
>          .hold_item = display_channel_hold_pipe_item,
>          .release_item = display_channel_release_item,
>          .handle_migrate_flush_mark = display_channel_handle_migrate_mark,
> -        .handle_migrate_data = display_channel_handle_migrate_data,
> -        .handle_migrate_data_get_serial = display_channel_handle_migrate_data_get_serial
> +        .handle_migrate_data = handle_migrate_data,
> +        .handle_migrate_data_get_serial = handle_migrate_data_get_serial

Same here ...

>      };
>
>      spice_return_if_fail(num_renderers > 0);
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

Apart from the comment, looks good. ACK!

-- 
Fabiano Fidêncio


More information about the Spice-devel mailing list