On 2012年06月29日 07:57, Eric Blake wrote:
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.
Foresighted. :-)
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])
Do we have to expose "with-interface"? It will give the user
a logic question, pick "with-interface", or 'with-netcf', or
both, even more when we have other implementations of interface
driver in future. however, the logic is simple, and we do it
inside actually: as long as one implementation of the interface
driver is picked to compile, we have the WITH_INTERFACE. so IMHO
no need to give the user the simple logic question. :-)
+
+dnl there's no use compiling the interface driver without the libvirt daemon
+if test "$with_libvirtd" = "no"; then
+ with_interface=no
+fi
If we don't expose 'with-interface', we don't need this......
+
+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"])
And above changes, what we need is just:
if test "$with_netfs" = "yes" || "with_something_else" = "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])
And no AC_MSG_NOTICE for $with_interface here, with keeping
$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} \
So, no these changes either.
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
We might want to rename it again if there is new implementations
in future. But it needs changes anyway, so it's fine.
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");
And no this.
#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
Regards,
Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
