[libvirt] [PATCH glib] Make use of DHCP API conditionally compiled

Daniel P. Berrange berrange at redhat.com
Tue Jul 21 15:09:17 UTC 2015


On Tue, Jul 21, 2015 at 03:56:54PM +0100, Zeeshan Ali (Khattak) wrote:
> On Tue, Jul 21, 2015 at 3:20 PM, Daniel P. Berrange <berrange at redhat.com> wrote:
> > Previously the use of virDomainOpenGraphicsFD API from libvirt
> > 1.2.8 was made to be conditionally compiled. Given this past
> > practice, make use of the virNetworkGetDHCPLeases API
> > conditional too, rather than requiring newer libvirt.
> > ---
> >  configure.ac                                       |  6 ++-
> >  .../libvirt-gobject-network-dhcp-lease.c           | 50 ++++++++++++++++++++++
> >  libvirt-gobject/libvirt-gobject-network.c          | 12 ++++++
> >  3 files changed, 67 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 26beada..228788e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -9,7 +9,7 @@ AC_CANONICAL_HOST
> >
> >  AM_SILENT_RULES([yes])
> >
> > -LIBVIRT_REQUIRED=1.2.6
> > +LIBVIRT_REQUIRED=0.10.2
> >  AC_SUBST([LIBVIRT_REQUIRED]) dnl used in the .spec file
> >  GLIB2_REQUIRED=2.36.0
> >  AC_SUBST([GLIB2_REQUIRED]) dnl used in the .spec file
> > @@ -97,6 +97,10 @@ PKG_CHECK_MODULES(LIBVIRT, libvirt >= $LIBVIRT_REQUIRED)
> >  AC_CHECK_LIB([virt],
> >               [virDomainOpenGraphicsFD],
> >               [AC_DEFINE([HAVE_VIR_DOMAIN_OPEN_GRAPHICS_FD], 1, [Have virDomainOpenGraphicsFD?])])
> > +# virNetworkGetDHCPLeases was introduced in libvirt 1.2.6
> > +AC_CHECK_LIB([virt],
> > +             [virNetworkGetDHCPLeases],
> > +             [AC_DEFINE([HAVE_VIR_NETWORK_GET_DHCP_LEASES], 1, [Have virNetworkGetDHCPLeases?])])
> >  enable_tests=no
> >  PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_TEST_REQUIRED,
> >                    [enable_tests=yes],
> > diff --git a/libvirt-gobject/libvirt-gobject-network-dhcp-lease.c b/libvirt-gobject/libvirt-gobject-network-dhcp-lease.c
> > index 6ac3c14..90a402b 100644
> > --- a/libvirt-gobject/libvirt-gobject-network-dhcp-lease.c
> > +++ b/libvirt-gobject/libvirt-gobject-network-dhcp-lease.c
> > @@ -30,14 +30,20 @@
> >  #include "libvirt-glib/libvirt-glib.h"
> >  #include "libvirt-gobject/libvirt-gobject.h"
> >  #include "libvirt-gobject-compat.h"
> > +#ifdef HAVE_VIR_NETWORK_GET_DHCP_LEASES
> >  #include "libvirt-gobject/libvirt-gobject-network-dhcp-lease-private.h"
> > +#endif /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> >
> >  #define GVIR_NETWORK_DHCP_LEASE_GET_PRIVATE(obj)                         \
> >          (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_NETWORK_DHCP_LEASE, GVirNetworkDHCPLeasePrivate))
> >
> >  struct _GVirNetworkDHCPLeasePrivate
> >  {
> > +#ifdef HAVE_VIR_NETWORK_GET_DHCP_LEASES
> >      virNetworkDHCPLeasePtr handle;
> > +#else
> > +    void *handle;
> > +#endif /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> >  };
> >
> >  G_DEFINE_TYPE(GVirNetworkDHCPLease, gvir_network_dhcp_lease, G_TYPE_OBJECT);
> > @@ -75,8 +81,10 @@ static void gvir_network_dhcp_lease_set_property(GObject *object,
> >
> >      switch (prop_id) {
> >      case PROP_HANDLE:
> > +#ifdef HAVE_VIR_NETWORK_GET_DHCP_LEASES
> >          if (priv->handle)
> >              virNetworkDHCPLeaseFree(priv->handle);
> > +#endif /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> >          priv->handle = g_value_get_pointer(value);
> >          break;
> >
> > @@ -89,11 +97,15 @@ static void gvir_network_dhcp_lease_set_property(GObject *object,
> >  static void gvir_network_dhcp_lease_finalize(GObject *object)
> >  {
> >      GVirNetworkDHCPLease *lease = GVIR_NETWORK_DHCP_LEASE(object);
> > +#ifdef HAVE_VIR_NETWORK_GET_DHCP_LEASES
> >      GVirNetworkDHCPLeasePrivate *priv = lease->priv;
> > +#endif /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> >
> >      g_debug("Finalize GVirNetworkDHCPLease=%p", lease);
> 
> Why not just move this debug below to avoid adding two #ifdef here?

I want the debug message to be the first thing, so you see the
debug message in the case that the libvirt API call crashes the
program.

> > diff --git a/libvirt-gobject/libvirt-gobject-network.c b/libvirt-gobject/libvirt-gobject-network.c
> > index 45dbb71..a278105 100644
> > --- a/libvirt-gobject/libvirt-gobject-network.c
> > +++ b/libvirt-gobject/libvirt-gobject-network.c
> > @@ -29,7 +29,9 @@
> >  #include "libvirt-glib/libvirt-glib.h"
> >  #include "libvirt-gobject/libvirt-gobject.h"
> >  #include "libvirt-gobject-compat.h"
> > +#ifdef HAVE_VIR_NETWORK_GET_DHCP_LEASES
> >  #include "libvirt-gobject/libvirt-gobject-network-dhcp-lease-private.h"
> > +#endif /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> >
> >  #define GVIR_NETWORK_GET_PRIVATE(obj)                         \
> >          (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_NETWORK, GVirNetworkPrivate))
> > @@ -249,14 +251,17 @@ GList *gvir_network_get_dhcp_leases(GVirNetwork *network,
> >                                      guint flags,
> >                                      GError **err)
> >  {
> > +#ifdef HAVE_VIR_NETWORK_GET_DHCP_LEASES
> >      virNetworkDHCPLeasePtr *leases;
> >      GList *ret = NULL;
> >      int num_leases, i;
> > +#endif /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> >
> >      g_return_val_if_fail(GVIR_IS_NETWORK(network), NULL);
> >      g_return_val_if_fail(err == NULL || *err == NULL, NULL);
> >      g_return_val_if_fail(flags == 0, NULL);
> >
> > +#ifdef HAVE_VIR_NETWORK_GET_DHCP_LEASES
> 
> Similarly here. I'd just add on #ifdef. If macro is not defined, just
> return NULL.

You mean to skip the g_return_val_if_fail calls ?

> 
> >      num_leases = virNetworkGetDHCPLeases(network->priv->handle, mac, &leases, flags);
> >      if (num_leases < 0) {
> >          gvir_set_error_literal(err, GVIR_NETWORK_ERROR,
> > @@ -277,4 +282,11 @@ GList *gvir_network_get_dhcp_leases(GVirNetwork *network,
> >      free(leases);
> >
> >      return g_list_reverse(ret);
> > +#else /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> > +    (void)mac;
> > +    gvir_set_error_literal(err, GVIR_NETWORK_ERROR,
> > +                          0,
> > +                          "Unable to get network DHCP leases");
> > +    return NULL;
> > +#endif /* HAVE_VIR_NETWORK_GET_DHCP_LEASES */
> >  }

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




More information about the libvir-list mailing list