Re: [libvirt] [PATCH] build: define WITH_INTERFACE for the driver

Our code was mistakenly relying on an undefined macro, WITH_INTERFACE, for determining whether to load the interface driver which wraps the netcf library. Clean this situation up by having only one automake conditional for the driver, and having both WITH_NETCF (library detected) and WITH_INTERFACE (driver enabled) in C code, in case a future patch ever adds a network management via means other than the netcf library.

While at it, output more information at the conclusion of configure about the various drivers we enabled. * configure.ac: Enhance with_netcf, and add with_interface. Improve output to list final decisions. Replace WITH_NETCF with WITH_INTERFACE. * src/interface/netcf_driver.c: Rename... * src/interface/interface_driver.c: ...to this. * src/interface/interface_driver.h: Likewise. * daemon/Makefile.am (libvirtd_LDADD): Reflect better naming. * src/Makefile.am (libvirt_driver_interface_la_*): Likewise. (INTERFACE_DRIVER_SOURCES): Reflect file moves. * daemon/libvirtd.c (daemonInitialize): Likewise. * tools/virsh.c (vshShowVersion): Show both driver and library decisions. * libvirt.spec.in (with_interface): Tweak to deal with new usage as a real switch. --- I think this addresses the point that Osier raised here: https://www.redhat.com/archives/libvir-list/2012-June/msg01266.html but it is complex enough that I'd appreciate a careful review. configure.ac | 44 ++++++++++++++++---- daemon/Makefile.am | 2 +- daemon/libvirtd.c | 6 +-- libvirt.spec.in | 10 +++-- src/Makefile.am | 4 +- .../{netcf_driver.c => interface_driver.c} | 4 +- .../{netcf_driver.h => interface_driver.h} | 0 tools/virsh.c | 11 +++-- 8 files changed, 59 insertions(+), 22 deletions(-) rename src/interface/{netcf_driver.c => interface_driver.c} (99%) rename src/interface/{netcf_driver.h => interface_driver.h} (100%) diff --git a/configure.ac b/configure.ac index 6436885..a29b3b2 100644 --- a/configure.ac +++ b/configure.ac @@ -1755,6 +1755,7 @@ if test "$with_network" = "yes" ; then fi AM_CONDITIONAL([WITH_NETWORK], [test "$with_network" = "yes"]) +dnl check whether helper code is needed for above selections with_bridge=no if test "$with_qemu:$with_lxc:$with_network" != "no:no:no"; then with_bridge=yes @@ -1762,16 +1763,31 @@ if test "$with_qemu:$with_lxc:$with_network" != "no:no:no"; then fi AM_CONDITIONAL([WITH_BRIDGE], [test "$with_bridge" = "yes"]) -dnl netcf library +dnl check if the interface driver should be compiled + +AC_ARG_WITH([interface], + AC_HELP_STRING([--with-interface], + [with host interface driver @<:@default=check@:>@]),[], + [with_interface=check])

+ +dnl there's no use compiling the interface driver without the libvirt daemon +if test "$with_libvirtd" = "no"; then + with_interface=no +fi

