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

[David Jaša]

Yonit Halperin píše v Čt 25. 04. 2013 v 10:15 -0400:
> Also forgot to add, that in general, regarding errors and asserts is 
> spice-server that are related to the connection to the client, we should 
> abort the server, but rather disconnect the client.

Spot on.

Problem with client connection should never be the reason of termination
of whole VM.

David

> For the specific assert the patch is referring to, I also believe the 
> client should be disconnected. When we hold a wrong list of surfaces 
> that were created in the client, it can lead to inconssitencies (e.g., 
> sending rendering commands for surfaces that doesn't exist on the 
> client, or if we believe that the client holds surface X which was never 
> created, and then we really create such surface, it will probably lead 
> to another assert...)
> On 04/25/2013 10:02 AM, Yonit Halperin wrote:
> > 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;
> >>   }
> >>
> >>
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

-- 

David Jaša, RHCE

SPICE QE based in Brno
GPG Key:     22C33E24 
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3790 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20130425/52e3cb22/attachment-0001.bin>