On 11/01/2011 10:10 AM, Alon Levy wrote:
This patch reuses Dispatcher in RedDispatcher. It adds two helpers
to red_worker to keep RedWorker opaque to the outside. The dispatcher is
abused in three places that use the underlying socket directly:
once sending a READY after red_init completes
once for each channel creation, replying with the RedChannel instance
for cursor and display.
exploit this opportunity to rename red_dispatcher? ->worker_dispatcher?
display_dispatcher?
Code is being added, not removed.
Readability?
wrote a script to review the patch :)
---
server/red_dispatcher.c | 585 +++++++++++++++++++++++++++------------
server/red_dispatcher.h | 182 ++++++++++++
server/red_worker.c | 716 +++++++++++++++++++++++++----------------------
server/red_worker.h | 2 +
4 files changed, 979 insertions(+), 506 deletions(-)
diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
index 7a0033f..4680b50 100644
--- a/server/red_dispatcher.c
+++ b/server/red_dispatcher.c
@@ -37,6 +37,7 @@
#include "reds_gl_canvas.h"
#endif // USE_OPENGL
#include "reds.h"
+#include "dispatcher.h"
#include "red_dispatcher.h"
#include "red_parse_qxl.h"
@@ -53,7 +54,7 @@ struct AsyncCommand {
struct RedDispatcher {
QXLWorker base;
QXLInstance *qxl;
- int channel;
+ Dispatcher dispatcher;
pthread_t worker_thread;
uint32_t pending;
int primary_active;
@@ -83,24 +84,196 @@ extern spice_wan_compression_t zlib_glz_state;
static RedDispatcher *dispatchers = NULL;
+static void register_callbacks(RedDispatcher *dispatcher)
+{
+ dispatcher_register_handler(&dispatcher->dispatcher,
+ RED_WORKER_MESSAGE_DISPLAY_CONNECT,
+ handle_dev_display_connect,
+ sizeof(RedWorkerMessageDisplayConnect),
+ 0);
+ dispatcher_register_handler(&dispatcher->dispatcher,
+ RED_WORKER_MESSAGE_DISPLAY_DISCONNECT,
+ handle_dev_display_disconnect,
+ sizeof(RedWorkerMessageDisplayDisconnect),
+ 1);
+ dispatcher_register_handler(&dispatcher->dispatcher,
+ RED_WORKER_MESSAGE_DISPLAY_MIGRATE,
+ handle_dev_display_migrate,
+ sizeof(RedWorkerMessageDisplayMigrate),
+ 0);
<snip>
.
.
.
+ dispatcher_register_handler(&dispatcher->dispatcher,
+ RED_WORKER_MESSAGE_RESET_MEMSLOTS,
+ handle_dev_reset_memslots,
+ sizeof(RedWorkerMessageResetMemslots),
+ 0);
+}
+
What do you think about moving the registering the cbs to red_worker?
Then you will not need to add all the cbs prototype to red_dispatcher.h
(but you will need to expose Dispatcher to red_worker)
static void red_dispatcher_set_display_peer(RedChannel *channel, RedClient
*client,
RedsStream *stream, int migration,
int num_common_caps, uint32_t
*common_caps, int num_caps,
uint32_t *caps)
{
+ RedWorkerMessageDisplayConnect payload;
RedDispatcher *dispatcher;
red_printf("");
dispatcher = (RedDispatcher *)channel->data;
- RedWorkerMessage message = RED_WORKER_MESSAGE_DISPLAY_CONNECT;
- write_message(dispatcher->channel,&message);
- send_data(dispatcher->channel,&client, sizeof(RedClient *));
- send_data(dispatcher->channel,&stream, sizeof(RedsStream *));
- send_data(dispatcher->channel,&migration, sizeof(int));
+ payload.client = client;
+ payload.stream = stream;
+ payload.migration = migration;
+ dispatcher_send_message(&dispatcher->dispatcher,
+ RED_WORKER_MESSAGE_DISPLAY_CONNECT,
+&payload);
}
<snip>
diff --git a/server/red_worker.c b/server/red_worker.c
index 00efc05..6e5a238 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -866,8 +866,8 @@ typedef struct RedWorker {
CursorChannel *cursor_channel;
QXLInstance *qxl;
RedDispatcher *dispatcher;
- int id;
int channel;
+ int id;
int running;
uint32_t *pending;
int epoll;
@@ -10102,21 +10102,19 @@ static void surface_dirty_region_to_rects(RedSurface
*surface,
free(dirty_rects);
}
-static inline void handle_dev_update_async(RedWorker *worker)
+void handle_dev_update_async(void *opaque, void *payload)
{
- QXLRect qxl_rect;
+ RedWorker *worker = opaque;
+ RedWorkerMessageUpdateAsync *msg = payload;
SpiceRect rect;
- uint32_t surface_id;
- uint32_t clear_dirty_region;
QXLRect *qxl_dirty_rects;
uint32_t num_dirty_rects;
RedSurface *surface;
+ uint32_t surface_id = msg->surface_id;
+ QXLRect qxl_area = msg->qxl_area;
+ uint32_t clear_dirty_region = msg->clear_dirty_region;
- receive_data(worker->channel,&surface_id, sizeof(uint32_t));
- receive_data(worker->channel,&qxl_rect, sizeof(QXLRect));
- receive_data(worker->channel,&clear_dirty_region, sizeof(uint32_t));
-
- red_get_rect_ptr(&rect,&qxl_rect);
+ red_get_rect_ptr(&rect,&qxl_area);
flush_display_commands(worker);
ASSERT(worker->running);
@@ -10124,12 +10122,12 @@ static inline void handle_dev_update_async(RedWorker
*worker)
validate_surface(worker, surface_id);
red_update_area(worker,&rect, surface_id);
if (!worker->qxl->st->qif->update_area_complete) {
- return;
+ goto complete;
}
surface =&worker->surfaces[surface_id];
num_dirty_rects = pixman_region32_n_rects(&surface->draw_dirty_region);
if (num_dirty_rects == 0) {
- return;
+ goto complete;
}
qxl_dirty_rects = spice_new0(QXLRect, num_dirty_rects);
surface_dirty_region_to_rects(surface, qxl_dirty_rects, num_dirty_rects,
@@ -10137,26 +10135,25 @@ static inline void handle_dev_update_async(RedWorker
*worker)
worker->qxl->st->qif->update_area_complete(worker->qxl, surface_id,
qxl_dirty_rects, num_dirty_rects);
free(qxl_dirty_rects);
+
+complete:
+ red_dispatcher_async_complete(worker->dispatcher, msg->cmd);
You can register only one callback to all the async messages, and then
handle each one differently, but call red_dispatcher_async_complete for
all of them.
Thanks,
Yonit.
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
