Re: [libvirt] [PATCH libvirt-glib 2/5] Add support for creating watches on streams

On Mon, Nov 28, 2011 at 01:13:45PM +0000, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > 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

> +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?

> + > + 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.

> +{ > + 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.