[PATCH spice-server 1/2] Dispatcher: remove async_done callback

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

 



This callback was only executed for message types that were registered
with DISPATCHER_ASYNC ack type. However, the async_done handler was
called immediately after the message-specific handler and was called in
the same thread, so the async_done stuff can just as easily be done from
within the message-specific handler. This allows to simplify the
dispatcher_register_handler() method to simply require a boolean
argument for whether the message type requires an ACK or not.
---

I notice Frediano kept the enum type, but in this patch I changed it to
a simple boolean. Is there any reason to keep the enum?

 server/dispatcher.c | 19 +++-------
 server/dispatcher.h | 25 ++------------
 server/red-worker.c | 99 +++++++++++++++++++++++++----------------------------
 3 files changed, 53 insertions(+), 90 deletions(-)

diff --git a/server/dispatcher.c b/server/dispatcher.c
index 6f2c4d85e..97ebe32a6 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -40,7 +40,7 @@ static void setup_dummy_signal_handler(void);
 
 typedef struct DispatcherMessage {
     size_t size;
-    int ack;
+    bool ack;
     dispatcher_handle_message handler;
 } DispatcherMessage;
 
@@ -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;
 };
 
@@ -303,14 +302,12 @@ static int dispatcher_handle_single_read(Dispatcher *dispatcher)
     } else {
         spice_printerr("error: no handler for message type %d", type);
     }
