[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 16:49:55 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Fri, Apr 17, 2015 at 05:16:26PM +0200, Paolo Bonzini wrote:
>
>
> On 17/04/2015 16:22, Daniel P. Berrange wrote:
> > A number of I/O operations need to be performed asynchronously
> > to avoid blocking the main loop. The caller of such APIs need
> > to provide a callback to be invoked on completion/error and
> > need access to the error, if any. The small QIOTask provides
> > a simple framework for dealing with such probes. The API
> > docs inline provide an outline of how this is to be used.
> >
> > In this series, the QIOTask class will be used for things like
> > the TLS handshake, the websockets handshake and TCP connect()
> > progress.
>
> Is this actually worth making it a heavyweight QOM object? In general
> if you don't do object_property_add_child (and I don't think here you
> do), it's simpler to just make it a C struct.
Essentially I just used QOM anywhere that I wanted to have ref
counting, even if I didn't need the property feature. I figure
this particular usage isn't performance critical so using the
more heavyweight QOM framework wasn't the end of the world.
> In this case I even think you're leaking the task. You do object_ref
> twice (once at creation time, once before qio_channel_add_watch_full)
> and only have one object_unref. I would just do without reference
> counting, and add a qio_task_free() function that calls
> qio_task_finalize() and frees the QIOTask.
Are you referring to the qio_channel_tls_handshake() method in the
next patch ? If so it does actually have two object_unref calls
so shouldn't be leaking. In more complex scenarios I thnk the
ref counting ability will come in useful. Of course I could add
ref counting to a plain struct without using QOM, but it felt
better to just use QOM and be done with it, so people don't have
to remember which particular unref method they must use.
> BTW, do you have plans to use the GDestroyNotify argument to
> qio_task_new, or is it just for consistency?
I'm not 100% sure if I'll need it or not yet. One of my todo items
is to double check the sequence of operations when a VNC/chardev
client disconnects while a background task is in progress. It is
possible I might need to hold a reference on the VNC/chardev state
in which case the GDestroyNotify arg will come in useful. So I just
added it for consistency initially.
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 :|
- [Qemu-devel] [PATCH v1 RFC 21/34] io: add abstract QIOChannel classes, (continued)
- [Qemu-devel] [PATCH v1 RFC 21/34] io: add abstract QIOChannel classes, Daniel P. Berrange, 2015/04/17
- [Qemu-devel] [PATCH v1 RFC 20/34] ui: convert VNC to use generic cipher API, Daniel P. Berrange, 2015/04/17
- [Qemu-devel] [PATCH v1 RFC 22/34] io: add helper module for creating watches on UNIX FDs, Daniel P. Berrange, 2015/04/17
- [Qemu-devel] [PATCH v1 RFC 24/34] io: add QIOChannelFile class, Daniel P. Berrange, 2015/04/17
- [Qemu-devel] [PATCH v1 RFC 23/34] io: add QIOChannelSocket class, Daniel P. Berrange, 2015/04/17
- [Qemu-devel] [PATCH v1 RFC 25/34] io: add QIOTask class for async operations, Daniel P. Berrange, 2015/04/17
[Qemu-devel] [PATCH v1 RFC 26/34] io: add QIOChannelTLS class, Daniel P. Berrange, 2015/04/17
[Qemu-devel] [PATCH v1 RFC 28/34] io: add QIOChannelWebsock class, Daniel P. Berrange, 2015/04/17
[Qemu-devel] [PATCH v1 RFC 30/34] ui: convert VNC server to use QIOChannelTLS, Daniel P. Berrange, 2015/04/17
[Qemu-devel] [PATCH v1 RFC 31/34] ui: convert VNC server to use QIOChannelWebsock, Daniel P. Berrange, 2015/04/17
[Qemu-devel] [PATCH v1 RFC 32/34] char: convert from GIOChannel to QIOChannel, Daniel P. Berrange, 2015/04/17
[Qemu-devel] [PATCH v1 RFC 33/34] char: don't assume telnet initialization will not block, Daniel P. Berrange, 2015/04/17