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

Hi, On Tue, Jul 21, 2015 at 03:20:03PM +0100, Daniel P. Berrange 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. I've tested compilation on f22 with/without finding virNetworkGetDHCPLeases (I hacked the configure check for the latter). I do not have a RHEL 7.0 handy to test this.

> 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 > 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; An alternative would be to add a G_GNUC_UNUSED in the argument list as this only indicates that the argument may be unused (as opposed to "is really unused"). Both are fine with me.

> + gvir_set_error_literal(err, GVIR_NETWORK_ERROR, > + 0, > + "Unable to get network DHCP leases"); This should be g_set_error_literal().

ACK from me otherwise.