On Mon, Nov 28, 2011 at 06:52:43PM +0100, Christophe Fergeau wrote:
On Mon, Nov 28, 2011 at 01:13:45PM +0000, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The GIO GInputStream/GOutputStream async model for I/O does not work for working with non-blocking bi-directional streams. To allow that to be done more effectively, add an API to allow main loop watches to be registered against streams.
Since the libvirt level virStreamEventAddCallback API only allows a single callback to be registered to a stream at any time, the GVirStream object needs to be multiplexing of multiple watches into a single libvirt level callback.
Watches can be removed in the normal way with g_source_remove
* libvirt-gobject/libvirt-gobject-stream.c, libvirt-gobject/libvirt-gobject-stream.h, libvirt-gobject/libvirt-gobject.sym: Add gvir_stream_add_watch --- libvirt-gobject/libvirt-gobject-stream.c | 180 ++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-stream.h | 17 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 198 insertions(+), 0 deletions(-)
@@ -199,6 +212,14 @@ static void gvir_stream_finalize(GObject *object) virStreamFree(priv->handle); }
+ tmp = priv->sources; + while (tmp) { + GVirStreamSource *source = tmp->data; + g_source_remove(g_source_get_id((GSource*)source));
I think g_source_destroy can be used here
Yes, I believe so.
+static void gvir_stream_source_finalize(GSource *source) +{ + GVirStreamSource *gsource = (GVirStreamSource*)source; + GVirStreamPrivate *priv = gsource->stream->priv; + GList *tmp, *prev = NULL; + + tmp = priv->sources; + while (tmp) { + if (tmp->data == source) { + if (prev) { + prev->next = tmp->next; + } else { + priv->sources = tmp->next; + } + tmp->next = NULL; + g_list_free(tmp); + break; + } + + prev = tmp; + tmp = tmp->next; + }
isn't it doing the same as g_list_remove?
Yes, indeed it is. I will simplify that.
+ + gvir_stream_update_events(gsource->stream); +} + +GSourceFuncs gvir_stream_source_funcs = { + .prepare = gvir_stream_source_prepare, + .check = gvir_stream_source_check, + .dispatch = gvir_stream_source_dispatch, + .finalize = gvir_stream_source_finalize, +}; + +gint gvir_stream_add_watch(GVirStream *stream, + GVirStreamIOCondition cond, + GVirStreamIOFunc func, + gpointer opaque, + GDestroyNotify notify)
Dunno if it's worth having both gvir_stream_add_watch and gvir_stream_add_watch_full to be consistent with most glib source functions (g_timeout_add, g_idle_add, g_io_add_watch, ...). The notify argument would only be in the _full variant.
Consistency is always nice, so I'll add a _full variant.
+{ + GVirStreamPrivate *priv = stream->priv; + gint id; + GVirStreamSource *source = (GVirStreamSource*)g_source_new(&gvir_stream_source_funcs, + sizeof(GVirStreamSource)); + + source->stream = stream; + source->cond = cond; + + priv->sources = g_list_append(priv->sources, source); + + gvir_stream_update_events(source->stream); + + g_source_set_callback((GSource*)source, (GSourceFunc)func, opaque, notify); + g_source_attach((GSource*)source, g_main_context_default()); + + id = g_source_get_id((GSource*)source);
g_source_attach returns this id which is of type guint.
Ahh, didn't notice that. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|