+ +dnl The interface driver depends on the netcf library AC_ARG_WITH([netcf], AC_HELP_STRING([--with-netcf], [libnetcf support to configure physical host network interfaces @<:@default=check@:>@]), [], [with_netcf=check]) NETCF_CFLAGS= NETCF_LIBS= -if test "$with_libvirtd" = "no" ; then +if test "$with_libvirtd" = "no" || test "$with_interface" = "no"; then with_netcf=no fi +if test "$with_interface:$with_netcf" = "yes:check"; then + with_netcf=yes +fi if test "$with_netcf" = "yes" || test "$with_netcf" = "check"; then PKG_CHECK_MODULES(NETCF, netcf>= $NETCF_REQUIRED, [with_netcf=yes], [ @@ -1792,11 +1808,21 @@ if test "$with_netcf" = "yes" || test "$with_netcf" = "check"; then fi fi fi -AM_CONDITIONAL([WITH_NETCF], [test "$with_netcf" = "yes"]) AC_SUBST([NETCF_CFLAGS]) AC_SUBST([NETCF_LIBS]) +dnl Final decision on the interface driver +if test "$with_interface" = "check"; then + with_interface=$with_netcf +fi + +if test "$with_interface" = "yes" ; then + AC_DEFINE_UNQUOTED([WITH_INTERFACE], [1], + [whether interface driver is enabled]) +fi +AM_CONDITIONAL([WITH_INTERFACE], [test "$with_interface" = "yes"])

+dnl Check whether the Secrets driver is needed AC_ARG_WITH([secrets], AC_HELP_STRING([--with-secrets], [with local secrets management driver @<:@default=yes@:>@]),[],[with_secrets=yes]) @@ -2807,11 +2833,12 @@ AC_MSG_NOTICE([ ESX: $with_esx]) AC_MSG_NOTICE([ Hyper-V: $with_hyperv]) AC_MSG_NOTICE([ Test: $with_test]) AC_MSG_NOTICE([ Remote: $with_remote]) -AC_MSG_NOTICE([ Network: $with_network]) AC_MSG_NOTICE([Libvirtd: $with_libvirtd]) -AC_MSG_NOTICE([ netcf: $with_netcf])

-AC_MSG_NOTICE([ macvtap: $with_macvtap]) -AC_MSG_NOTICE([virtport: $with_virtualport]) +AC_MSG_NOTICE([ Network: $with_network]) +AC_MSG_NOTICE([ Iface: $with_interface]) +AC_MSG_NOTICE([ Secrets: $with_secrets]) +AC_MSG_NOTICE([ NodeDev: $with_nodedev]) +AC_MSG_NOTICE([NWfilter: $with_nwfilter]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Storage Drivers]) AC_MSG_NOTICE([]) @@ -2977,6 +3004,9 @@ AC_MSG_NOTICE([ Readline: $lv_use_readline]) AC_MSG_NOTICE([ Python: $with_python]) AC_MSG_NOTICE([ DTrace: $with_dtrace]) AC_MSG_NOTICE([ numad: $with_numad]) +AC_MSG_NOTICE([ bridge: $with_bridge]) +AC_MSG_NOTICE([ macvtap: $with_macvtap]) +AC_MSG_NOTICE([ virtport: $with_virtualport]) AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) AC_MSG_NOTICE([ Init script: $with_init_script]) AC_MSG_NOTICE([Console locks: $with_console_lock_files]) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 71e91cd..f0e422e 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -148,7 +148,7 @@ if WITH_NETWORK libvirtd_LDADD += ../src/libvirt_driver_network.la endif -if WITH_NETCF +if WITH_INTERFACE libvirtd_LDADD += ../src/libvirt_driver_interface.la endif diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 9c06344..a4f98cf 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -74,8 +74,8 @@ # ifdef WITH_NETWORK # include "network/bridge_driver.h" # endif -# ifdef WITH_NETCF -# include "interface/netcf_driver.h" +# ifdef WITH_INTERFACE +# include "interface/interface_driver.h" # endif # ifdef WITH_STORAGE # include "storage/storage_driver.h" @@ -400,7 +400,7 @@ static void daemonInitialize(void) # ifdef WITH_NETWORK networkRegister(); # endif -# ifdef WITH_NETCF +# ifdef WITH_INTERFACE interfaceRegister(); # endif # ifdef WITH_STORAGE diff --git a/libvirt.spec.in b/libvirt.spec.in index 896ef51..26ea2d9 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -70,6 +70,7 @@ # Then the secondary host drivers, which run inside libvirtd %define with_network 0%{!?_without_network:%{server_drivers}} +%define with_interface 0%{!?_without_interface:%{server_drivers}} %define with_storage_fs 0%{!?_without_storage_fs:%{server_drivers}} %define with_storage_lvm 0%{!?_without_storage_lvm:%{server_drivers}} %define with_storage_iscsi 0%{!?_without_storage_iscsi:%{server_drivers}} @@ -214,6 +215,7 @@ # The logic is the same as in configure.ac %if ! %{with_libvirtd} %define with_network 0 +%define with_interface 0 %define with_qemu 0 %define with_lxc 0 %define with_uml 0 @@ -267,9 +269,7 @@ %define with_nodedev 0 %endif -%if %{with_netcf} -%define with_interface 1 -%else +%if !%{with_netcf} %define with_interface 0 %endif @@ -1056,6 +1056,9 @@ of recent versions of Linux (and other OSes). %define _without_network --without-network %endif +%if ! %{with_interface} +%define _without_interface --without-interface +%endif %if ! %{with_storage_fs} %define _without_storage_fs --without-storage-fs %endif @@ -1171,6 +1174,7 @@ autoreconf -if %{?_without_hyperv} \ %{?_without_vmware} \ %{?_without_network} \ + %{?_without_interface} \ %{?_with_rhel5_api} \ %{?_without_storage_fs} \ %{?_without_storage_lvm} \

diff --git a/src/Makefile.am b/src/Makefile.am index 2309984..3cfaf01 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -483,7 +483,7 @@ NETWORK_DRIVER_SOURCES = \ network/bridge_driver.h network/bridge_driver.c INTERFACE_DRIVER_SOURCES = \ - interface/netcf_driver.h interface/netcf_driver.c + interface/interface_driver.h interface/interface_driver.c SECRET_DRIVER_SOURCES = \ secret/secret_driver.h secret/secret_driver.c @@ -924,7 +924,7 @@ EXTRA_DIST += network/default.xml -if WITH_NETCF +if WITH_INTERFACE if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_interface.la else diff --git a/src/interface/netcf_driver.c b/src/interface/interface_driver.c similarity index 99% rename from src/interface/netcf_driver.c rename to src/interface/interface_driver.c

index 45e6442..4959c72 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/interface_driver.c @@ -2,7 +2,7 @@ * interface_driver.c: backend driver methods to handle physical * interface configuration using the netcf library. * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -27,7 +27,7 @@ #include "virterror_internal.h" #include "datatypes.h" -#include "netcf_driver.h" +#include "interface_driver.h" #include "interface_conf.h" #include "memory.h" diff --git a/src/interface/netcf_driver.h b/src/interface/interface_driver.h similarity index 100% rename from src/interface/netcf_driver.h rename to src/interface/interface_driver.h diff --git a/tools/virsh.c b/tools/virsh.c index 53d1825..c1e7010 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -20832,15 +20832,18 @@ vshShowVersion(vshControl *ctl ATTRIBUTE_UNUSED) #ifdef WITH_NETWORK vshPrint(ctl, " Network"); #endif -#ifdef WITH_BRIDGE - vshPrint(ctl, " Bridging"); -#endif -#ifdef WITH_NETCF +#ifdef WITH_INTERFACE vshPrint(ctl, " Interface");

#endif #ifdef WITH_NWFILTER vshPrint(ctl, " Nwfilter"); #endif +#ifdef WITH_BRIDGE + vshPrint(ctl, " Bridging"); +#endif +#ifdef WITH_NETCF + vshPrint(ctl, " Netcf"); +#endif #ifdef WITH_VIRTUALPORT vshPrint(ctl, " VirtualPort"); #endif