[Libguestfs] [PATCH 5/7] New APIs: add-domain and add-libvirt-dom.

[Daniel P. Berrange]

On Wed, Nov 10, 2010 at 12:11:22PM +0000, Richard W.M. Jones wrote:
> On Wed, Nov 10, 2010 at 11:58:40AM +0000, Daniel P. Berrange wrote:
> > On Wed, Nov 10, 2010 at 11:46:31AM +0000, Richard W.M. Jones wrote:
> > > diff --git a/src/guestfs.h b/src/guestfs.h
> > > index 0f4f9fd..be7398a 100644
> > > --- a/src/guestfs.h
> > > +++ b/src/guestfs.h
> > > @@ -79,6 +79,11 @@ extern void guestfs_set_private (guestfs_h *g, const char *key, void *data);
> > >  extern void *guestfs_get_private (guestfs_h *g, const char *key);
> > >  
> > >  /*--- Structures and actions ---*/
> > > +
> > > +/* Hack to avoid needing to include <libvirt.h> */
> > > +typedef struct _virDomain virDomain;
> > > +typedef virDomain *virDomainPtr;
> > > +
> > >  #include <rpc/types.h>
> > >  #include <rpc/xdr.h>
> > >  #include <guestfs-structs.h>
> > 
> > Surely this will break any application whose code does
> > 
> >   #include <libvirt/libvirt.h>
> >   #include <guestfs.h>
> > 
> > because they'll get a duplicated definition of virDomain from
> > both headers. 
> 
> I checked and it works fine both ways round.  It would break if
> libvirt changed the definition, but you've boxed yourself into a
> corner there by documenting the current definition:

Surely the compiler should complain about duplicated typedefs, even
if they are identical, because C doesn't allow duplicate typedefs
within the same scope.

  http://david.tribble.com/text/cdiffs.htm#C99-typedefs

eg

  $ cat st.c 
  typedef struct foo foo;
  typedef struct foo foo;
  $ gcc -Wall -o st st.c
  st.c:3:20: error: redefinition of typedef ‘foo’
  st.c:2:20: note: previous declaration of ‘foo’ was here

> > IMHO, this hack is a waste of time and it is better to just
> > #include <libvirt/libvirt.h> from the header files instead.
> 
> The problem is libvirt would no longer be an optional dependency for
> programmers using libguestfs.
> 
> > A build time dependancy on the libvirt client is really not
> > a burden, when you consider that weight of the entire build
> > chain you already have to pull in. Even with the build time
> > dep, you can still have the dep be optional at runtime via
> > your use of dlopen().
> 
> In some quarters libvirt is not welcome and libguestfs doesn't depend
> on libvirt being there, but will use it if available to provide
> enhanced features.

Then perhaps the integration glue should be a separate library, 
so libguestfs.so provides the core infrastructure, and then a
libguest-libvirt.so provides higher level libvirt integration.
This way people who don't want it can avoid any dep, but apps
that do want libvirt can use the extra library and guarentee
they'll have the functionality there, avoiding any nasty runtime
surprises from libvirt being missing.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|