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@xxxxxxxxxx>
> >
> > 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list