[Spice-devel] [PATCH spice-server] EXPERIMENTAL: Remove DISPATCHER_ASYNC
Jonathon Jongsma
jjongsma at redhat.com
Wed Sep 6 19:50:42 UTC 2017
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,
More information about the Spice-devel
mailing list