[Libguestfs] Libguestfs gobject bindings

[Daniel P. Berrange]

On Fri, Jan 13, 2012 at 12:15:08PM +0000, Richard W.M. Jones wrote:
> On Fri, Jan 13, 2012 at 11:40:40AM +0000, Daniel P. Berrange wrote:
> > On Fri, Jan 13, 2012 at 11:33:33AM +0000, Matthew Booth wrote:
> > > On 01/13/2012 10:58 AM, Daniel P. Berrange wrote:
> > > >On Thu, Jan 12, 2012 at 11:59:22AM +0000, Matthew Booth wrote:
> > > >>I'm currently working on gobject bindings for libguestfs. I haven't
> > > >>got as far as compiling anything yet, but I've attached the C header
> > > >>for initial review.
> > > >>typedef struct _GuestfsPV GuestfsPV;
> > > >>struct _GuestfsPV {
> > > >>   gchar *pv_name;
> > > >>   /* The next field is NOT nul-terminated, be careful when printing it: */
> > > >>   gchar pv_uuid[32];
> > > >
> > > >How about a:
> > > >
> > > >    #define GUESTFS_LVM_UUID_BUFLEN 32
> > > >
> > > >and use that throughout
> > > 
> > > I don't think this one is massively important, because you can
> > > always use sizeof pv_uuid as an equivalent constant. As the API is
> > > automatically generated, these are always going to be consistent.
> 
> I think keep the definition in the gobject header.
> 
> > sizeof() works fine for C, but not for languages. If you add
> > the constant, then it'll get exposed to the languages automatically.
> >
> > > >For the vast majority of these, or at least, any which take non-trivial
> > > >execution time, I think it would be important to add _async variants,
> > > >following the GIO design pattern. eg
> > > >
> > > >   void guest_session_tune2fs_async(GuestfsSession *session, const gchar *device,
> > > >                                    GuestfsTune2FS *optargs,
> > > >                                    GCancellable *cancellable,
> > > >                                    GAsyncReadyCallback callback,
> > > >                                    gpointer user_data);
> > > >   gboolean guest_session_tune2fs_finish(GAsyncResult *result,
> > > >                                         GError **error);
> > > >
> > > >This is a style that is widely preferred by people writing modern GTK
> > > >applications, since it avoids the need to use threads in GUI apps.
> 
> Tricky to actually implement this though, without either creating a
> hidden thread somewhere, or substantially rearchitecting the
> libguestfs C API.

GIO provides all the infrastructure you need to do this, via the
GSimpleAsyncResult object, and its g_simple_async_result_run_in_thread
method.

> virt-manager & guestfs-browser use libguestfs by creating a separate
> thread and sending it instructions.  It's not that hard to implement,
> so for the moment I'd say forget about providing async functions.

GTK doesn't play nice with threads though, so you have to deal with
message passing back & forth between threads. With the way the GIO
framework works, threads are always hidden, so your app code just
deals with signals which always occurs in the main thread & can thus
use GTK safely.

> > The only issue would be if you wanted to add in 'GCancellable' to
> > the existing blocking APIs. I imagine there's no easy way to allow
> > cancellation of API calls at the libguestfs level, so it is probably
> > not worth it.
> 
> Actually, this is possible for some (by no means all) calls:
> http://libguestfs.org/guestfs.3.html#cancelling_long_transfers
> You can also kill the qemu subprocess, although you risk losing any
> state you were writing to the disk.

Ok, then for any of APIs which support this cancellation mode, I'd
strongly suggest adding the GCancellable * argument to the existing
method definitions.

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