On Tue, 2016-01-26 at 14:51 +0100, Pavel Grunt wrote:
> On Tue, 2016-01-26 at 09:44 +0000, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > 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));
> > +}
red_disconnect_all_display_TODO_remove_me() is exactly the same implementation
as red_channel_disconnect(), except that it has a TODO comment in it. If we move
that TODO note here (assuming we really want to keep it), we could just call
red_channel_disconnect() here and remove the _remove_me() function...
> >
> > - 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)
> _______________________________________________
> 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