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

On Tue, Jul 21, 2015 at 3:20 PM, Daniel P. Berrange <berrange(a)redhat.com&gt; 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?

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

> 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 */ > }