-    if (msg->ack == DISPATCHER_ACK) {
+    if (msg->ack) {
         if (write_safe(dispatcher->priv->recv_fd,
                        (uint8_t*)&ack, sizeof(ack)) == -1) {
             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;
 }
@@ -346,7 +343,7 @@ void dispatcher_send_message(Dispatcher *dispatcher, uint32_t message_type,
                    message_type);
         goto unlock;
     }
-    if (msg->ack == DISPATCHER_ACK) {
+    if (msg->ack) {
         if (read_safe(send_fd, (uint8_t*)&ack, sizeof(ack), 1) == -1) {
             spice_printerr("error: failed to read ack");
         } else if (ack != ACK) {
@@ -359,17 +356,9 @@ 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)
+                                 size_t size, bool ack)
 {
     DispatcherMessage *msg;
 
diff --git a/server/dispatcher.h b/server/dispatcher.h
index 97b01de9c..eb93c1358 100644
--- a/server/dispatcher.h
+++ b/server/dispatcher.h
@@ -72,38 +72,17 @@ typedef void (*dispatcher_handle_async_done)(void *opaque,
 void dispatcher_send_message(Dispatcher *dispatcher, uint32_t message_type,
                              void *payload);
 
-enum {
-    DISPATCHER_NONE = 0,
-    DISPATCHER_ACK,
-    DISPATCHER_ASYNC
-};
-
 /*
  * dispatcher_register_handler
  * @dispatcher:     dispatcher
  * @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.
- *                  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.
+ * @ack:            whether the dispatcher should send an ACK to the sender
  */
 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);
+                                 bool ack);
 
 /*
  * Hack to allow red_record to see the message being sent so it can record
diff --git a/server/red-worker.c b/server/red-worker.c
index 675c232e7..36bcef796 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)
@@ -581,14 +582,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)
@@ -688,15 +692,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)
@@ -705,6 +712,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)
@@ -801,6 +809,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? */
@@ -927,6 +936,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)
@@ -999,17 +1009,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;
@@ -1019,196 +1018,192 @@ 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,
                                 handle_dev_display_connect,
                                 sizeof(RedWorkerMessageDisplayConnect),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DISPLAY_DISCONNECT,
                                 handle_dev_display_disconnect,
                                 sizeof(RedWorkerMessageDisplayDisconnect),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
                                 handle_dev_display_migrate,
                                 sizeof(RedWorkerMessageDisplayMigrate),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_CURSOR_CONNECT,
                                 handle_dev_cursor_connect,
                                 sizeof(RedWorkerMessageCursorConnect),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_CURSOR_DISCONNECT,
                                 handle_dev_cursor_disconnect,
                                 sizeof(RedWorkerMessageCursorDisconnect),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_CURSOR_MIGRATE,
                                 handle_dev_cursor_migrate,
                                 sizeof(RedWorkerMessageCursorMigrate),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_UPDATE,
                                 handle_dev_update,
                                 sizeof(RedWorkerMessageUpdate),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_UPDATE_ASYNC,
                                 handle_dev_update_async,
                                 sizeof(RedWorkerMessageUpdateAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_ADD_MEMSLOT,
                                 handle_dev_add_memslot,
                                 sizeof(RedWorkerMessageAddMemslot),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_ADD_MEMSLOT_ASYNC,
                                 handle_dev_add_memslot_async,
                                 sizeof(RedWorkerMessageAddMemslotAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DEL_MEMSLOT,
                                 handle_dev_del_memslot,
                                 sizeof(RedWorkerMessageDelMemslot),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DESTROY_SURFACES,
                                 handle_dev_destroy_surfaces,
                                 sizeof(RedWorkerMessageDestroySurfaces),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DESTROY_SURFACES_ASYNC,
                                 handle_dev_destroy_surfaces_async,
                                 sizeof(RedWorkerMessageDestroySurfacesAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE,
                                 handle_dev_destroy_primary_surface,
                                 sizeof(RedWorkerMessageDestroyPrimarySurface),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DESTROY_PRIMARY_SURFACE_ASYNC,
                                 handle_dev_destroy_primary_surface_async,
                                 sizeof(RedWorkerMessageDestroyPrimarySurfaceAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC,
                                 handle_dev_create_primary_surface_async,
                                 sizeof(RedWorkerMessageCreatePrimarySurfaceAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE,
                                 handle_dev_create_primary_surface,
                                 sizeof(RedWorkerMessageCreatePrimarySurface),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_RESET_IMAGE_CACHE,
                                 handle_dev_reset_image_cache,
                                 sizeof(RedWorkerMessageResetImageCache),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_RESET_CURSOR,
                                 handle_dev_reset_cursor,
                                 sizeof(RedWorkerMessageResetCursor),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_WAKEUP,
                                 handle_dev_wakeup,
                                 sizeof(RedWorkerMessageWakeup),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_OOM,
                                 handle_dev_oom,
                                 sizeof(RedWorkerMessageOom),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_START,
                                 handle_dev_start,
                                 sizeof(RedWorkerMessageStart),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC,
                                 handle_dev_flush_surfaces_async,
                                 sizeof(RedWorkerMessageFlushSurfacesAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_STOP,
                                 handle_dev_stop,
                                 sizeof(RedWorkerMessageStop),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_LOADVM_COMMANDS,
                                 handle_dev_loadvm_commands,
                                 sizeof(RedWorkerMessageLoadvmCommands),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_SET_COMPRESSION,
                                 handle_dev_set_compression,
                                 sizeof(RedWorkerMessageSetCompression),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_SET_STREAMING_VIDEO,
                                 handle_dev_set_streaming_video,
                                 sizeof(RedWorkerMessageSetStreamingVideo),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_SET_VIDEO_CODECS,
                                 handle_dev_set_video_codecs,
                                 sizeof(RedWorkerMessageSetVideoCodecs),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_SET_MOUSE_MODE,
                                 handle_dev_set_mouse_mode,
                                 sizeof(RedWorkerMessageSetMouseMode),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT,
                                 handle_dev_destroy_surface_wait,
                                 sizeof(RedWorkerMessageDestroySurfaceWait),
-                                DISPATCHER_ACK);
+                                true);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC,
                                 handle_dev_destroy_surface_wait_async,
                                 sizeof(RedWorkerMessageDestroySurfaceWaitAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_RESET_MEMSLOTS,
                                 handle_dev_reset_memslots,
                                 sizeof(RedWorkerMessageResetMemslots),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC,
                                 handle_dev_monitors_config_async,
                                 sizeof(RedWorkerMessageMonitorsConfigAsync),
-                                DISPATCHER_ASYNC);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_DRIVER_UNLOAD,
                                 handle_dev_driver_unload,
                                 sizeof(RedWorkerMessageDriverUnload),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_GL_SCANOUT,
                                 handle_dev_gl_scanout,
                                 sizeof(RedWorkerMessageGlScanout),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_GL_DRAW_ASYNC,
                                 handle_dev_gl_draw_async,
                                 sizeof(RedWorkerMessageGlDraw),
-                                DISPATCHER_NONE);
+                                false);
     dispatcher_register_handler(dispatcher,
                                 RED_WORKER_MESSAGE_CLOSE_WORKER,
                                 handle_dev_close,
                                 sizeof(RedWorkerMessageClose),
-                                DISPATCHER_NONE);
+                                false);
 }
 
 
-- 
2.13.3

_______________________________________________
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]