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");
>
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
