[Spice-devel] [PATCH v2 7/9] worker: unify flush_cursor_commands and flush_display_commands

[Pavel Grunt]

On Tue, 2016-01-26 at 09:44 +0000, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/red-worker.c | 68 ++++++++++++++++++-------------------------
> ----------
>  1 file changed, 23 insertions(+), 45 deletions(-)
> 
> diff --git a/server/red-worker.c b/server/red-worker.c
> index dd20bd5..5eb54f2 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -368,20 +368,22 @@ static void red_migrate_display(DisplayChannel
> *display, RedChannelClient *rcc)
>      }
>  }
>  
> -static void flush_display_commands(RedWorker *worker)
> -{
> -    RedChannel *red_channel = RED_CHANNEL(worker->display_channel);
> +typedef int (*red_process_t)(RedWorker *worker, int *ring_is_empty);
> +typedef void (*red_disconnect_t)(RedWorker *worker);
>  
> +static void flush_commands(RedWorker *worker, RedChannel
> *red_channel,
> +                           red_process_t process, red_disconnect_t
> disconnect)
> +{
>      for (;;) {
>          uint64_t end_time;
>          int ring_is_empty;
>  
> -        red_process_display(worker, &ring_is_empty);
> +        process(worker, &ring_is_empty);
>          if (ring_is_empty) {
>              break;
>          }
>  
> -        while (red_process_display(worker, &ring_is_empty)) {
> +        while (process(worker, &ring_is_empty)) {
>              red_channel_push(red_channel);
>          }
>  
> @@ -389,7 +391,6 @@ static void flush_display_commands(RedWorker
> *worker)
>              break;
>          }
>          end_time = spice_get_monotonic_time_ns() +
> COMMON_CLIENT_TIMEOUT;
> -        int sleep_count = 0;
>          for (;;) {
>              red_channel_push(red_channel);
>              if (red_channel_max_pipe_size(red_channel) <=
> MAX_PIPE_SIZE) {
> @@ -400,54 +401,30 @@ static void flush_display_commands(RedWorker
> *worker)
>              // TODO: MC: the whole timeout will break since it takes
> lowest timeout, should
>              // do it client by client.
>              if (spice_get_monotonic_time_ns() >= end_time) {
> -                spice_warning("update timeout");
> -                red_disconnect_all_display_TODO_remove_me(red_channe
> l);
> +                disconnect(worker);
>              } else {
> -                sleep_count++;
>                  usleep(DISPLAY_CLIENT_RETRY_INTERVAL);
>              }
>          }
>      }
>  }
>  
> -static void flush_cursor_commands(RedWorker *worker)
> +static void red_disconnect_display(RedWorker *worker)
>  {
> -    RedChannel *red_channel = RED_CHANNEL(worker->cursor_channel);
> -
> -    for (;;) {
> -        uint64_t end_time;
> -        int ring_is_empty = FALSE;
> -
> -        red_process_cursor(worker, &ring_is_empty);
> -        if (ring_is_empty) {
> -            break;
> -        }
> +    spice_warning("update timeout");
> +    red_disconnect_all_display_TODO_remove_me(RED_CHANNEL(worker-
> >display_channel));
> +}
>  
> -        while (red_process_cursor(worker, &ring_is_empty)) {
> -            red_channel_push(red_channel);
> -        }
> +static void flush_display_commands(RedWorker *worker)
> +{
> +    flush_commands(worker, RED_CHANNEL(worker->display_channel),
> +                   red_process_display, red_disconnect_display);
> +}
>  
> -        if (ring_is_empty) {
> -            break;
> -        }
> -        end_time = spice_get_monotonic_time_ns() +
> COMMON_CLIENT_TIMEOUT;
> -        int sleep_count = 0;
> -        for (;;) {
> -            red_channel_push(red_channel);
> -            if (red_channel_max_pipe_size(red_channel) <=
> MAX_PIPE_SIZE) {
> -                break;
> -            }
> -            red_channel_receive(red_channel);
> -            red_channel_send(red_channel);
> -            if (spice_get_monotonic_time_ns() >= end_time) {
> -                spice_warning("flush cursor timeout");
> -                cursor_channel_disconnect(worker->cursor_channel);
> -            } else {
> -                sleep_count++;
> -                usleep(DISPLAY_CLIENT_RETRY_INTERVAL);
> -            }
> -        }
> -    }
> +static void red_disconnect_cursor(RedWorker *worker)
> +{
> +    spice_warning("flush cursor timeout");
> +    cursor_channel_disconnect(worker->cursor_channel);
>  }
>  
>  // TODO: on timeout, don't disconnect all channels immediatly - try
> to disconnect the slowest ones
> @@ -456,7 +433,8 @@ static void flush_cursor_commands(RedWorker
> *worker)
>  static void flush_all_qxl_commands(RedWorker *worker)
>  {
>      flush_display_commands(worker);
> -    flush_cursor_commands(worker);
> +    flush_commands(worker, RED_CHANNEL(worker->cursor_channel),
> +                   red_process_cursor, red_disconnect_cursor);

Looks good, but for symmetry I would keep flush_cursor_commands

Pavel
>  }
>  
>  static int common_channel_config_socket(RedChannelClient *rcc)