Re: [PATCH spice-server] EXPERIMENTAL: Remove DISPATCHER_ASYNC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



So I didn't know you were going to do this and after lunch I basically
implemented the same thing and sent it without noticing this patch :/

Anyway, since then I made a couple of changes and rebased it before the
documentation patch. I'll send an updated patch series.


On Wed, 2017-09-06 at 18:40 +0100, Frediano Ziglio wrote:
> Very quick patch to check Jonathon suggestion, quick tests seems
> to indicate that it works
> ---
>  server/dispatcher.c | 11 -----------
>  server/dispatcher.h | 16 +---------------
>  server/red-worker.c | 52 +++++++++++++++++++++++++----------------
> -----------
>  3 files changed, 26 insertions(+), 53 deletions(-)
> 
> diff --git a/server/dispatcher.c b/server/dispatcher.c
> index 4e03ea046..51e09b942 100644
> --- a/server/dispatcher.c
> +++ b/server/dispatcher.c
> @@ -60,7 +60,6 @@ struct DispatcherPrivate {
>      void *payload; /* allocated as max of message sizes */
>      size_t payload_size; /* used to track realloc calls */
>      void *opaque;
> -    dispatcher_handle_async_done handle_async_done;
>      dispatcher_handle_any_message any_handler;
>  };
>  
> @@ -305,8 +304,6 @@ static int
> dispatcher_handle_single_read(Dispatcher *dispatcher)
>              spice_printerr("error writing ack for message %d",
> type);
>              /* TODO: close socketpair? */
>          }
> -    } else if (msg->ack == DISPATCHER_ASYNC && dispatcher->priv-
> >handle_async_done) {
> -        dispatcher->priv->handle_async_done(dispatcher->priv-
> >opaque, type, payload);
>      }
>      return 1;
>  }
> @@ -355,14 +352,6 @@ unlock:
>      pthread_mutex_unlock(&dispatcher->priv->lock);
>  }
>  
> -void dispatcher_register_async_done_callback(
> -                                        Dispatcher *dispatcher,
> -                                        dispatcher_handle_async_done
> handler)
> -{
> -    assert(dispatcher->priv->handle_async_done == NULL);
> -    dispatcher->priv->handle_async_done = handler;
> -}
> -
>  void dispatcher_register_handler(Dispatcher *dispatcher, uint32_t
> message_type,
>                                   dispatcher_handle_message handler,
>                                   size_t size, int ack)
> diff --git a/server/dispatcher.h b/server/dispatcher.h
> index 97b01de9c..bb3f3943f 100644
> --- a/server/dispatcher.h
> +++ b/server/dispatcher.h
> @@ -75,7 +75,6 @@ void dispatcher_send_message(Dispatcher
> *dispatcher, uint32_t message_type,
>  enum {
>      DISPATCHER_NONE = 0,
>      DISPATCHER_ACK,
> -    DISPATCHER_ASYNC
>  };
>  
>  /*
> @@ -84,28 +83,15 @@ enum {
>   * @messsage_type:  message type
>   * @handler:        message handler
>   * @size:           message size. Each type has a fixed associated
> size.
> - * @ack:            One of DISPATCHER_NONE, DISPATCHER_ACK,
> DISPATCHER_ASYNC.
> + * @ack:            One of DISPATCHER_NONE, DISPATCHER_ACK.
>   *                  DISPATCHER_NONE - only send the message
>   *                  DISPATCHER_ACK - send an ack after the message
> - *                  DISPATCHER_ASYNC - call send an ack. This is per
> message type - you can't send the
> - *                  same message type with and without. Register two
> different
> - *                  messages if that is what you want.
>   */
>  void dispatcher_register_handler(Dispatcher *dispatcher, uint32_t
> message_type,
>                                   dispatcher_handle_message handler,
> size_t size,
>                                   int ack);
>  
>  /*
> - * dispatcher_register_async_done_callback
> - * @dispatcher:     dispatcher
> - * @handler:        callback on the receiver side called *after* the
> - *                  message callback in case ack ==
> DISPATCHER_ASYNC.
> - */
> -void dispatcher_register_async_done_callback(
> -                                    Dispatcher *dispatcher,
> -                                    dispatcher_handle_async_done
> handler);
> -
> -/*
>   * Hack to allow red_record to see the message being sent so it can
> record
>   * it to file.
>   */
> diff --git a/server/red-worker.c b/server/red-worker.c
> index b18e05461..708a431d1 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -432,6 +432,7 @@ static void handle_dev_update_async(void *opaque,
> void *payload)
>      red_qxl_update_area_complete(worker->qxl, msg->surface_id,
>                                   qxl_dirty_rects, num_dirty_rects);
>      free(qxl_dirty_rects);
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  static void handle_dev_update(void *opaque, void *payload)
> @@ -582,14 +583,17 @@ static void
> handle_dev_destroy_primary_surface_async(void *opaque, void *payload
>      uint32_t surface_id = msg->surface_id;
>  
>      destroy_primary_surface(worker, surface_id);
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  static void handle_dev_flush_surfaces_async(void *opaque, void
> *payload)
>  {
>      RedWorker *worker = opaque;
> +    RedWorkerMessageFlushSurfacesAsync *msg =
> (RedWorkerMessageFlushSurfacesAsync*) payload;
>  
>      flush_all_qxl_commands(worker);
>      display_channel_flush_all_surfaces(worker->display_channel);
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  static void handle_dev_stop(void *opaque, void *payload)
> @@ -689,15 +693,19 @@ static void
> handle_dev_destroy_surface_wait_async(void *opaque, void *payload)
>      RedWorker *worker = opaque;
>  
>      display_channel_destroy_surface_wait(worker->display_channel,
> msg->surface_id);
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  static void handle_dev_destroy_surfaces_async(void *opaque, void
> *payload)
>  {
>      RedWorker *worker = opaque;
> +    RedWorkerMessageDestroySurfacesAsync *msg =
> (RedWorkerMessageDestroySurfacesAsync *) payload;
>  
>      flush_all_qxl_commands(worker);
>      display_channel_destroy_surfaces(worker->display_channel);
>      cursor_channel_reset(worker->cursor_channel);
> +
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  static void handle_dev_create_primary_surface_async(void *opaque,
> void *payload)
> @@ -706,6 +714,7 @@ static void
> handle_dev_create_primary_surface_async(void *opaque, void *payload)
>      RedWorker *worker = opaque;
>  
>      dev_create_primary_surface(worker, msg->surface_id, msg-
> >surface);
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  static void handle_dev_display_connect(void *opaque, void *payload)
> @@ -774,21 +783,21 @@ static void
> handle_dev_monitors_config_async(void *opaque, void *payload)
>  
>      if (error) {
>          /* TODO: raise guest bug (requires added QXL interface) */
> -        return;
> +        goto reply;
>      }
>      worker->driver_cap_monitors_config = 1;
>      count = dev_monitors_config->count;
>      max_allowed = dev_monitors_config->max_allowed;
>      if (count == 0) {
>          spice_warning("ignoring an empty monitors config message
> from driver");
> -        return;
> +        goto reply;
>      }
>      if (count > max_allowed) {
>          spice_warning("ignoring malformed monitors_config from
> driver, "
>                        "count > max_allowed %d > %d",
>                        count,
>                        max_allowed);
> -        return;
> +        goto reply;
>      }
>      /* get pointer again to check virtual size */
>      dev_monitors_config =
> @@ -797,11 +806,14 @@ static void
> handle_dev_monitors_config_async(void *opaque, void *payload)
>                                               msg->group_id, &error);
>      if (error) {
>          /* TODO: raise guest bug (requires added QXL interface) */
> -        return;
> +        goto reply;
>      }
>      display_channel_update_monitors_config(worker->display_channel,
> dev_monitors_config,
>                                             MIN(count, msg-
> >max_monitors),
>                                             MIN(max_allowed, msg-
> >max_monitors));
> +
> +reply:
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  /* TODO: special, perhaps use another dispatcher? */
> @@ -928,6 +940,7 @@ static void handle_dev_add_memslot_async(void
> *opaque, void *payload)
>      RedWorker *worker = opaque;
>  
>      dev_add_memslot(worker, msg->mem_slot);
> +    red_qxl_async_complete(worker->qxl, msg->base.cmd);
>  }
>  
>  static void handle_dev_reset_memslots(void *opaque, void *payload)
> @@ -1000,17 +1013,6 @@ static void handle_dev_loadvm_commands(void
> *opaque, void *payload)
>      }
>  }
>  
> -static void worker_handle_dispatcher_async_done(void *opaque,
> -                                                uint32_t
> message_type,
> -                                                void *payload)
> -{
> -    RedWorker *worker = opaque;
> -    RedWorkerMessageAsync *msg_async = payload;
> -
> -    spice_debug("trace");
> -    red_qxl_async_complete(worker->qxl, msg_async->cmd);
> -}
> -
>  static void worker_dispatcher_record(void *opaque, uint32_t
> message_type, void *payload)
>  {
>      RedWorker *worker = opaque;
> @@ -1020,10 +1022,6 @@ static void worker_dispatcher_record(void
> *opaque, uint32_t message_type, void *
>  
>  static void register_callbacks(Dispatcher *dispatcher)
>  {
> -    dispatcher_register_async_done_callback(
> -                                    dispatcher,
> -                                    worker_handle_dispatcher_async_d
> one);
> -
>      /* TODO: register cursor & display specific msg in respective
> channel files */
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_DISPLAY_CONNECT,
> @@ -1064,7 +1062,7 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  RED_WORKER_MESSAGE_UPDATE_ASYNC,
>                                  handle_dev_update_async,
>                                  sizeof(RedWorkerMessageUpdateAsync),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_ADD_MEMSLOT,
>                                  handle_dev_add_memslot,
> @@ -1074,7 +1072,7 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC
> ,
>                                  handle_dev_add_memslot_async,
>                                  sizeof(RedWorkerMessageAddMemslotAsy
> nc),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_DEL_MEMSLOT,
>                                  handle_dev_del_memslot,
> @@ -1089,7 +1087,7 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  RED_WORKER_MESSAGE_DESTROY_SURFACES_
> ASYNC,
>                                  handle_dev_destroy_surfaces_async,
>                                  sizeof(RedWorkerMessageDestroySurfac
> esAsync),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_DESTROY_PRIMARY_S
> URFACE,
>                                  handle_dev_destroy_primary_surface,
> @@ -1099,12 +1097,12 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  RED_WORKER_MESSAGE_DESTROY_PRIMARY_S
> URFACE_ASYNC,
>                                  handle_dev_destroy_primary_surface_a
> sync,
>                                  sizeof(RedWorkerMessageDestroyPrimar
> ySurfaceAsync),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_CREATE_PRIMARY_SU
> RFACE_ASYNC,
>                                  handle_dev_create_primary_surface_as
> ync,
>                                  sizeof(RedWorkerMessageCreatePrimary
> SurfaceAsync),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_CREATE_PRIMARY_SU
> RFACE,
>                                  handle_dev_create_primary_surface,
> @@ -1139,7 +1137,7 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  RED_WORKER_MESSAGE_FLUSH_SURFACES_AS
> YNC,
>                                  handle_dev_flush_surfaces_async,
>                                  sizeof(RedWorkerMessageFlushSurfaces
> Async),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_STOP,
>                                  handle_dev_stop,
> @@ -1179,7 +1177,7 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  RED_WORKER_MESSAGE_DESTROY_SURFACE_W
> AIT_ASYNC,
>                                  handle_dev_destroy_surface_wait_asyn
> c,
>                                  sizeof(RedWorkerMessageDestroySurfac
> eWaitAsync),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_RESET_MEMSLOTS,
>                                  handle_dev_reset_memslots,
> @@ -1189,7 +1187,7 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  RED_WORKER_MESSAGE_MONITORS_CONFIG_A
> SYNC,
>                                  handle_dev_monitors_config_async,
>                                  sizeof(RedWorkerMessageMonitorsConfi
> gAsync),
> -                                DISPATCHER_ASYNC);
> +                                DISPATCHER_NONE);
>      dispatcher_register_handler(dispatcher,
>                                  RED_WORKER_MESSAGE_DRIVER_UNLOAD,
>                                  handle_dev_driver_unload,
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]