[Spice-devel] [PATCH] server: don't reset the display channel when disconnecting all its clients , FDBZ #43977

Alon Levy alevy at redhat.com
Thu Dec 22 02:43:09 PST 2011


On Thu, Dec 22, 2011 at 12:36:04PM +0200, Yonit Halperin wrote:
> The display channel was unnecessarily set to NULL when we disconnect all the clients
> (on flush display commands timeout).
> As a result, we recreated the display channel when a new client was connected.
> The display channel was created with default red_channel.client_cbs, while its
> correct client_cbs are the ones that are set by the red_dispatcher when it creates
> the first display_channel.
> This fix enforce a single creation of the display channel (per qxl), via the red_dispatcher.

If you can split the rename patch seperately it would be nice.

ACK. (with or without the split)

> ---
>  server/red_worker.c |   18 ++++++------------
>  1 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index cb48f09..0d873ed 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -8660,18 +8660,13 @@ static void display_channel_client_on_disconnect(RedChannelClient *rcc)
>  
>  void red_disconnect_all_display_TODO_remove_me(RedChannel *channel)
>  {
> -    DisplayChannel *display_channel;
> -    RedWorker *worker;
>      // TODO: we need to record the client that actually causes the timeout. So
>      // we need to check the locations of the various pipe heads when counting,
>      // and disconnect only those/that.
>      if (!channel) {
>          return;
>      }
> -    display_channel = SPICE_CONTAINEROF(channel, DisplayChannel, common.base);
> -    worker = display_channel->common.worker;
>      red_channel_apply_clients(channel, display_channel_client_disconnect);
> -    worker->display_channel = NULL;
>  }
>  
>  static void red_migrate_display(RedWorker *worker, RedChannelClient *rcc)
> @@ -9707,7 +9702,7 @@ static void display_channel_release_item(RedChannelClient *rcc, PipeItem *item,
>      }
>  }
>  
> -static void ensure_display_channel_created(RedWorker *worker, int migrate)
> +static void display_channel_create(RedWorker *worker, int migrate)
>  {
>      DisplayChannel *display_channel;
>  
> @@ -9761,8 +9756,8 @@ static void handle_new_display_channel(RedWorker *worker, RedClient *client, Red
>      size_t stream_buf_size;
>      int is_low_bandwidth = main_channel_client_is_low_bandwidth(red_client_get_main(client));
>  
> -    ensure_display_channel_created(worker, migrate);
>      if (!worker->display_channel) {
> +        red_printf("Warning: Display channel was not created");
>          return;
>      }
>      display_channel = worker->display_channel;
> @@ -9925,7 +9920,7 @@ static void cursor_channel_release_item(RedChannelClient *rcc, PipeItem *item, i
>      }
>  }
>  
> -static void ensure_cursor_channel_created(RedWorker *worker, int migrate)
> +static void cursor_channel_create(RedWorker *worker, int migrate)
>  {
>      if (worker->cursor_channel != NULL) {
>          return;
> @@ -9951,9 +9946,8 @@ static void red_connect_cursor(RedWorker *worker, RedClient *client, RedsStream
>      CursorChannel *channel;
>      CursorChannelClient *ccc;
>  
> -    ensure_cursor_channel_created(worker, migrate);
>      if (worker->cursor_channel == NULL) {
> -        red_printf("failed to create cursor channel");
> +        red_printf("Warning: cursor channel was not created");
>          return;
>      }
>      channel = worker->cursor_channel;
> @@ -10514,7 +10508,7 @@ void handle_dev_display_channel_create(void *opaque, void *payload)
>  
>      RedChannel *red_channel;
>      // TODO: handle seemless migration. Temp, setting migrate to FALSE
> -    ensure_display_channel_created(worker, FALSE);
> +    display_channel_create(worker, FALSE);
>      red_channel = &worker->display_channel->common.base;
>      send_data(worker->channel, &red_channel, sizeof(RedChannel *));
>  }
> @@ -10559,7 +10553,7 @@ void handle_dev_cursor_channel_create(void *opaque, void *payload)
>      RedChannel *red_channel;
>  
>      // TODO: handle seemless migration. Temp, setting migrate to FALSE
> -    ensure_cursor_channel_created(worker, FALSE);
> +    cursor_channel_create(worker, FALSE);
>      red_channel = &worker->cursor_channel->common.base;
>      send_data(worker->channel, &red_channel, sizeof(RedChannel *));
>  }
> -- 
> 1.7.6.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel


More information about the Spice-devel mailing list