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