[Spice-devel] [PATCH spice-server] RFC: Remove DISPATCHER_ASYNC type
Jonathon Jongsma
jjongsma at redhat.com
Wed Sep 6 19:03:35 UTC 2017
---
>From a previous conversation:
On Wed, 2017-09-06 at 13:14 -0400, Frediano Ziglio wrote:
> > [OT: it's a little bit unclear to me why the async_done handler is
> > necessary at all. It is called immediately after the message-
> > specific
> > handler is called, and it is called from the same thread. So
> > anything
> > that is done in the async_done callback could just as easily be
> > done in
> > the message-specific handler, no? I suppose it allows you to reduce
> > code duplication slightly, but that seems to be the only advantage.
> > If
> > this async_done feature did not exist, we could simply use a
> > boolean
> > argument for whether to send an ACK or not, and the enum would be
> > unnecessary. But maybe I'm missing something.]
> >
>
> Maybe you really got a nice idea! Remove the ASYNC and have ACK/NONE
> !
> Would avoid the switch on the async call and also the code would run
> in the worker thread (that is code should be in a RedWorker or
> {Display,Cursor}Channel but actually is in RedQXL which should not
> have code that runs in the worker).
>
> Not checked how easy is...
>
> Frediano
I guess an initial implementation would look something like this. I didn't
really move any red-qxl code into red-worker, etc.
server/dispatcher.c | 11 -----------
server/dispatcher.h | 32 +-------------------------------
server/red-worker.c | 41 ++++++++++++++++++-----------------------
3 files changed, 19 insertions(+), 65 deletions(-)
diff --git a/server/dispatcher.c b/server/dispatcher.c
index 76f5eeaf4..931c7efa0 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;
};
@@ -309,8 +308,6 @@ static int dispatcher_handle_single_read(Dispatcher *dispatcher)
spice_printerr("error writing ack for message %d", type);
/* TODO: close socketpair? */
}
- } else if (msg->flag == DISPATCHER_FLAG_ASYNC && dispatcher->priv->handle_async_done) {
- dispatcher->priv->handle_async_done(dispatcher->priv->opaque, type, payload);
}
return 1;
}
@@ -359,14 +356,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, DispatcherFlag flag)
diff --git a/server/dispatcher.h b/server/dispatcher.h
index 28d78479e..837522243 100644
--- a/server/dispatcher.h
+++ b/server/dispatcher.h
@@ -85,15 +85,6 @@ typedef void (*dispatcher_handle_any_message)(void *opaque,
uint32_t message_type,
void *payload);
-/* The signature of a function that handles async done notifcations for all
- * messages that are registered with DISPATCHER_FLAG_ASYNC flag. See
- * dispatcher_register_async_done_callback() and dispatcher_register_handler()
- * for more information. */
-typedef void (*dispatcher_handle_async_done)(void *opaque,
- uint32_t message_type,
- void *payload);
-
-
/* dispatcher_send_message
*
* Sends a message to the receiving thread. The message type must have been
@@ -124,15 +115,10 @@ void dispatcher_send_message(Dispatcher *dispatcher, uint32_t message_type,
* since the sender will block until it receives the ACK. This is useful when
* the sender wants to ensure that the receiver has handled the message before
* proceeding.
- *
- * DISPATCHER_FLAG_ASYNC - When a message is received, the message handler is
- * called and then the dispatcher will call the registered async_done handler.
- * See dispatcher_register_async_done_callback() for more information
*/
typedef enum {
DISPATCHER_FLAG_NONE = 0,
- DISPATCHER_FLAG_ACK,
- DISPATCHER_FLAG_ASYNC
+ DISPATCHER_FLAG_ACK
} DispatcherFlag;
/* dispatcher_register_handler
@@ -155,22 +141,6 @@ void dispatcher_register_handler(Dispatcher *dispatcher, uint32_t message_type,
dispatcher_handle_message handler, size_t size,
DispatcherFlag flag);
-/* dispatcher_register_async_done_callback
- *
- * register an async_done callback for this dispatcher. For all message types
- * that were registered with a DISPATCHER_FLAG_ASYNC flag, this function will
- * be called after the normal message handler, and the message will be passed
- * to this function. Note that this function will execute within the receiving
- * thread.
- *
- * @dispatcher: dispatcher
- * @handler: callback on the receiver side called *after* the
- * message callback in case flag == DISPATCHER_FLAG_ASYNC.
- */
-void dispatcher_register_async_done_callback(
- Dispatcher *dispatcher,
- dispatcher_handle_async_done handler);
-
/* dispatcher_register_universal_handler
*
* Register a universal handler that will be called when *any* message is
diff --git a/server/red-worker.c b/server/red-worker.c
index c3f2dbfa0..cfeba7b35 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -431,6 +431,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)
@@ -577,14 +578,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 = 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)
@@ -684,15 +688,18 @@ 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 = 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)
@@ -701,6 +708,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)
@@ -797,6 +805,7 @@ static void handle_dev_monitors_config_async(void *opaque, void *payload)
display_channel_update_monitors_config(worker->display_channel, dev_monitors_config,
MIN(count, msg->max_monitors),
MIN(max_allowed, msg->max_monitors));
+ red_qxl_async_complete(worker->qxl, msg->base.cmd);
}
/* TODO: special, perhaps use another dispatcher? */
@@ -923,6 +932,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)
@@ -995,17 +1005,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;
@@ -1015,10 +1014,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_done);
-
/* TODO: register cursor & display specific msg in respective channel files */
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_DISPLAY_CONNECT,
@@ -1059,7 +1054,7 @@ static void register_callbacks(Dispatcher *dispatcher)
RED_WORKER_MESSAGE_UPDATE_ASYNC,
handle_dev_update_async,
sizeof(RedWorkerMessageUpdateAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_ADD_MEMSLOT,
handle_dev_add_memslot,
@@ -1069,7 +1064,7 @@ static void register_callbacks(Dispatcher *dispatcher)
RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC,
handle_dev_add_memslot_async,
sizeof(RedWorkerMessageAddMemslotAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_DEL_MEMSLOT,
handle_dev_del_memslot,
@@ -1084,7 +1079,7 @@ static void register_callbacks(Dispatcher *dispatcher)
RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC,
handle_dev_destroy_surfaces_async,
sizeof(RedWorkerMessageDestroySurfacesAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE,
handle_dev_destroy_primary_surface,
@@ -1094,12 +1089,12 @@ static void register_callbacks(Dispatcher *dispatcher)
RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC,
handle_dev_destroy_primary_surface_async,
sizeof(RedWorkerMessageDestroyPrimarySurfaceAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC,
handle_dev_create_primary_surface_async,
sizeof(RedWorkerMessageCreatePrimarySurfaceAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE,
handle_dev_create_primary_surface,
@@ -1134,7 +1129,7 @@ static void register_callbacks(Dispatcher *dispatcher)
RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC,
handle_dev_flush_surfaces_async,
sizeof(RedWorkerMessageFlushSurfacesAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_STOP,
handle_dev_stop,
@@ -1174,7 +1169,7 @@ static void register_callbacks(Dispatcher *dispatcher)
RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC,
handle_dev_destroy_surface_wait_async,
sizeof(RedWorkerMessageDestroySurfaceWaitAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_RESET_MEMSLOTS,
handle_dev_reset_memslots,
@@ -1184,7 +1179,7 @@ static void register_callbacks(Dispatcher *dispatcher)
RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC,
handle_dev_monitors_config_async,
sizeof(RedWorkerMessageMonitorsConfigAsync),
- DISPATCHER_FLAG_ASYNC);
+ DISPATCHER_FLAG_NONE);
dispatcher_register_handler(dispatcher,
RED_WORKER_MESSAGE_DRIVER_UNLOAD,
handle_dev_driver_unload,
--
2.13.3
More information about the Spice-devel
mailing list