[Spice-devel] [RFC] server: add main_dispatcher

[Alon Levy]

On Tue, Oct 18, 2011 at 01:34:09PM +0200, Yonit Halperin wrote:
> On 09/08/2011 02:17 AM, Alon Levy wrote:
> Hi,
> I think this patch is sufficient for the 0.8 branch (+ the comments
> bellow + locks (for several display channels threads)).
> However for master, due to the red_channel refactoring I think we
> should have a more general dispatcher. It would be nice if (1) we
> could register events and their handlers, so that main_dispatcher
> won't need to know about the specific events. (2) the dispatcher
> will be bi-directional and red_dispatcher will reuse it.
> 

Ok for (1).

(2) is also doable - you want to have the dispatcher support both the
synchronous calls like all the old red_dispatcher (old as in age, not as
in obsolete) calls, and async like this patch adds and like all the
async red_dispatcher calls.

> Now that we have a main dispatcher, the calls to
> reds_update_mouse_mode should also be done via the red_dispatcher
> (those that are triggered by red_dispatcher_async_complete, and
> those that are executed from the vcpu thread).

I think you're right, without looking too much into it.

Ok, guess I'll do a respin for 0.8

> 
> <snip>
> 
> >+
> >+static void main_dispatcher_handle_read(int fd, int event, void *opaque)
> >+{
> >+    int ret;
> >+    MainDispatcher *md = opaque;
> >+    MainDispatcherMessage msg;
> >+
> >+    ret = read(md->main_fd,&msg, sizeof(msg));
> >+    if (ret == -1)
> >+        if (errno == EAGAIN) {
> >+            return;
> >+        } else {
> >+            red_printf("error reading from main dispatcher: %d\n", errno);
> >+            /* TODO: close channel? */
> >+            return;
> >+        }
> >+    }
> >+    if (ret != sizeof(msg)) {
> Even if it is very rare, I think we should support it and read till
> we get the whole msg.
> >+        red_printf("short read in main dispatcher: %d<  %zd\n", ret, sizeof(msg));
> >+        /* TODO: close channel?  */
> >+        return;
> >+    }
> >+
> >+    switch (msg.type) {
> >+        case MAIN_DISPATCHER_CHANNEL_EVENT:
> >+            main_dispatcher_handle_channel_event(&msg);
> >+            break;
> >+        default:
> >+            red_printf("error: unhandled main dispatcher message type %d\n",
> >+                       msg.type);
> >+    }
> >+}
> >+
> >+void main_dispatcher_init(SpiceCoreInterface *core)
> >+{
> >+    int channels[2];
> >+
> >+    memset(&main_dispatcher, 0, sizeof(main_dispatcher));
> >+    main_dispatcher.core = core;
> >+
> >+    if (socketpair(AF_LOCAL, SOCK_STREAM, 0, channels) == -1) {
> >+        red_error("socketpair failed %s", strerror(errno));
> >+        return;
> >+    }
> >+    main_dispatcher.main_fd = channels[0];
> >+    main_dispatcher.other_fd = channels[1];
> >+    main_dispatcher.self = pthread_self();
> >+
> >+    core->watch_add(main_dispatcher.main_fd, SPICE_WATCH_EVENT_READ,
> >+                    main_dispatcher_handle_read,&main_dispatcher);
> >+}
> >diff --git a/server/main_dispatcher.h b/server/main_dispatcher.h
> >new file mode 100644
> >index 0000000..2c201c7
> >--- /dev/null
> >+++ b/server/main_dispatcher.h
> >@@ -0,0 +1,9 @@
> >+#ifndef MAIN_DISPATCHER_H
> >+#define MAIN_DISPATCHER_H
> >+
> >+#include<spice.h>
> >+
> >+void main_dispatcher_channel_event(int event, SpiceChannelEventInfo *info);
> >+void main_dispatcher_init(SpiceCoreInterface *core);
> >+
> >+#endif //MAIN_DISPATCHER_H
> >diff --git a/server/reds.c b/server/reds.c
> >index 90779ff..ef9da4f 100644
> >--- a/server/reds.c
> >+++ b/server/reds.c
> >@@ -56,6 +56,7 @@
> >  #include "main_channel.h"
> >  #include "red_common.h"
> >  #include "red_dispatcher.h"
> >+#include "main_dispatcher.h"
> >  #include "snd_worker.h"
> >  #include<spice/stats.h>
> >  #include "stat.h"
> >@@ -322,7 +323,8 @@ static void reds_stream_channel_event(RedsStream *s, int event)
> >  {
> >      if (core->base.minor_version<  3 || core->channel_event == NULL)
> >          return;
> >-    core->channel_event(event,&s->info);
> >+
> >+    main_dispatcher_channel_event(event,&s->info);
> >  }
> >
> >  static ssize_t stream_write_cb(RedsStream *s, const void *buf, size_t size)
> >@@ -3519,6 +3521,7 @@ static int do_spice_init(SpiceCoreInterface *core_interface)
> >      init_vd_agent_resources();
> >      ring_init(&reds->clients);
> >      reds->num_clients = 0;
> >+    main_dispatcher_init(core);
> >
> >      if (!(reds->mig_timer = core->timer_add(migrate_timout, NULL))) {
> >          red_error("migration timer create failed");
>