qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v1 RFC 25/34] io: add QIOTask class for async op


From: Daniel P. Berrange
Subject: Re: [Qemu-devel] [PATCH v1 RFC 25/34] io: add QIOTask class for async operations
Date: Fri, 17 Apr 2015 18:38:32 +0100
User-agent: Mutt/1.5.23 (2014-03-12)

On Fri, Apr 17, 2015 at 07:06:04PM +0200, Paolo Bonzini wrote:
> 
> 
> On 17/04/2015 18:11, Daniel P. Berrange wrote:
> > 
> > +    task = qio_task_new(OBJECT(ioc),
> > +                        func, opaque, destroy);
> > +
> > +    qio_channel_tls_handshake_task(ioc, task);
> > +
> > +    object_unref(OBJECT(task));
> > 
> > The second ref taken at time of qio_channel_add_watch_full() gets released
> > by the GDestroyNotify that is passed to that method.
> 
> /me is blind.
> 
> So this means you really do not need the reference counting;
> qio_channel_tls_handshake_task (which is static anyway) is what manages
> the lifetime of the QIOTask, it can take ownership of the task right
> after it is created.
> 
> In effect QIOTask is just wrapping the (func, opaque, destroy)
> continuation.  It makes sense that it doesn't need reference counting,
> because it has a well-defined point of death (just before the
> continuation is called, or just before you find out it will never be
> called).  So I think it's okay to remove the reference counting and make
> it a simple heap-allocated struct.  gio_channel_tls_handshake_task can
> free it after calling gio_task_{abort,complete}.
> 
> Without the reference counting the GDestroyNotify also becomes
> unnecessary (and the fact that you're using QIOTask as a simple
> continuation explains why you didn't need GDestroyNotify), but it's okay
> if you want to leave it in.
> 
> I would also have QIOTaskFunc take the "exploded" struct, i.e. typedef
> void QIOTaskFunc(QMyObject *, gpointer, Error *), and hide QIOTask
> entirely from the user.

Yeah, I can try that approach and see how it works out.

Regards,
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 :|



reply via email to

[Prev in Thread] Current Thread [Next in Thread]