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