[Spice-devel] [PATCH spice] migration: set low_bandwidth if enable_jpeg

[Yonit Halperin]

Hi,

Good Catch!
Actually, the bandwidth is not getting evaluated after migration .
I think it is better, if we replace 
display_channel_client_is_low_bandwidth with a boolean in the display 
channel client that will be set once, when the channel is created (by 
calling main_channel_client_is_low_bandwidth), or when the migration 
data is retrieved, by copying the value in 
migrate_data.low_bandwidth_setting.  Other calls to 
main_channel_client_is_low_bandwidth should be replaced with checking 
this boolean(there are places that call 
display_channel_client_is_low_bandwidth, and others that call 
main_channel_client_is_low_bandwidth  directly...argh....).

Cheers,
Yonit.

On 04/25/2013 07:37 AM, Marc-André Lureau wrote:
> The migration dest will use only the value of low_bandwidth to restore
> the surfaces with lossy settings. But the server estimates
> low_bandwidth on each connection, which might not be the same after
> each migration.
>
> Older origin servers will still be vulnerable to assert() on destination.
> To mitigate the issue, let's replace the assert with a warning.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=956345
> ---
>   server/red_worker.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 8ba8070..5c17b97 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -8770,7 +8770,10 @@ static void display_channel_marshall_migrate_data(RedChannelClient *rcc,
>                    MIGRATE_DATA_DISPLAY_MAX_CACHE_CLIENTS == MAX_CACHE_CLIENTS);
>
>       display_data.message_serial = red_channel_client_get_message_serial(rcc);
> -    display_data.low_bandwidth_setting = display_channel_client_is_low_bandwidth(dcc);
> +
> +    display_data.low_bandwidth_setting =
> +        display_channel_client_is_low_bandwidth(dcc) |
> +        display_channel->enable_jpeg;
>
>       display_data.pixmap_cache_freezer = pixmap_cache_freeze(dcc->pixmap_cache);
>       display_data.pixmap_cache_id = dcc->pixmap_cache->id;
> @@ -10110,7 +10113,7 @@ static void display_channel_client_restore_surface(DisplayChannelClient *dcc, ui
>   {
>       /* 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]);
> +    spice_warn_if_fail(!dcc->surface_client_created[surface_id]);
>       dcc->surface_client_created[surface_id] = TRUE;
>   }
>
>