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.
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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel