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

[Daniel P. Berrange]

On Tue, Jul 21, 2015 at 04:55:15PM +0200, Christophe Fergeau wrote:
> 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.

FYI I built a copy of 0.10.2 and tested against it

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

Yep

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

Ah good point.

> ACK from me otherwise.

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