[libvirt] [PATCH] build: define WITH_INTERFACE for the driver
Osier Yang
jyang at redhat.com
Fri Jun 29 07:34:30 UTC 2012
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
More information about the libvir-list
mailing list