[libvirt] [PATCH 0/9 v2] Refactor the configure.ac to reduce duplication
This is an update to https://www.redhat.com/archives/libvir-list/2012-September/msg01443.html Aside from the rebase, the main changes are all in the first patch, where I believe I addresses all Erics feedback with one exception. The exception was about the suggestion to support action-if-found, action-if-not-found, instead of the 2nd variant of macro. This is not actually possible, since the 2nd variant is not equivalent to a pair of calls to the 1st variant. To avoid flooding the list I've reduced this to just the first 9 patches. If these are approved there are another 30 patches to follow doing more conversions :-)
From: "Daniel P. Berrange" <berrange@redhat.com> Most checks for libraries take the same format * --with-libFOO=yes|no|check|/some/path argument * check for a function NNN in libFOO.so * check for a header file DDD/HHH.h * Define a WITH_FOO config.h symbol * Define a WITH_FOO make conditional * Substitute FOO_CFLAGS and FOO_LIBS make variables * Print CFLAGS & LIBS summary at the end Doing all this correctly is rather difficult, typically done by copy+paste of a previous usage. Further small improvements people make are not applied to all previous usages. Improve this by creating some helper macros to apply good practice. First, to perform the actual checks: LIBVIRT_CHECK_LIB([SELINUX], [selinux], [getfilecon], [selinux/selinux.h]) This checks for 'getfilecon' in -lselinux, and the existence of 'selinux/selinux.h' header file. If successful it sets SELINUX_CFLAGS and SELINUX_LIBS. The WITH_SELINUX config.h macro and WITH_SELINUX make conditional are also defined. In some cases we need to check two variants of the same library LIBVIRT_CHECK_LIB_ALT([SASL], [sasl2], [sasl_client_init], [sasl/sasl.h], [SASL1], [sasl], [sasl_client_init], [sasl/sasl.h]) This checks for sasl_client_init in libsasl2, and if that is not found, checks sasl_client_init in libsasl. If the first check succeeds WITH_SASL is set, while if the second check succeeds *both* WITH_SASL and WITH_SASL1 are set. If the library supports pkg-config, then another variant is available LIBVIRT_CHECK_PKG([AVAHI], [avahi-client], [0.6.0]) This checks for avahi-client >= 0.6.0 via pkg-config and sets WITH_AVAHI if found. Finally to print a summary of CFLAGS & LIBs found (if any): LIBVIRT_RESULT_LIB([SELINUX]) Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- m4/virt-lib.m4 | 391 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ m4/virt-result.m4 | 29 ++++ 2 files changed, 420 insertions(+) create mode 100644 m4/virt-lib.m4 create mode 100644 m4/virt-result.m4 diff --git a/m4/virt-lib.m4 b/m4/virt-lib.m4 new file mode 100644 index 0000000..669ec66 --- /dev/null +++ b/m4/virt-lib.m4 @@ -0,0 +1,391 @@ +dnl +dnl virt-lib.m4: Helper macros for checking for libraries +dnl +dnl Copyright (C) 2012-2013 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl + + +dnl Probe for existance of libXXXX and set WITH_XXX +dnl config header var, WITH_XXXX make conditional and +dnl with_XXX configure shell var. +dnl +dnl LIBVIRT_CHECK_LIB([CHECK_NAME], [LIBRARY_NAME], +dnl [FUNCTION_NAME], [HEADER_NAME]) +dnl +dnl CHECK_NAME: Suffix/prefix used for variables / flags, in uppercase. +dnl Used to set +dnl config.h: WITH_XXX macro +dnl Makefile: WITH_XXX conditional +dnl Makefile: XXX_CFLAGS, XXX_LIBS variables +dnl configure: --with-xxx argument +dnl configure: with_xxx variable +dnl +dnl LIBRARY_NAME: base name of library to check for eg libXXX.so +dnl FUNCTION_NAME: function to check for in libXXX.so +dnl HEADER_NAME: header file to check for +dnl +dnl e.g. +dnl +dnl LIBVIRT_CHECK_LIB([SELINUX], [selinux], +dnl [getfilecon], [selinux/selinux.h]) +dnl LIBVIRT_CHECK_LIB([SANLOCK], [sanlock_client], +dnl [sanlock_init], [sanlock.h]) +dnl LIBVIRT_CHECK_LIB([LIBATTR], [attr], +dnl [getxattr], [attr/attr.h]) +dnl +AC_DEFUN([LIBVIRT_CHECK_LIB],[ + m4_pushdef([check_name], [$1]) + m4_pushdef([library_name], [$2]) + m4_pushdef([function_name], [$3]) + m4_pushdef([header_name], [$4]) + + m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z')) + + m4_pushdef([config_var], [WITH_]check_name) + m4_pushdef([make_var], [WITH_]check_name) + m4_pushdef([cflags_var], check_name[_CFLAGS]) + m4_pushdef([libs_var], check_name[_LIBS]) + m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([with_var], [with_]check_name_lc) + + AC_ARG_WITH(check_name_lc, + [AS_HELP_STRING([--arg_var], + [with lib]]m4_dquote(library_name)[[ support @<:@default=check@:>@])], + [],[with_var][=check]) + + old_LIBS="$LIBS" + old_CFLAGS="$CFLAGS" + m4_expand(cflags_var[=]) + m4_expand(libs_var[=]) + + fail=0 + if test "x$with_var" != "xno" ; then + if test "x$with_var" != "xyes" && test "x$with_var" != "xcheck" ; then + m4_expand(cflags_var=["-I$with_var/include"]) + m4_expand(libs_var=["-L$with_var/lib"]) + fi + CFLAGS="$CFLAGS $cflags_var" + LIBS="$LIBS $libs_var" + AC_CHECK_LIB(library_name, function_name, [],[ + if test "x$with_var" != "xcheck"; then + fail=1 + fi + m4_expand(with_var[=no]) + ]) + if test "$fail" = "0" && test "x$with_var" != "xno" ; then + AC_CHECK_HEADER([header_name], [ + m4_expand(with_var[=yes]) + ],[ + if test "x$with_var" != "xcheck"; then + fail=1 + fi + m4_expand(with_var[=no]) + ]) + fi + fi + + LIBS="$old_LIBS" + CFLAGS="$old_CFLAGS" + + if test $fail = 1; then + AC_MSG_ERROR([You must install the lib]library_name[ library & headers to compile libvirt]) + else + if test "x$with_var" = "xyes" ; then + if test "x$libs_var" = 'x' ; then + m4_expand(libs_var=["-l]library_name["]) + else + m4_expand(libs_var=["$libs_var -l]library_name["]) + fi + AC_DEFINE_UNQUOTED(config_var, 1, [whether lib]library_name[ is available]) + fi + + AM_CONDITIONAL(make_var, [test "x$with_var" = "xyes"]) + + AC_SUBST(cflags_var) + AC_SUBST(libs_var) + fi + + m4_popdef([with_var]) + m4_popdef([arg_var]) + m4_popdef([libs_var]) + m4_popdef([cflags_var]) + m4_popdef([make_var]) + m4_popdef([config_var]) + + m4_popdef([check_name_lc]) + + m4_popdef([header_name]) + m4_popdef([function_name]) + m4_popdef([library_name]) + m4_popdef([check_name]) +]) + +dnl Probe for existance of libXXXX and set WITH_XXX +dnl config header var, WITH_XXXX make conditional and +dnl with_XXX configure shell var. +dnl +dnl LIBVIRT_CHECK_LIB_ALT([CHECK_NAME], [LIBRARY_NAME], +dnl [FUNCTION_NAME], [HEADER_NAME], +dnl [CHECK_NAME_ALT, [LIBRARY_NAME_ALT], +dnl [FUNCTION_NAME_ALT], [HEADER_NAME_ALT]) +dnl +dnl CHECK_NAME: Suffix/prefix used for variables / flags, in uppercase. +dnl Used to set +dnl config.h: WITH_XXX macro +dnl Makefile: WITH_XXX conditional +dnl Makefile: XXX_CFLAGS, XXX_LIBS variables +dnl configure: --with-xxx argument +dnl configure: with_xxx variable +dnl +dnl LIBRARY_NAME: base name of library to check for eg libXXX.so +dnl FUNCTION_NAME: function to check for in libXXX.so +dnl HEADER_NAME: header file to check for +dnl +dnl CHECK_NAME_ALT: Suffix/prefix used to set additional +dnl variables if alternative check suceeds +dnl config.h: WITH_XXX macro +dnl Makefile: WITH_XXX conditional +dnl NB all vars for CHECK_NAME are also set +dnl LIBRARY_NAME_ALT: alternative library name to check for +dnl FUNCTION_NAME_ALT: alternative function name to check for +dnl HEADER_NAME_ALT: alterantive header file to check for +dnl +dnl e.g. +dnl +dnl LIBVIRT_CHECK_LIB([YAJL], [yajl], +dnl [yajl_parse_complete], [yajl/yajl_common.h], +dnl [YAJL2], [yajl], +dnl [yajl_tree_parse], [yajl/yajl_common.h]) +dnl +AC_DEFUN([LIBVIRT_CHECK_LIB_ALT],[ + m4_pushdef([check_name], [$1]) + m4_pushdef([library_name], [$2]) + m4_pushdef([function_name], [$3]) + m4_pushdef([header_name], [$4]) + m4_pushdef([check_name_alt], [$5]) + m4_pushdef([library_name_alt], [$6]) + m4_pushdef([function_name_alt], [$7]) + m4_pushdef([header_name_alt], [$8]) + + m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z')) + + m4_pushdef([config_var], [WITH_]check_name) + m4_pushdef([make_var], [WITH_]check_name) + m4_pushdef([cflags_var], check_name[_CFLAGS]) + m4_pushdef([libs_var], check_name[_LIBS]) + m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([with_var], [with_]check_name_lc) + m4_pushdef([config_var_alt], [WITH_]check_name_alt) + m4_pushdef([make_var_alt], [WITH_]check_name_alt) + + AC_ARG_WITH(check_name_lc, + [AS_HELP_STRING([--arg_var], + [with lib]]m4_dquote(library_name)[[ support @<:@default=check@:>@])], + [],[with_var][=check]) + + old_LIBS="$LIBS" + old_CFLAGS="$CFLAGS" + m4_expand(cflags_var[=]) + m4_expand(libs_var[=]) + + fail=0 + alt=0 + if test "x$with_var" != "xno" ; then + if test "x$with_var" != "xyes" && test "x$with_var" != "xcheck" ; then + m4_expand(cflags_var=["-I$with_var/include"]) + m4_expand(libs_var=["-L$with_var/lib"]) + fi + CFLAGS="$CFLAGS $cflags_var" + LIBS="$LIBS $libs_var" + AC_CHECK_LIB(library_name, function_name, [],[ + AC_CHECK_LIB(library_name_alt, function_name_alt, [ + alt=1 + ],[ + if test "x$with_var" != "xcheck"; then + fail=1 + fi + m4_expand(with_var[=no]) + ]) + ]) + if test "$fail" = "0" && test "x$with_var" != "xno" ; then + AC_CHECK_HEADER([header_name], [ + m4_expand(with_var[=yes]) + ],[ + AC_CHECK_HEADER([header_name_alt], [ + m4_expand(with_var[=yes]) + ],[ + if test "x$with_var" != "xcheck"; then + fail=1 + fi + m4_expand(with_var[=no]) + ]) + ]) + fi + fi + + LIBS="$old_LIBS" + CFLAGS="$old_CFLAGS" + + if test $fail = 1; then + AC_MSG_ERROR([You must install the lib]library_name[ library & headers to compile libvirt]) + else + if test "x$with_var" = "xyes" ; then + if test "x$libs_var" = 'x' ; then + m4_expand(libs_var=["-l]library_name["]) + else + m4_expand(libs_var=["$libs_var -l]library_name["]) + fi + + AC_DEFINE_UNQUOTED(config_var, 1, [whether lib]library_name[ is available]) + if test "$alt" = "1" ; then + AC_DEFINE_UNQUOTED(config_var_alt, 1, [whether lib]library_name[ is available]) + fi + fi + + AM_CONDITIONAL(make_var, [test "x$with_var" = "xyes"]) + AM_CONDITIONAL(make_var_alt, [test "x$with_var" = "xyes" && test "$alt" = "1"]) + + AC_SUBST(cflags_var) + AC_SUBST(libs_var) + fi + + m4_popdef([make_var_alt]) + m4_popdef([config_var_alt]) + m4_popdef([with_var]) + m4_popdef([arg_var]) + m4_popdef([libs_var]) + m4_popdef([cflags_var]) + m4_popdef([make_var]) + m4_popdef([config_var]) + + m4_popdef([check_name_lc]) + + m4_popdef([header_name_alt]) + m4_popdef([function_name_alt]) + m4_popdef([library_name_alt]) + m4_popdef([header_name]) + m4_popdef([function_name]) + m4_popdef([library_name]) + m4_popdef([check_name]) +]) + +dnl +dnl Probe for existance of libXXXX and set WITH_XXX +dnl config header var, WITH_XXXX make conditional and +dnl with_XXX configure shell var. +dnl +dnl LIBVIRT_CHECK_PKG([CHECK_NAME], [PC_NAME], [PC_VERSION]) +dnl +dnl CHECK_NAME: Suffix/prefix used for variables / flags, in uppercase. +dnl Used to set +dnl config.h: WITH_XXX macro +dnl Makefile: WITH_XXX conditional +dnl Makefile: XXX_CFLAGS, XXX_LIBS variables +dnl configure: --with-xxx argument +dnl configure: with_xxx variable +dnl PC_NAME: Name of the pkg-config module +dnl PC_VERSION: Version of the pkg-config module +dnl +dnl eg +dnl +dnl LIBVIRT_CHECK_PKG([NETCF], [netcf], [0.1.4]) +dnl +AC_DEFUN([LIBVIRT_CHECK_PKG],[ + m4_pushdef([check_name], [$1]) + m4_pushdef([pc_name], [$2]) + m4_pushdef([pc_version], [$3]) + + m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z')) + + m4_pushdef([config_var], [WITH_]check_name) + m4_pushdef([make_var], [WITH_]check_name) + m4_pushdef([cflags_var], check_name[_CFLAGS]) + m4_pushdef([libs_var], check_name[_LIBS]) + m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([with_var], [with_]check_name_lc) + + AC_ARG_WITH(check_name_lc, + [AS_HELP_STRING([--arg_var], + [with ]]m4_dquote(pc_name)[[ (>= ]]m4_dquote(pc_version)[[) support @<:@default=check@:>@])], + [],[with_var][=check]) + + fail=0 + if test "x$with_var" != "xno" ; then + PKG_CHECK_MODULES(check_name, pc_name[ >= ]pc_version, [ + m4_expand(with_var[=yes]) + ],[ + if test "x$with_var" != "xcheck"; then + fail=1 + fi + m4_expand(with_var[=no]) + ]) + fi + + if test $fail = 1; then + AC_MSG_ERROR([You must install the ]pc_name[ >= ]pc_version[pkg-config module to compile libvirt]) + fi + + if test "x$with_var" = "xyes" ; then + AC_DEFINE_UNQUOTED(config_var, 1, [whether ]pc_name[ >= ]pc_version[ is available]) + fi + + AM_CONDITIONAL(make_var, [test "x$with_var" = "xyes"]) + + m4_popdef([with_var]) + m4_popdef([arg_var]) + m4_popdef([libs_var]) + m4_popdef([cflags_var]) + m4_popdef([make_var]) + m4_popdef([config_var]) + + m4_popdef([check_name_lc]) + + m4_popdef([pc_version]) + m4_popdef([pc_name]) + m4_popdef([check_name]) +]) + +dnl +dnl To be used after a call to LIBVIRT_CHECK_LIB, +dnl LIBVIRT_CHECK_LIB_ALT or LIBVIRT_CHECK_PKG +dnl to print the result status +dnl +dnl LIBVIRT_RESULT_LIB([CHECK_NAME]) +dnl +dnl CHECK_NAME: Suffix/prefix used for variables / flags, in uppercase. +dnl +dnl LIBVIRT_RESULT_LIB([SELINUX]) +dnl +AC_DEFUN([LIBVIRT_RESULT_LIB],[ + m4_pushdef([check_name], [$1]) + + m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z')) + + m4_pushdef([cflags_var], check_name[_CFLAGS]) + m4_pushdef([libs_var], check_name[_LIBS]) + m4_pushdef([with_var], [with_]check_name_lc) + + LIBVIRT_RESULT(check_name_lc, [$with_var], [CFLAGS='$cflags_var' LIBS='$libs_var']) + + m4_popdef([with_var]) + m4_popdef([libs_var]) + m4_popdef([cflags_var]) + + m4_popdef([check_name_lc]) + + m4_popdef([check_name]) +]) diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 new file mode 100644 index 0000000..6eef35a --- /dev/null +++ b/m4/virt-result.m4 @@ -0,0 +1,29 @@ +dnl +dnl virt-lib.m4: Helper macros for checking for libraries +dnl +dnl Copyright (C) 2012-2013 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl + +AC_DEFUN([LIBVIRT_RESULT], [ + if test "$2" = "no" || test -z "$3" ; then + STR=`printf "%10s: %-3s" "$1" "$2"` + else + STR=`printf "%10s: %-3s (%s)" "$1" "$2" "$3"` + fi + + AC_MSG_NOTICE([$STR]) +]) -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Most checks for libraries take the same format
* --with-libFOO=yes|no|check|/some/path argument * check for a function NNN in libFOO.so * check for a header file DDD/HHH.h * Define a WITH_FOO config.h symbol * Define a WITH_FOO make conditional * Substitute FOO_CFLAGS and FOO_LIBS make variables * Print CFLAGS & LIBS summary at the end
General impression - nice! Putting on my autoconf maintainer hat: full of non-robust underquoted potential gotchas, for example if someone ever tries to link to a library named DNL. I'll go ahead and point out LOTS of instances of where you would use "m4_defn([var])" instead of plain "var" to avoid those issues, but before you hack up code to deal with all my nits (and realize that a lot more lines of code need the same "fixes" than just those I called out), read this entire message.
--- m4/virt-lib.m4 | 391 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ m4/virt-result.m4 | 29 ++++ 2 files changed, 420 insertions(+) create mode 100644 m4/virt-lib.m4 create mode 100644 m4/virt-result.m4
diff --git a/m4/virt-lib.m4 b/m4/virt-lib.m4 new file mode 100644 index 0000000..669ec66 --- /dev/null +++ b/m4/virt-lib.m4 @@ -0,0 +1,391 @@ +dnl +dnl virt-lib.m4: Helper macros for checking for libraries
+ +dnl Probe for existance of libXXXX and set WITH_XXX
s/existance/existence/
+dnl config header var, WITH_XXXX make conditional and +dnl with_XXX configure shell var. +dnl +dnl LIBVIRT_CHECK_LIB([CHECK_NAME], [LIBRARY_NAME], +dnl [FUNCTION_NAME], [HEADER_NAME]) +dnl
+dnl +AC_DEFUN([LIBVIRT_CHECK_LIB],[ + m4_pushdef([check_name], [$1]) + m4_pushdef([library_name], [$2]) + m4_pushdef([function_name], [$3]) + m4_pushdef([header_name], [$4]) + + m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z'))
Pedantic: Underquoted (using a library named 'libdnl' would wreak havoc) Real (but harmless) bug: you used the wrong quote characters ([] not `'); not to mention autoconf already provides a helper macro m4_tolower that does the same thing in less typing. Recommendation: replace this line with either: libvirt-quality: m4_pushdef([check_name_lc], m4_tolower(check_name)) pedantic-quality: m4_pushdef([check_name_lc], m4_tolower(m4_defn([check_name])))
+ + m4_pushdef([config_var], [WITH_]check_name)
Pedantic-quality: m4_pushdef([config_var], [WITH_]m4_defn([check_name]))
+ m4_pushdef([make_var], [WITH_]check_name) + m4_pushdef([cflags_var], check_name[_CFLAGS]) + m4_pushdef([libs_var], check_name[_LIBS]) + m4_pushdef([arg_var], [with-]check_name_lc) + m4_pushdef([with_var], [with_]check_name_lc)
etc. with s/var/m4_defn([var])/
+ + AC_ARG_WITH(check_name_lc, + [AS_HELP_STRING([--arg_var], + [with lib]]m4_dquote(library_name)[[ support @<:@default=check@:>@])], + [],[with_var][=check])
We hashed this one out on IRC :) It's fine for libvirt, but for pedantic quality, it would be better as: AC_ARG_WITH(m4_defn([check_name_lc]), [AS_HELP_STRING([--]m4_defn([arg_var]), [with lib]]m4_dquote( m4_defn([library_name]))[[ support @<:@default=check@:>@])], [], m4_defn([with_var])[=check])
+ + old_LIBS="$LIBS" + old_CFLAGS="$CFLAGS"
Pedantic: The "" are not necessary here, but don't hurt either.
+ m4_expand(cflags_var[=]) + m4_expand(libs_var[=])
Overkill; you could get by with: libvirt quality: cflags_var= libs_var= Pedantic quality: m4_defn([cflags_var])= m4_defn([libs_var])=
+ + fail=0 + if test "x$with_var" != "xno" ; then
Pedantic: if test "x$m4_defn([with_var])" != "xno"; then
+ if test "x$with_var" != "xyes" && test "x$with_var" != "xcheck" ; then + m4_expand(cflags_var=["-I$with_var/include"]) + m4_expand(libs_var=["-L$with_var/lib"])
Overkill. libvirt-quality: cflags_var="-I$with_var/include" Pedantic: m4_defn([cflags_var])="-I$m4_defn([with_var])/include"
+ fi + CFLAGS="$CFLAGS $cflags_var" + LIBS="$LIBS $libs_var" + AC_CHECK_LIB(library_name, function_name, [],[
AC_CHECK_LIB(m4_defn([library_name]), m4_defn([function_name]), [], [
+ if test "x$with_var" != "xcheck"; then + fail=1 + fi + m4_expand(with_var[=no])
m4_defn([with_var])=no
+ ]) + if test "$fail" = "0" && test "x$with_var" != "xno" ; then + AC_CHECK_HEADER([header_name], [
AC_CHECK_HEADER(m4_defn([header_name]), [
+ m4_expand(with_var[=yes])
m4_defn([with_var])=yes
+ ],[ + if test "x$with_var" != "xcheck"; then + fail=1 + fi + m4_expand(with_var[=no]) + ]) + fi + fi + + LIBS="$old_LIBS" + CFLAGS="$old_CFLAGS"
Again, "" aren't needed, but don't hurt.
+ + if test $fail = 1; then + AC_MSG_ERROR([You must install the lib]library_name[ library & headers to compile libvirt])
It would be really nice if you could run './configure' once and know _all_ of the libraries to be installed, rather than having to run once per library because each missing library aborts the script immediately. I can probably do that as a followup patch, where instead of directly issuing the error, we instead append the latest error string to a series of messages, then use a single m4_wrap to do AC_MSG_ERROR at the end of all collected messages. But that doesn't impact this patch.
+ else + if test "x$with_var" = "xyes" ; then + if test "x$libs_var" = 'x' ; then + m4_expand(libs_var=["-l]library_name["])
Overkill. libvirt-quality: libs_var="-l[]library_name" pedantic: m4_defn([libs_var])="-l[]m4_defn([library_name])"
+ else + m4_expand(libs_var=["$libs_var -l]library_name["]) + fi + AC_DEFINE_UNQUOTED(config_var, 1, [whether lib]library_name[ is available])
Pedantic: AC_DEFINE_UNQUOTED(m4_defn([config_var]), [1], [whether lib]m4_defn([library_name])[ is available])
+ fi + + AM_CONDITIONAL(make_var, [test "x$with_var" = "xyes"]) + + AC_SUBST(cflags_var)
AC_SUBST(m4_defn([cflags_var]))
+dnl +dnl CHECK_NAME_ALT: Suffix/prefix used to set additional +dnl variables if alternative check suceeds
s/suceeds/succeeds/
+dnl config.h: WITH_XXX macro +dnl Makefile: WITH_XXX conditional +dnl NB all vars for CHECK_NAME are also set +dnl LIBRARY_NAME_ALT: alternative library name to check for +dnl FUNCTION_NAME_ALT: alternative function name to check for +dnl HEADER_NAME_ALT: alterantive header file to check for
s/alterantive/alternative/
+ m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z'))
Again, wrong quote characters, and m4_tolower already does this for you.
+ +dnl +dnl Probe for existance of libXXXX and set WITH_XXX
s/existance/existence/
+AC_DEFUN([LIBVIRT_CHECK_PKG],[ + m4_pushdef([check_name], [$1]) + m4_pushdef([pc_name], [$2]) + m4_pushdef([pc_version], [$3]) + + m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z'))
And again.
+ AC_ARG_WITH(check_name_lc, + [AS_HELP_STRING([--arg_var], + [with ]]m4_dquote(pc_name)[[ (>= ]]m4_dquote(pc_version)[[) support @<:@default=check@:>@])],
This is a long line; you can break anywhere after a ( with no impact, to stay within 80 columns, such as ... m4-dquote( pc_version)...
+ +dnl +dnl To be used after a call to LIBVIRT_CHECK_LIB, +dnl LIBVIRT_CHECK_LIB_ALT or LIBVIRT_CHECK_PKG +dnl to print the result status +dnl +dnl LIBVIRT_RESULT_LIB([CHECK_NAME]) +dnl +dnl CHECK_NAME: Suffix/prefix used for variables / flags, in uppercase. +dnl +dnl LIBVIRT_RESULT_LIB([SELINUX]) +dnl +AC_DEFUN([LIBVIRT_RESULT_LIB],[ + m4_pushdef([check_name], [$1]) + + m4_pushdef([check_name_lc], m4_translit(check_name, `A-Z', `a-z'))
One more place for m4_tolower
+++ b/m4/virt-result.m4
+ +AC_DEFUN([LIBVIRT_RESULT], [
No doc comment beforehand? Basically, all of my suggestions for using more m4_defn are for safety against tricky library names, but don't really impact the common case where a library name cannot be confused with an m4 macro. And libvirt doesn't use any libraries with an unsafe name. My priority listing of which things to fix (or ignore): * spelling errors - must fix before committing * use m4_tolower instead of m4_translit - please fix before committing * doc comment for LIBVIRT_RESULT - please fix before committing * avoid m4_expand - up to you, but it looks simpler if you fix to at least libvirt quality * use m4_defn in more places - up to you, and I'll look the other way if you choose not to fix ACK once you cover at least the first bullet, and preferably at least the first 3 bullets; and of course if your testing of later patches in the series shows that things still work. I don't need to see a v2 unless you go for all 5 bullets. (Part of me wonders if I should take the best parts of these macros, make them more robust to quoting, and make it officially part of some future release of autoconf.) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Jan 10, 2013 at 03:20:27PM -0700, Eric Blake wrote:
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
+ + if test $fail = 1; then + AC_MSG_ERROR([You must install the lib]library_name[ library & headers to compile libvirt])
It would be really nice if you could run './configure' once and know _all_ of the libraries to be installed, rather than having to run once per library because each missing library aborts the script immediately. I can probably do that as a followup patch, where instead of directly issuing the error, we instead append the latest error string to a series of messages, then use a single m4_wrap to do AC_MSG_ERROR at the end of all collected messages. But that doesn't impact this patch.
Hmm, I guess my vision is that in typical usage all the library/pkg tests will always default to 'check', so we'll test them all and report success/failure at the end. If you're actually using --with-sasl=yes, then I'm assuming you've looked at the configure script and decided what you need upfront.
My priority listing of which things to fix (or ignore):
* spelling errors - must fix before committing * use m4_tolower instead of m4_translit - please fix before committing * doc comment for LIBVIRT_RESULT - please fix before committing * avoid m4_expand - up to you, but it looks simpler if you fix to at least libvirt quality * use m4_defn in more places - up to you, and I'll look the other way if you choose not to fix
ACK once you cover at least the first bullet, and preferably at least the first 3 bullets; and of course if your testing of later patches in the series shows that things still work. I don't need to see a v2 unless you go for all 5 bullets.
Ok, I've made changes 1->4 and pushed 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 :|
On 01/11/2013 03:37 AM, Daniel P. Berrange wrote:
On Thu, Jan 10, 2013 at 03:20:27PM -0700, Eric Blake wrote:
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
+ + if test $fail = 1; then + AC_MSG_ERROR([You must install the lib]library_name[ library & headers to compile libvirt])
It would be really nice if you could run './configure' once and know _all_ of the libraries to be installed, rather than having to run once per library because each missing library aborts the script immediately. I can probably do that as a followup patch, where instead of directly issuing the error, we instead append the latest error string to a series of messages, then use a single m4_wrap to do AC_MSG_ERROR at the end of all collected messages. But that doesn't impact this patch.
Hmm, I guess my vision is that in typical usage all the library/pkg tests will always default to 'check', so we'll test them all and report success/failure at the end. If you're actually using --with-sasl=yes, then I'm assuming you've looked at the configure script and decided what you need upfront.
But in reality, there are several instances where asking for one feature implies that a particular library becomes mandatory. For example, even if you don't specify --with-yajl at all, merely compiling './configure --with-qemu' on a system with qemu 1.x will behave as if you had done --with-yajl=yes instead of --with-yajl=check. Hmm, another thought - when do we decide which features need which libraries? One style is to probe for all libraries first, and then run through features, failing the features that are lacking a dependent library. This would still benefit from the idea of listing all errors at once, instead of one error per run. However, if you disable features (such as --without-libvirtd), checking for those libraries could be a waste of configure time. Another style would be to default all libraries to 'no', then run through all features, and each feature changes the libraries it depends on to either 'check' (if the dependency is optional) or 'yes' (if mandatory), then do the library checks (hitting just the libraries that are needed), and finally back to the feature checks to see the results of the libraries. But all of this is grounds for further improvement, and doesn't really impact the usefulness of your current series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Jan 11, 2013 at 10:01:45AM -0700, Eric Blake wrote:
On 01/11/2013 03:37 AM, Daniel P. Berrange wrote:
On Thu, Jan 10, 2013 at 03:20:27PM -0700, Eric Blake wrote:
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
+ + if test $fail = 1; then + AC_MSG_ERROR([You must install the lib]library_name[ library & headers to compile libvirt])
It would be really nice if you could run './configure' once and know _all_ of the libraries to be installed, rather than having to run once per library because each missing library aborts the script immediately. I can probably do that as a followup patch, where instead of directly issuing the error, we instead append the latest error string to a series of messages, then use a single m4_wrap to do AC_MSG_ERROR at the end of all collected messages. But that doesn't impact this patch.
Hmm, I guess my vision is that in typical usage all the library/pkg tests will always default to 'check', so we'll test them all and report success/failure at the end. If you're actually using --with-sasl=yes, then I'm assuming you've looked at the configure script and decided what you need upfront.
But in reality, there are several instances where asking for one feature implies that a particular library becomes mandatory. For example, even if you don't specify --with-yajl at all, merely compiling './configure --with-qemu' on a system with qemu 1.x will behave as if you had done --with-yajl=yes instead of --with-yajl=check.
Hmm, another thought - when do we decide which features need which libraries? One style is to probe for all libraries first, and then run through features, failing the features that are lacking a dependent library. This would still benefit from the idea of listing all errors at once, instead of one error per run. However, if you disable features (such as --without-libvirtd), checking for those libraries could be a waste of configure time.
Another style would be to default all libraries to 'no', then run through all features, and each feature changes the libraries it depends on to either 'check' (if the dependency is optional) or 'yes' (if mandatory), then do the library checks (hitting just the libraries that are needed), and finally back to the feature checks to see the results of the libraries.
As structured today we're obviously checking for all libraries separately from hypervisors. My intent long term though is that we introduce macros for the hypervisors which conditionally require the libraries. This is sort of inspired by the way gnulib does dependancies between its various m4 modules. Amd of course --with-qemu shoudl default to check, so you'd get something like this printed Checking if QEMU can be enabled....no. missing yajl, gnutls, sasl 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 :|
On 01/10/2013 03:20 PM, Eric Blake wrote:
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Most checks for libraries take the same format
* --with-libFOO=yes|no|check|/some/path argument * check for a function NNN in libFOO.so * check for a header file DDD/HHH.h * Define a WITH_FOO config.h symbol * Define a WITH_FOO make conditional * Substitute FOO_CFLAGS and FOO_LIBS make variables * Print CFLAGS & LIBS summary at the end
General impression - nice!
+ m4_expand(cflags_var[=]) + m4_expand(libs_var[=])
Overkill; you could get by with:
libvirt quality: cflags_var= libs_var=
Pedantic quality: m4_defn([cflags_var])= m4_defn([libs_var])=
Worse, broken on RHEL 5, where autoconf 2.59 lacks m4_expand: ./configure: line 90263: syntax error near unexpected token `with_audit=no' ./configure: line 90263: ` m4_expand(with_audit=no)' I'll push the obvious patch shortly. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Fri, Jan 11, 2013 at 11:09:15AM -0700, Eric Blake wrote:
On 01/10/2013 03:20 PM, Eric Blake wrote:
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Most checks for libraries take the same format
* --with-libFOO=yes|no|check|/some/path argument * check for a function NNN in libFOO.so * check for a header file DDD/HHH.h * Define a WITH_FOO config.h symbol * Define a WITH_FOO make conditional * Substitute FOO_CFLAGS and FOO_LIBS make variables * Print CFLAGS & LIBS summary at the end
General impression - nice!
+ m4_expand(cflags_var[=]) + m4_expand(libs_var[=])
Overkill; you could get by with:
libvirt quality: cflags_var= libs_var=
Pedantic quality: m4_defn([cflags_var])= m4_defn([libs_var])=
Worse, broken on RHEL 5, where autoconf 2.59 lacks m4_expand:
./configure: line 90263: syntax error near unexpected token `with_audit=no' ./configure: line 90263: ` m4_expand(with_audit=no)'
Oh, I got rid of all the m4_expand() bits quoted above, but missed some other usages 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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 6 +++--- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/util/virjson.c | 14 +++++++------- tests/Makefile.am | 2 +- tests/qemuhelptest.c | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/configure.ac b/configure.ac index 5acba9b..988d167 100644 --- a/configure.ac +++ b/configure.ac @@ -1206,15 +1206,15 @@ if test "x$with_yajl" != "xno"; then CPPFLAGS="$old_cppflags" LIBS="$old_libs" if test "x$with_yajl" = "xyes" ; then - AC_DEFINE_UNQUOTED([HAVE_YAJL], 1, + AC_DEFINE_UNQUOTED([WITH_YAJL], 1, [whether YAJL is available for JSON parsing/formatting]) fi if test "x$with_yajl2" = "xyes" ; then - AC_DEFINE_UNQUOTED([HAVE_YAJL2], 1, + AC_DEFINE_UNQUOTED([WITH_YAJL2], 1, [whether YAJL has API version 2]) fi fi -AM_CONDITIONAL([HAVE_YAJL], [test "x$with_yajl" = "xyes"]) +AM_CONDITIONAL([WITH_YAJL], [test "x$with_yajl" = "xyes"]) AC_SUBST([YAJL_CFLAGS]) AC_SUBST([YAJL_LIBS]) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 320d8c8..97c921c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1142,7 +1142,7 @@ qemuCapsComputeCmdFlags(const char *help, * backported for libvirt. The benefits of JSON mode now * outweigh the downside. */ -#if HAVE_YAJL +#if WITH_YAJL if (version >= 13000) { qemuCapsSet(caps, QEMU_CAPS_MONITOR_JSON); } else if (version >= 12000 && diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 06b0d28..33abdfc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1991,7 +1991,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) goto cleanup; } } else { -#if HAVE_YAJL +#if WITH_YAJL if (qemuCapsGet(priv->caps, QEMU_CAPS_MONITOR_JSON)) { if (!qemuCapsGet(priv->caps, QEMU_CAPS_NO_SHUTDOWN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2003,7 +2003,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Reboot is not supported without the JSON monitor")); goto cleanup; -#if HAVE_YAJL +#if WITH_YAJL } #endif } diff --git a/src/util/virjson.c b/src/util/virjson.c index db5eda2..e6a3b1b 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -29,11 +29,11 @@ #include "virlog.h" #include "virutil.h" -#if HAVE_YAJL +#if WITH_YAJL # include <yajl/yajl_gen.h> # include <yajl/yajl_parse.h> -# ifdef HAVE_YAJL2 +# ifdef WITH_YAJL2 # define yajl_size_t size_t # else # define yajl_size_t unsigned int @@ -659,7 +659,7 @@ int virJSONValueObjectIsNull(virJSONValuePtr object, const char *key) } -#if HAVE_YAJL +#if WITH_YAJL static int virJSONParserInsertValue(virJSONParserPtr parser, virJSONValuePtr value) { @@ -937,13 +937,13 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring) yajl_handle hand; virJSONParser parser = { NULL, NULL, 0 }; virJSONValuePtr ret = NULL; -# ifndef HAVE_YAJL2 +# ifndef WITH_YAJL2 yajl_parser_config cfg = { 1, 1 }; # endif VIR_DEBUG("string=%s", jsonstring); -# ifdef HAVE_YAJL2 +# ifdef WITH_YAJL2 hand = yajl_alloc(&parserCallbacks, NULL, &parser); if (hand) { yajl_config(hand, yajl_allow_comments, 1); @@ -1061,13 +1061,13 @@ char *virJSONValueToString(virJSONValuePtr object, const unsigned char *str; char *ret = NULL; yajl_size_t len; -# ifndef HAVE_YAJL2 +# ifndef WITH_YAJL2 yajl_gen_config conf = { pretty ? 1 : 0, pretty ? " " : " "}; # endif VIR_DEBUG("object=%p", object); -# ifdef HAVE_YAJL2 +# ifdef WITH_YAJL2 g = yajl_gen_alloc(NULL); if (g) { yajl_gen_config(g, yajl_gen_beautify, pretty ? 1 : 0); diff --git a/tests/Makefile.am b/tests/Makefile.am index 9c7c6fb..0e5aec4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -146,7 +146,7 @@ if WITH_CIL test_programs += object-locking endif -if HAVE_YAJL +if WITH_YAJL test_programs += jsontest endif diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 252ad3a..bf09288 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -56,7 +56,7 @@ static int testHelpStrParsing(const void *data) &version, &is_kvm, &kvm_version, false) == -1) goto cleanup; -# ifndef HAVE_YAJL +# ifndef WITH_YAJL if (qemuCapsGet(info->flags, QEMU_CAPS_MONITOR_JSON)) qemuCapsSet(flags, QEMU_CAPS_MONITOR_JSON); # endif -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 6 +++--- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- src/util/virjson.c | 14 +++++++------- tests/Makefile.am | 2 +- tests/qemuhelptest.c | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-)
Mechanical rename, to make later patches easier. ACK (could even be applied before 1/9 if desired). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 87 ++------------------------------------------------------- m4/virt-yajl.m4 | 34 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 84 deletions(-) create mode 100644 m4/virt-yajl.m4 diff --git a/configure.ac b/configure.ac index 988d167..ded622b 100644 --- a/configure.ac +++ b/configure.ac @@ -155,6 +155,8 @@ AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS]) LIBVIRT_COMPILE_WARNINGS +LIBVIRT_CHECK_YAJL + AC_MSG_CHECKING([for CPUID instruction]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM( [[ @@ -1140,85 +1142,6 @@ AC_SUBST([SASL_CFLAGS]) AC_SUBST([SASL_LIBS]) -dnl YAJL JSON library http://lloyd.github.com/yajl/ -AC_ARG_WITH([yajl], - AC_HELP_STRING([--with-yajl], [use YAJL for JSON parsing/formatting @<:@default=check@:>@]), - [], - [with_yajl=check]) - -if test "$with_qemu:$with_yajl" = yes:check; then - dnl Some versions of qemu require the use of yajl; try to detect them - dnl here, although we do not require qemu to exist in order to compile. - dnl This check mirrors src/qemu/qemu_capabilities.c - AC_PATH_PROGS([QEMU], [qemu-kvm qemu kvm qemu-system-x86_64], - [], [$PATH:/usr/bin:/usr/libexec]) - if test -x "$QEMU"; then - if `$QEMU -help | grep libvirt` >/dev/null; then - with_yajl=yes - else - [qemu_version_sed='s/.*ersion \([0-9.,]*\).*/\1/'] - qemu_version=`$QEMU -version | sed "$qemu_version_sed"` - case $qemu_version in - [[1-9]].* | 0.15.* ) with_yajl=yes ;; - 0.* | '' ) ;; - *) AC_MSG_ERROR([Unexpected qemu version string]) ;; - esac - fi - fi -fi - -YAJL_CFLAGS= -YAJL_LIBS= -with_yajl2=no -if test "x$with_yajl" != "xno"; then - if test "x$with_yajl" != "xyes" && test "x$with_yajl" != "xcheck"; then - YAJL_CFLAGS="-I$with_yajl/include" - YAJL_LIBS="-L$with_yajl/lib" - fi - fail=0 - old_cppflags="$CPPFLAGS" - old_libs="$LIBS" - CPPFLAGS="$CPPFLAGS $YAJL_CFLAGS" - LIBS="$LIBS $YAJL_LIBS" - AC_CHECK_HEADER([yajl/yajl_common.h],[],[ - if test "x$with_yajl" = "xcheck" ; then - with_yajl=no - else - fail=1 - fi]) - if test "x$with_yajl" != "xno" ; then - AC_CHECK_LIB([yajl], [yajl_parse],[ - YAJL_LIBS="$YAJL_LIBS -lyajl" - with_yajl=yes - AC_CHECK_LIB([yajl], [yajl_tree_parse],[ - with_yajl2=yes - ],[]) - ],[ - if test "x$with_yajl" = "xcheck" ; then - with_yajl=no - else - fail=1 - fi - ]) - fi - test $fail = 1 && - AC_MSG_ERROR([You must install the YAJL development package in order to compile libvirt]) - CPPFLAGS="$old_cppflags" - LIBS="$old_libs" - if test "x$with_yajl" = "xyes" ; then - AC_DEFINE_UNQUOTED([WITH_YAJL], 1, - [whether YAJL is available for JSON parsing/formatting]) - fi - if test "x$with_yajl2" = "xyes" ; then - AC_DEFINE_UNQUOTED([WITH_YAJL2], 1, - [whether YAJL has API version 2]) - fi -fi -AM_CONDITIONAL([WITH_YAJL], [test "x$with_yajl" = "xyes"]) -AC_SUBST([YAJL_CFLAGS]) -AC_SUBST([YAJL_LIBS]) - - dnl SANLOCK https://fedorahosted.org/sanlock/ AC_ARG_WITH([sanlock], AC_HELP_STRING([--with-sanlock], [build Sanlock plugin for lock management @<:@default=check@:>@]), @@ -3183,6 +3106,7 @@ fi AC_MSG_NOTICE([]) AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) +LIBVIRT_RESULT_YAJL AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) AC_MSG_NOTICE([ dlopen: $DLOPEN_LIBS]) if test "$have_curl" = "yes" ; then @@ -3210,11 +3134,6 @@ AC_MSG_NOTICE([ sasl: $SASL_CFLAGS $SASL_LIBS]) else AC_MSG_NOTICE([ sasl: no]) fi -if test "$with_yajl" != "no" ; then -AC_MSG_NOTICE([ yajl: $YAJL_CFLAGS $YAJL_LIBS]) -else -AC_MSG_NOTICE([ yajl: no]) -fi if test "$with_sanlock" != "no" ; then AC_MSG_NOTICE([ sanlock: $SANLOCK_CFLAGS $SANLOCK_LIBS]) else diff --git a/m4/virt-yajl.m4 b/m4/virt-yajl.m4 new file mode 100644 index 0000000..b11c11b --- /dev/null +++ b/m4/virt-yajl.m4 @@ -0,0 +1,34 @@ +dnl The libyajl.so library + +AC_DEFUN([LIBVIRT_CHECK_YAJL],[ + dnl YAJL JSON library http://lloyd.github.com/yajl/ + if test "$with_qemu:$with_yajl" = yes:check; then + dnl Some versions of qemu require the use of yajl; try to detect them + dnl here, although we do not require qemu to exist in order to compile. + dnl This check mirrors src/qemu/qemu_capabilities.c + AC_PATH_PROGS([QEMU], [qemu-kvm qemu kvm qemu-system-x86_64], + [], [$PATH:/usr/bin:/usr/libexec]) + if test -x "$QEMU"; then + if `$QEMU -help | grep libvirt` >/dev/null; then + with_yajl=yes + else + [qemu_version_sed='s/.*ersion \([0-9.,]*\).*/\1/'] + qemu_version=`$QEMU -version | sed "$qemu_version_sed"` + case $qemu_version in + [[1-9]].* | 0.15.* ) with_yajl=yes ;; + 0.* | '' ) ;; + *) AC_MSG_ERROR([Unexpected qemu version string]) ;; + esac + fi + fi + fi + + LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl], + [yajl_parse_complete], [yajl/yajl_common.h], + [YAJL2], [yajl], + [yajl_tree_parse], [yajl/yajl_common.h]) +]) + +AC_DEFUN([LIBVIRT_RESULT_YAJL],[ + LIBVIRT_RESULT_LIB([YAJL]) +]) -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 87 ++------------------------------------------------------- m4/virt-yajl.m4 | 34 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 84 deletions(-) create mode 100644 m4/virt-yajl.m4
ACK once you fix the following problems:
+++ b/m4/virt-yajl.m4 @@ -0,0 +1,34 @@ +dnl The libyajl.so library
Missing a Copyright notice.
+ +AC_DEFUN([LIBVIRT_CHECK_YAJL],[ + dnl YAJL JSON library http://lloyd.github.com/yajl/ + if test "$with_qemu:$with_yajl" = yes:check; then + dnl Some versions of qemu require the use of yajl; try to detect them + dnl here, although we do not require qemu to exist in order to compile. + dnl This check mirrors src/qemu/qemu_capabilities.c
Now that you have moved the comment, you need to update src/qemu/qemu_capabilities.c to point to virt-yajl.m4 instead of configure.ac as the other end of the linked check. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- src/Makefile.am | 10 +++++----- tools/Makefile.am | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index ded622b..0501d5c 100644 --- a/configure.ac +++ b/configure.ac @@ -1189,7 +1189,7 @@ if test "x$with_sanlock" != "xno"; then CPPFLAGS="$old_cppflags" LIBS="$old_libs" if test "x$with_sanlock" = "xyes" ; then - AC_DEFINE_UNQUOTED([HAVE_SANLOCK], 1, + AC_DEFINE_UNQUOTED([WITH_SANLOCK], 1, [whether Sanlock plugin for lock management is available]) AC_CHECK_LIB([sanlock_client], [sanlock_killpath], @@ -1208,7 +1208,7 @@ if test "x$with_sanlock" != "xno"; then fi fi fi -AM_CONDITIONAL([HAVE_SANLOCK], [test "x$with_sanlock" = "xyes"]) +AM_CONDITIONAL([WITH_SANLOCK], [test "x$with_sanlock" = "xyes"]) AC_SUBST([SANLOCK_CFLAGS]) AC_SUBST([SANLOCK_LIBS]) diff --git a/src/Makefile.am b/src/Makefile.am index da571c7..87aa2f2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1353,7 +1353,7 @@ else check-augeas-lxc: endif -if HAVE_SANLOCK +if WITH_SANLOCK test_libvirt_sanlock.aug: locking/test_libvirt_sanlock.aug.in \ locking/qemu-sanlock.conf $(AUG_GENTEST) $(AM_V_GEN)$(AUG_GENTEST) locking/qemu-sanlock.conf $< $@ @@ -1718,7 +1718,7 @@ virtlockd.socket: locking/virtlockd.socket.in $(top_builddir)/config.status mv $@-t $@ -if HAVE_SANLOCK +if WITH_SANLOCK lockdriver_LTLIBRARIES += sanlock.la sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES) sanlock_la_CFLAGS = -I$(top_srcdir)/src/conf $(AM_CFLAGS) @@ -1882,7 +1882,7 @@ endif EXTRA_DIST += $(STORAGE_HELPER_DISK_SOURCES) -if HAVE_SANLOCK +if WITH_SANLOCK libexec_PROGRAMS += libvirt_sanlock_helper libvirt_sanlock_helper_SOURCES = $(LOCK_DRIVER_SANLOCK_HELPER_SOURCES) @@ -1969,7 +1969,7 @@ endif $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/images" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/boot" -if HAVE_SANLOCK +if WITH_SANLOCK $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/sanlock" endif if WITH_QEMU @@ -2023,7 +2023,7 @@ endif rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/images" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/boot" ||: -if HAVE_SANLOCK +if WITH_SANLOCK rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/sanlock" ||: endif if WITH_QEMU diff --git a/tools/Makefile.am b/tools/Makefile.am index 3775914..878a114 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -43,7 +43,7 @@ bin_SCRIPTS = virt-xml-validate virt-pki-validate bin_PROGRAMS = virsh virt-host-validate libexec_SCRIPTS = libvirt-guests.sh -if HAVE_SANLOCK +if WITH_SANLOCK sbin_SCRIPTS = virt-sanlock-cleanup DISTCLEANFILES += virt-sanlock-cleanup endif @@ -53,7 +53,7 @@ dist_man1_MANS = \ virt-pki-validate.1 \ virt-xml-validate.1 \ virsh.1 -if HAVE_SANLOCK +if WITH_SANLOCK dist_man8_MANS = virt-sanlock-cleanup.8 endif -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- src/Makefile.am | 10 +++++----- tools/Makefile.am | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-)
ACK; mechanical and can be applied any time. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 78 ++---------------------------------------------------- m4/virt-sanlock.m4 | 38 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 76 deletions(-) create mode 100644 m4/virt-sanlock.m4 diff --git a/configure.ac b/configure.ac index 0501d5c..33abf5a 100644 --- a/configure.ac +++ b/configure.ac @@ -155,6 +155,7 @@ AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS]) LIBVIRT_COMPILE_WARNINGS +LIBVIRT_CHECK_SANLOCK LIBVIRT_CHECK_YAJL AC_MSG_CHECKING([for CPUID instruction]) @@ -1142,77 +1143,6 @@ AC_SUBST([SASL_CFLAGS]) AC_SUBST([SASL_LIBS]) -dnl SANLOCK https://fedorahosted.org/sanlock/ -AC_ARG_WITH([sanlock], - AC_HELP_STRING([--with-sanlock], [build Sanlock plugin for lock management @<:@default=check@:>@]), - [], - [with_sanlock=check]) - -SANLOCK_CFLAGS= -SANLOCK_LIBS= -if test "x$with_sanlock" != "xno"; then - if test "x$with_sanlock" != "xyes" && test "x$with_sanlock" != "xcheck"; then - SANLOCK_CFLAGS="-I$with_sanlock/include" - SANLOCK_LIBS="-L$with_sanlock/lib" - fi - fail=0 - old_cppflags="$CPPFLAGS" - old_libs="$LIBS" - CPPFLAGS="$CPPFLAGS $SANLOCK_CFLAGS" - LIBS="$LIBS $SANLOCK_LIBS" - AC_CHECK_HEADER([sanlock.h],[],[ - if test "x$with_sanlock" = "xcheck" ; then - with_sanlock=no - else - fail=1 - fi]) - if test "x$with_sanlock" != "xno" ; then - AC_CHECK_LIB([sanlock_client], [sanlock_init],[ - SANLOCK_LIBS="$SANLOCK_LIBS -lsanlock_client" - with_sanlock=yes - ],[ - if test "x$with_sanlock" = "xcheck" ; then - with_sanlock=no - else - fail=1 - fi - ]) - fi - if test $fail = 1; then - AC_MSG_ERROR([You must install the Sanlock development package in order to compile libvirt]) - else - AC_CHECK_DECLS([SANLK_INQ_WAIT], [sanlock_inq_wait=1], [sanlock_inq_wait=0], [[ - #include <stdint.h> - #include <sanlock_admin.h> - ]]) - fi - CPPFLAGS="$old_cppflags" - LIBS="$old_libs" - if test "x$with_sanlock" = "xyes" ; then - AC_DEFINE_UNQUOTED([WITH_SANLOCK], 1, - [whether Sanlock plugin for lock management is available]) - - AC_CHECK_LIB([sanlock_client], [sanlock_killpath], - [sanlock_killpath=yes], [sanlock_killpath=no]) - if test "x$sanlock_killpath" = "xyes" ; then - AC_DEFINE_UNQUOTED([HAVE_SANLOCK_KILLPATH], 1, - [whether Sanlock supports sanlock_killpath]) - fi - - AC_CHECK_LIB([sanlock_client], [sanlock_inq_lockspace], - [sanlock_inq_lockspace=yes], [sanlock_inq_lockspace=no]) - if test "x$sanlock_inq_lockspace" = "xyes" && \ - test $sanlock_inq_wait = 1; then - AC_DEFINE_UNQUOTED([HAVE_SANLOCK_INQ_LOCKSPACE], 1, - [whether sanlock supports sanlock_inq_lockspace]) - fi - fi -fi -AM_CONDITIONAL([WITH_SANLOCK], [test "x$with_sanlock" = "xyes"]) -AC_SUBST([SANLOCK_CFLAGS]) -AC_SUBST([SANLOCK_LIBS]) - - dnl DBus library DBUS_CFLAGS= DBUS_LIBS= @@ -3106,6 +3036,7 @@ fi AC_MSG_NOTICE([]) AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) +LIBVIRT_RESULT_SANLOCK LIBVIRT_RESULT_YAJL AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) AC_MSG_NOTICE([ dlopen: $DLOPEN_LIBS]) @@ -3134,11 +3065,6 @@ AC_MSG_NOTICE([ sasl: $SASL_CFLAGS $SASL_LIBS]) else AC_MSG_NOTICE([ sasl: no]) fi -if test "$with_sanlock" != "no" ; then -AC_MSG_NOTICE([ sanlock: $SANLOCK_CFLAGS $SANLOCK_LIBS]) -else -AC_MSG_NOTICE([ sanlock: no]) -fi AC_MSG_NOTICE([firewalld: $with_firewalld]) if test "$with_avahi" = "yes" ; then AC_MSG_NOTICE([ avahi: $AVAHI_CFLAGS $AVAHI_LIBS]) diff --git a/m4/virt-sanlock.m4 b/m4/virt-sanlock.m4 new file mode 100644 index 0000000..58ea0c5 --- /dev/null +++ b/m4/virt-sanlock.m4 @@ -0,0 +1,38 @@ +dnl The libsanlock_client.so library + +AC_DEFUN([LIBVIRT_CHECK_SANLOCK],[ + LIBVIRT_CHECK_LIB([SANLOCK], [sanlock_client], [sanlock_init], [sanlock.h]) + + AC_CHECK_DECLS([SANLK_INQ_WAIT], [sanlock_inq_wait=1], [sanlock_inq_wait=0], [[ + #include <stdint.h> + #include <sanlock_admin.h> + ]]) + + + old_cppflags="$CPPFLAGS" + old_libs="$LIBS" + CPPFLAGS="$CPPFLAGS $SANLOCK_CFLAGS" + LIBS="$LIBS $SANLOCK_LIBS" + + AC_CHECK_LIB([sanlock_client], [sanlock_killpath], + [sanlock_killpath=yes], [sanlock_killpath=no]) + if test "x$sanlock_killpath" = "xyes" ; then + AC_DEFINE_UNQUOTED([HAVE_SANLOCK_KILLPATH], 1, + [whether Sanlock supports sanlock_killpath]) + fi + + AC_CHECK_LIB([sanlock_client], [sanlock_inq_lockspace], + [sanlock_inq_lockspace=yes], [sanlock_inq_lockspace=no]) + if test "x$sanlock_inq_lockspace" = "xyes" && \ + test $sanlock_inq_wait = 1; then + AC_DEFINE_UNQUOTED([HAVE_SANLOCK_INQ_LOCKSPACE], 1, + [whether sanlock supports sanlock_inq_lockspace]) + fi + + CPPFLAGS="$old_cppflags" + LIBS="$old_libs" +]) + +AC_DEFUN([LIBVIRT_RESULT_SANLOCK],[ + LIBVIRT_RESULT_LIB([SANLOCK]) +]) -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 78 ++---------------------------------------------------- m4/virt-sanlock.m4 | 38 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 76 deletions(-) create mode 100644 m4/virt-sanlock.m4
+++ b/m4/virt-sanlock.m4 @@ -0,0 +1,38 @@ +dnl The libsanlock_client.so library + +AC_DEFUN([LIBVIRT_CHECK_SANLOCK],[ + LIBVIRT_CHECK_LIB([SANLOCK], [sanlock_client], [sanlock_init], [sanlock.h]) +
It might be worth making the rest of this macro conditional: if test "x$with_sanlock" != "xno" ; then
+ AC_CHECK_DECLS([SANLK_INQ_WAIT], [sanlock_inq_wait=1], [sanlock_inq_wait=0], [[ + #include <stdint.h> + #include <sanlock_admin.h> + ]]) + + + old_cppflags="$CPPFLAGS" + old_libs="$LIBS" + CPPFLAGS="$CPPFLAGS $SANLOCK_CFLAGS" + LIBS="$LIBS $SANLOCK_LIBS" + + AC_CHECK_LIB([sanlock_client], [sanlock_killpath], + [sanlock_killpath=yes], [sanlock_killpath=no])
That is, no need to waste time checking for a library function if we already know we aren't going to use the library.
+ if test "x$sanlock_killpath" = "xyes" ; then + AC_DEFINE_UNQUOTED([HAVE_SANLOCK_KILLPATH], 1, + [whether Sanlock supports sanlock_killpath]) + fi + + AC_CHECK_LIB([sanlock_client], [sanlock_inq_lockspace], + [sanlock_inq_lockspace=yes], [sanlock_inq_lockspace=no]) + if test "x$sanlock_inq_lockspace" = "xyes" && \ + test $sanlock_inq_wait = 1; then + AC_DEFINE_UNQUOTED([HAVE_SANLOCK_INQ_LOCKSPACE], 1, + [whether sanlock supports sanlock_inq_lockspace]) + fi + + CPPFLAGS="$old_cppflags" + LIBS="$old_libs"
fi
+]) + +AC_DEFUN([LIBVIRT_RESULT_SANLOCK],[ + LIBVIRT_RESULT_LIB([SANLOCK]) +])
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- daemon/Makefile.am | 2 +- daemon/libvirtd-config.c | 4 ++-- daemon/libvirtd.c | 4 ++-- daemon/libvirtd.h | 6 +++--- daemon/remote.c | 2 +- src/Makefile.am | 4 ++-- src/remote/remote_driver.c | 8 ++++---- src/rpc/virnetclient.c | 10 +++++----- src/rpc/virnetclient.h | 4 ++-- src/rpc/virnetserverclient.c | 13 ++++++------- src/rpc/virnetserverclient.h | 2 +- src/rpc/virnetsocket.c | 16 ++++++++-------- src/rpc/virnetsocket.h | 4 ++-- tests/libvirtdconftest.c | 2 +- 15 files changed, 42 insertions(+), 43 deletions(-) diff --git a/configure.ac b/configure.ac index 33abf5a..1a28e98 100644 --- a/configure.ac +++ b/configure.ac @@ -1134,11 +1134,11 @@ if test "x$with_sasl" != "xno"; then CFLAGS="$old_cflags" LIBS="$old_libs" if test "x$with_sasl" = "xyes" ; then - AC_DEFINE_UNQUOTED([HAVE_SASL], 1, + AC_DEFINE_UNQUOTED([WITH_SASL], 1, [whether Cyrus SASL is available for authentication]) fi fi -AM_CONDITIONAL([HAVE_SASL], [test "x$with_sasl" = "xyes"]) +AM_CONDITIONAL([WITH_SASL], [test "x$with_sasl" = "xyes"]) AC_SUBST([SASL_CFLAGS]) AC_SUBST([SASL_LIBS]) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index c59084c..e14b3c1 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -390,7 +390,7 @@ $(srcdir)/libvirtd.8.in: libvirtd.pod.in # This is needed for clients too, so can't wrap in # the WITH_LIBVIRTD conditional -if HAVE_SASL +if WITH_SASL install-data-sasl: $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sasl2/ $(INSTALL_DATA) $(srcdir)/libvirtd.sasl $(DESTDIR)$(sysconfdir)/sasl2/libvirt.conf diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index 8b74bf1..fe9fc10 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -178,7 +178,7 @@ static int remoteConfigGetAuth(virConfPtr conf, const char *key, int *auth, cons if (STREQ(p->str, "none")) { *auth = VIR_NET_SERVER_SERVICE_AUTH_NONE; -#if HAVE_SASL +#if WITH_SASL } else if (STREQ(p->str, "sasl")) { *auth = VIR_NET_SERVER_SERVICE_AUTH_SASL; #endif @@ -263,7 +263,7 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED) !data->unix_sock_rw_perms) goto no_memory; -#if HAVE_SASL +#if WITH_SASL data->auth_tcp = REMOTE_AUTH_SASL; #else data->auth_tcp = REMOTE_AUTH_NONE; diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index ff54af3..7a51387 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -98,7 +98,7 @@ #include "virdbus.h" -#if HAVE_SASL +#if WITH_SASL virNetSASLContextPtr saslCtxt = NULL; #endif virNetServerProgramPtr remoteProgram = NULL; @@ -578,7 +578,7 @@ static int daemonSetupNetworking(virNetServerPtr srv, #endif } -#if HAVE_SASL +#if WITH_SASL if (config->auth_unix_rw == REMOTE_AUTH_SASL || config->auth_unix_ro == REMOTE_AUTH_SASL || # if HAVE_GNUTLS diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 69a77ea..6b5ceb7 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -35,7 +35,7 @@ # include "qemu_protocol.h" # include "virlog.h" # include "virthread.h" -# if HAVE_SASL +# if WITH_SASL # include "virnetsaslcontext.h" # endif # include "virnetserverprogram.h" @@ -52,7 +52,7 @@ struct daemonClientPrivate { int domainEventCallbackID[VIR_DOMAIN_EVENT_ID_LAST]; -# if HAVE_SASL +# if WITH_SASL virNetSASLSessionPtr sasl; # endif @@ -66,7 +66,7 @@ struct daemonClientPrivate { bool keepalive_supported; }; -# if HAVE_SASL +# if WITH_SASL extern virNetSASLContextPtr saslCtxt; # endif extern virNetServerProgramPtr remoteProgram; diff --git a/daemon/remote.c b/daemon/remote.c index 67fe335..1a0af7e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2432,7 +2432,7 @@ cleanup: } -#ifdef HAVE_SASL +#ifdef WITH_SASL /* * Initializes the SASL session in prepare for authentication * and gives the client a list of allowed mechanisms to choose diff --git a/src/Makefile.am b/src/Makefile.am index 87aa2f2..bc288bd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1422,7 +1422,7 @@ if WITH_XENXS USED_SYM_FILES += $(srcdir)/libvirt_xenxs.syms endif -if HAVE_SASL +if WITH_SASL USED_SYM_FILES += $(srcdir)/libvirt_sasl.syms endif @@ -1787,7 +1787,7 @@ else EXTRA_DIST += \ rpc/virnettlscontext.h rpc/virnettlscontext.c endif -if HAVE_SASL +if WITH_SASL libvirt_net_rpc_la_SOURCES += \ rpc/virnetsaslcontext.h rpc/virnetsaslcontext.c else diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 43c1186..862e474 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -118,7 +118,7 @@ static int callWithFD(virConnectPtr conn, struct private_data *priv, xdrproc_t ret_filter, char *ret); static int remoteAuthenticate(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth, const char *authtype); -#if HAVE_SASL +#if WITH_SASL static int remoteAuthSASL(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth, const char *mech); #endif @@ -3489,7 +3489,7 @@ remoteAuthenticate(virConnectPtr conn, struct private_data *priv, } switch (type) { -#if HAVE_SASL +#if WITH_SASL case REMOTE_AUTH_SASL: { const char *mech = NULL; if (authtype && @@ -3532,7 +3532,7 @@ remoteAuthenticate(virConnectPtr conn, struct private_data *priv, -#if HAVE_SASL +#if WITH_SASL static int remoteAuthCredVir2SASL(int vircred) { switch (vircred) { @@ -4072,7 +4072,7 @@ remoteAuthSASL(virConnectPtr conn, struct private_data *priv, return ret; } -#endif /* HAVE_SASL */ +#endif /* WITH_SASL */ #if HAVE_POLKIT diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index a79b79b..10ed4f0 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -81,7 +81,7 @@ struct _virNetClient { /* For incoming message packets */ virNetMessage msg; -#if HAVE_SASL +#if WITH_SASL virNetSASLSessionPtr sasl; #endif @@ -632,7 +632,7 @@ void virNetClientDispose(void *obj) #if HAVE_GNUTLS virObjectUnref(client->tls); #endif -#if HAVE_SASL +#if WITH_SASL virObjectUnref(client->sasl); #endif @@ -671,7 +671,7 @@ virNetClientCloseLocked(virNetClientPtr client) virObjectUnref(client->tls); client->tls = NULL; #endif -#if HAVE_SASL +#if WITH_SASL virObjectUnref(client->sasl); client->sasl = NULL; #endif @@ -739,7 +739,7 @@ void virNetClientClose(virNetClientPtr client) } -#if HAVE_SASL +#if WITH_SASL void virNetClientSetSASLSession(virNetClientPtr client, virNetSASLSessionPtr sasl) { @@ -864,7 +864,7 @@ bool virNetClientIsEncrypted(virNetClientPtr client) if (client->tls) ret = true; #endif -#if HAVE_SASL +#if WITH_SASL if (client->sasl) ret = true; #endif diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index d594add..7e35b9c 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -27,7 +27,7 @@ # include "virnettlscontext.h" # endif # include "virnetmessage.h" -# ifdef HAVE_SASL +# ifdef WITH_SASL # include "virnetsaslcontext.h" # endif # include "virnetclientprogram.h" @@ -104,7 +104,7 @@ int virNetClientSendWithReplyStream(virNetClientPtr client, virNetMessagePtr msg, virNetClientStreamPtr st); -# ifdef HAVE_SASL +# ifdef WITH_SASL void virNetClientSetSASLSession(virNetClientPtr client, virNetSASLSessionPtr sasl); # endif diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index bf23d24..05e0e06 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -23,7 +23,7 @@ #include <config.h> -#if HAVE_SASL +#if WITH_SASL # include <sasl/sasl.h> #endif @@ -70,7 +70,7 @@ struct _virNetServerClient virNetTLSContextPtr tlsCtxt; virNetTLSSessionPtr tls; #endif -#if HAVE_SASL +#if WITH_SASL virNetSASLSessionPtr sasl; #endif int sockTimer; /* Timer to be fired upon cached data, @@ -652,7 +652,7 @@ bool virNetServerClientIsSecure(virNetServerClientPtr client) if (client->tls) secure = true; #endif -#if HAVE_SASL +#if WITH_SASL if (client->sasl) secure = true; #endif @@ -663,8 +663,7 @@ bool virNetServerClientIsSecure(virNetServerClientPtr client) } - -#if HAVE_SASL +#if WITH_SASL void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionPtr sasl) { @@ -761,7 +760,7 @@ void virNetServerClientDispose(void *obj) client->privateDataFreeFunc(client->privateData); VIR_FREE(client->identity); -#if HAVE_SASL +#if WITH_SASL virObjectUnref(client->sasl); #endif if (client->sockTimer > 0) @@ -1181,7 +1180,7 @@ virNetServerClientDispatchWrite(virNetServerClientPtr client) client->tx->donefds++; } -#if HAVE_SASL +#if WITH_SASL /* Completed this 'tx' operation, so now read for all * future rx/tx to be under a SASL SSF layer */ diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index b11b9a9..40010c8 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -83,7 +83,7 @@ bool virNetServerClientHasTLSSession(virNetServerClientPtr client); int virNetServerClientGetTLSKeySize(virNetServerClientPtr client); # endif -# ifdef HAVE_SASL +# ifdef WITH_SASL void virNetServerClientSetSASLSession(virNetServerClientPtr client, virNetSASLSessionPtr sasl); # endif diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index a817999..549aabe 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -82,7 +82,7 @@ struct _virNetSocket { #if HAVE_GNUTLS virNetTLSSessionPtr tlsSession; #endif -#if HAVE_SASL +#if WITH_SASL virNetSASLSessionPtr saslSession; const char *saslDecoded; @@ -1021,7 +1021,7 @@ void virNetSocketDispose(void *obj) virNetTLSSessionSetIOCallbacks(sock->tlsSession, NULL, NULL, NULL); virObjectUnref(sock->tlsSession); #endif -#if HAVE_SASL +#if WITH_SASL virObjectUnref(sock->saslSession); #endif @@ -1217,7 +1217,7 @@ void virNetSocketSetTLSSession(virNetSocketPtr sock, } #endif -#if HAVE_SASL +#if WITH_SASL void virNetSocketSetSASLSession(virNetSocketPtr sock, virNetSASLSessionPtr sess) { @@ -1239,7 +1239,7 @@ bool virNetSocketHasCachedData(virNetSocketPtr sock ATTRIBUTE_UNUSED) hasCached = true; #endif -#if HAVE_SASL +#if WITH_SASL if (sock->saslDecoded) hasCached = true; #endif @@ -1267,7 +1267,7 @@ bool virNetSocketHasPendingData(virNetSocketPtr sock ATTRIBUTE_UNUSED) { bool hasPending = false; virMutexLock(&sock->lock); -#if HAVE_SASL +#if WITH_SASL if (sock->saslEncoded) hasPending = true; #endif @@ -1378,7 +1378,7 @@ rewrite: } -#if HAVE_SASL +#if WITH_SASL static ssize_t virNetSocketReadSASL(virNetSocketPtr sock, char *buf, size_t len) { ssize_t got; @@ -1481,7 +1481,7 @@ ssize_t virNetSocketRead(virNetSocketPtr sock, char *buf, size_t len) { ssize_t ret; virMutexLock(&sock->lock); -#if HAVE_SASL +#if WITH_SASL if (sock->saslSession) ret = virNetSocketReadSASL(sock, buf, len); else @@ -1496,7 +1496,7 @@ ssize_t virNetSocketWrite(virNetSocketPtr sock, const char *buf, size_t len) ssize_t ret; virMutexLock(&sock->lock); -#if HAVE_SASL +#if WITH_SASL if (sock->saslSession) ret = virNetSocketWriteSASL(sock, buf, len); else diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index ce15bb8..f1cb2ab 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -30,7 +30,7 @@ # include "virnettlscontext.h" # endif # include "virobject.h" -# ifdef HAVE_SASL +# ifdef WITH_SASL # include "virnetsaslcontext.h" # endif # include "virjson.h" @@ -129,7 +129,7 @@ void virNetSocketSetTLSSession(virNetSocketPtr sock, virNetTLSSessionPtr sess); # endif -# ifdef HAVE_SASL +# ifdef WITH_SASL void virNetSocketSetSASLSession(virNetSocketPtr sock, virNetSASLSessionPtr sess); # endif diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c index fd033f8..01e9415 100644 --- a/tests/libvirtdconftest.c +++ b/tests/libvirtdconftest.c @@ -120,7 +120,7 @@ testCorrupt(const void *opaque) goto cleanup; } -#if !HAVE_SASL +#if !WITH_SASL if (strstr(err->message, "unsupported auth sasl")) { VIR_DEBUG("sasl unsupported, skipping this config"); goto cleanup; -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- daemon/Makefile.am | 2 +- daemon/libvirtd-config.c | 4 ++-- daemon/libvirtd.c | 4 ++-- daemon/libvirtd.h | 6 +++--- daemon/remote.c | 2 +- src/Makefile.am | 4 ++-- src/remote/remote_driver.c | 8 ++++---- src/rpc/virnetclient.c | 10 +++++----- src/rpc/virnetclient.h | 4 ++-- src/rpc/virnetserverclient.c | 13 ++++++------- src/rpc/virnetserverclient.h | 2 +- src/rpc/virnetsocket.c | 16 ++++++++-------- src/rpc/virnetsocket.h | 4 ++-- tests/libvirtdconftest.c | 2 +- 15 files changed, 42 insertions(+), 43 deletions(-)
ACK; mechanical, and can go in now. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 62 ++------------------------------------------------------- m4/virt-sasl.m4 | 12 +++++++++++ 2 files changed, 14 insertions(+), 60 deletions(-) create mode 100644 m4/virt-sasl.m4 diff --git a/configure.ac b/configure.ac index 1a28e98..0ce91f3 100644 --- a/configure.ac +++ b/configure.ac @@ -156,6 +156,7 @@ AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS]) LIBVIRT_COMPILE_WARNINGS LIBVIRT_CHECK_SANLOCK +LIBVIRT_CHECK_SASL LIBVIRT_CHECK_YAJL AC_MSG_CHECKING([for CPUID instruction]) @@ -1088,61 +1089,6 @@ AC_SUBST([GNUTLS_CFLAGS]) AC_SUBST([GNUTLS_LIBS]) -dnl Cyrus SASL -AC_ARG_WITH([sasl], - AC_HELP_STRING([--with-sasl], [use cyrus SASL for authentication @<:@default=check@:>@]), - [], - [with_sasl=check]) - -SASL_CFLAGS= -SASL_LIBS= -if test "x$with_sasl" != "xno"; then - if test "x$with_sasl" != "xyes" && test "x$with_sasl" != "xcheck"; then - SASL_CFLAGS="-I$with_sasl" - SASL_LIBS="-L$with_sasl" - fi - fail=0 - old_cflags="$CFLAGS" - old_libs="$LIBS" - CFLAGS="$CFLAGS $SASL_CFLAGS" - LIBS="$LIBS $SASL_LIBS" - AC_CHECK_HEADER([sasl/sasl.h],[],[ - if test "x$with_sasl" = "xcheck" ; then - with_sasl=no - else - fail=1 - fi]) - if test "x$with_sasl" != "xno" ; then - AC_CHECK_LIB([sasl2], [sasl_client_init],[ - SASL_LIBS="$SASL_LIBS -lsasl2" - with_sasl=yes - ],[ - AC_CHECK_LIB([sasl], [sasl_client_init],[ - SASL_LIBS="$SASL_LIBS -lsasl" - with_sasl=yes - ],[ - if test "x$with_sasl" = "xcheck" ; then - with_sasl=no - else - fail=1 - fi - ]) - ]) - fi - test $fail = 1 && - AC_MSG_ERROR([You must install the Cyrus SASL development package in order to compile libvirt]) - CFLAGS="$old_cflags" - LIBS="$old_libs" - if test "x$with_sasl" = "xyes" ; then - AC_DEFINE_UNQUOTED([WITH_SASL], 1, - [whether Cyrus SASL is available for authentication]) - fi -fi -AM_CONDITIONAL([WITH_SASL], [test "x$with_sasl" = "xyes"]) -AC_SUBST([SASL_CFLAGS]) -AC_SUBST([SASL_LIBS]) - - dnl DBus library DBUS_CFLAGS= DBUS_LIBS= @@ -3037,6 +2983,7 @@ AC_MSG_NOTICE([]) AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) LIBVIRT_RESULT_SANLOCK +LIBVIRT_RESULT_SASL LIBVIRT_RESULT_YAJL AC_MSG_NOTICE([ libxml: $LIBXML_CFLAGS $LIBXML_LIBS]) AC_MSG_NOTICE([ dlopen: $DLOPEN_LIBS]) @@ -3060,11 +3007,6 @@ AC_MSG_NOTICE([ gnutls: $GNUTLS_CFLAGS $GNUTLS_LIBS]) else AC_MSG_NOTICE([ gnutls: no]) fi -if test "$with_sasl" != "no" ; then -AC_MSG_NOTICE([ sasl: $SASL_CFLAGS $SASL_LIBS]) -else -AC_MSG_NOTICE([ sasl: no]) -fi AC_MSG_NOTICE([firewalld: $with_firewalld]) if test "$with_avahi" = "yes" ; then AC_MSG_NOTICE([ avahi: $AVAHI_CFLAGS $AVAHI_LIBS]) diff --git a/m4/virt-sasl.m4 b/m4/virt-sasl.m4 new file mode 100644 index 0000000..cf5fe2d --- /dev/null +++ b/m4/virt-sasl.m4 @@ -0,0 +1,12 @@ +dnl The libsasl2.so or libsasl.so library + +AC_DEFUN([LIBVIRT_CHECK_SASL],[ + LIBVIRT_CHECK_LIB_ALT([SASL], [sasl2], + [sasl_client_init], [sasl/sasl.h], + [SASL1], [sasl], + [sasl_client_init], [sasl/sasl.h]) +]) + +AC_DEFUN([LIBVIRT_RESULT_SASL],[ + LIBVIRT_RESULT_LIB([SASL]) +]) -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 62 ++------------------------------------------------------- m4/virt-sasl.m4 | 12 +++++++++++ 2 files changed, 14 insertions(+), 60 deletions(-) create mode 100644 m4/virt-sasl.m4
+++ b/m4/virt-sasl.m4 @@ -0,0 +1,12 @@ +dnl The libsasl2.so or libsasl.so library
No copyright. Probably a recurring theme in your series.
+ +AC_DEFUN([LIBVIRT_CHECK_SASL],[ + LIBVIRT_CHECK_LIB_ALT([SASL], [sasl2], + [sasl_client_init], [sasl/sasl.h], + [SASL1], [sasl], + [sasl_client_init], [sasl/sasl.h]) +]) + +AC_DEFUN([LIBVIRT_RESULT_SASL],[ + LIBVIRT_RESULT_LIB([SASL]) +])
ACK with that fixed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- src/util/viraudit.c | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 0ce91f3..5a17097 100644 --- a/configure.ac +++ b/configure.ac @@ -1258,13 +1258,13 @@ if test "$with_audit" != "no" ; then if test "$with_audit" = "yes" ; then AUDIT_LIBS="$AUDIT_LIBS -laudit" - AC_DEFINE_UNQUOTED([HAVE_AUDIT], 1, [whether libaudit is available]) + AC_DEFINE_UNQUOTED([WITH_AUDIT], 1, [whether libaudit is available]) fi CFLAGS="$old_cflags" LIBS="$old_libs" fi -AM_CONDITIONAL([HAVE_AUDIT], [test "$with_audit" = "yes"]) +AM_CONDITIONAL([WITH_AUDIT], [test "$with_audit" = "yes"]) AC_SUBST([AUDIT_CFLAGS]) AC_SUBST([AUDIT_LIBS]) diff --git a/src/util/viraudit.c b/src/util/viraudit.c index 04ac323..0f4bb4f 100644 --- a/src/util/viraudit.c +++ b/src/util/viraudit.c @@ -21,7 +21,7 @@ #include <config.h> -#ifdef HAVE_AUDIT +#ifdef WITH_AUDIT # include <libaudit.h> #endif #include <stdio.h> @@ -48,14 +48,14 @@ #define VIR_FROM_THIS VIR_FROM_AUDIT -#if HAVE_AUDIT +#if WITH_AUDIT static int auditfd = -1; #endif static int auditlog = 0; int virAuditOpen(void) { -#if HAVE_AUDIT +#if WITH_AUDIT if ((auditfd = audit_open()) < 0) { virReportSystemError(errno, "%s", _("Unable to initialize audit layer")); return -1; @@ -87,7 +87,7 @@ void virAuditSend(const char *filename, /* Duplicate later checks, to short circuit & avoid printf overhead * when nothing is enabled */ -#if HAVE_AUDIT +#if WITH_AUDIT if (!auditlog && auditfd < 0) return; #else @@ -113,7 +113,7 @@ void virAuditSend(const char *filename, NULL, "success=no %s", str); } -#if HAVE_AUDIT +#if WITH_AUDIT if (auditfd < 0) { VIR_FREE(str); return; @@ -141,14 +141,14 @@ void virAuditSend(const char *filename, void virAuditClose(void) { -#if HAVE_AUDIT +#if WITH_AUDIT VIR_FORCE_CLOSE(auditfd); #endif } char *virAuditEncode(const char *key, const char *value) { -#if HAVE_AUDIT +#if WITH_AUDIT return audit_encode_nv_string(key, value, 0); #else char *str; -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 4 ++-- src/util/viraudit.c | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 51 ++------------------------------------------------- m4/virt-audit.m4 | 10 ++++++++++ 2 files changed, 12 insertions(+), 49 deletions(-) create mode 100644 m4/virt-audit.m4 diff --git a/configure.ac b/configure.ac index 5a17097..d5b53d3 100644 --- a/configure.ac +++ b/configure.ac @@ -155,6 +155,7 @@ AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS]) LIBVIRT_COMPILE_WARNINGS +LIBVIRT_CHECK_AUDIT LIBVIRT_CHECK_SANLOCK LIBVIRT_CHECK_SASL LIBVIRT_CHECK_YAJL @@ -1223,50 +1224,6 @@ fi AC_SUBST([AVAHI_CFLAGS]) AC_SUBST([AVAHI_LIBS]) -dnl Audit library -AC_ARG_WITH([audit], - AC_HELP_STRING([--with-audit], [use audit library @<:@default=check@:>@]), - [], - [with_audit=check]) - -AUDIT_CFLAGS= -AUDIT_LIBS= -if test "$with_audit" != "no" ; then - old_cflags="$CFLAGS" - old_libs="$LIBS" - if test "$with_audit" != "check" && test "$with_audit" != "yes" ; then - AUDIT_CFLAGS="-I$with_audit/include" - AUDIT_LIBS="-L$with_audit/lib" - fi - CFLAGS="$CFLAGS $AUDIT_CFLAGS" - LIBS="$LIBS $AUDIT_LIBS" - fail=0 - AC_CHECK_HEADER([libaudit.h], [], [fail=1]) - AC_CHECK_LIB([audit], [audit_encode_nv_string], [], [fail=1]) - - if test $fail = 1 ; then - if test "$with_audit" = "yes" ; then - AC_MSG_ERROR([You must install the Audit library in order to compile and run libvirt]) - else - with_audit=no - AUDIT_CFLAGS= - AUDIT_LIBS= - fi - else - with_audit=yes - fi - - if test "$with_audit" = "yes" ; then - AUDIT_LIBS="$AUDIT_LIBS -laudit" - AC_DEFINE_UNQUOTED([WITH_AUDIT], 1, [whether libaudit is available]) - fi - - CFLAGS="$old_cflags" - LIBS="$old_libs" -fi -AM_CONDITIONAL([WITH_AUDIT], [test "$with_audit" = "yes"]) -AC_SUBST([AUDIT_CFLAGS]) -AC_SUBST([AUDIT_LIBS]) dnl UUCP style file locks for character devices if test "$with_chrdev_lock_files" != "no"; then @@ -2982,6 +2939,7 @@ fi AC_MSG_NOTICE([]) AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) +LIBVIRT_RESULT_AUDIT LIBVIRT_RESULT_SANLOCK LIBVIRT_RESULT_SASL LIBVIRT_RESULT_YAJL @@ -3022,11 +2980,6 @@ fi else AC_MSG_NOTICE([ polkit: no]) fi -if test "$with_audit" = "yes" ; then -AC_MSG_NOTICE([ audit: $AUDIT_CFLAGS $AUDIT_LIBS]) -else -AC_MSG_NOTICE([ audit: no]) -fi if test "$with_selinux" = "yes" ; then AC_MSG_NOTICE([ selinux: $SELINUX_CFLAGS $SELINUX_LIBS]) else diff --git a/m4/virt-audit.m4 b/m4/virt-audit.m4 new file mode 100644 index 0000000..9fb7335 --- /dev/null +++ b/m4/virt-audit.m4 @@ -0,0 +1,10 @@ +dnl The libaudit.so library + +AC_DEFUN([LIBVIRT_CHECK_AUDIT],[ + LIBVIRT_CHECK_LIB([AUDIT], [audit], + [audit_encode_nv_string], [libaudit.h]) +]) + +AC_DEFUN([LIBVIRT_RESULT_AUDIT],[ + LIBVIRT_RESULT_LIB([AUDIT]) +]) -- 1.7.11.7
On 01/10/2013 01:18 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 51 ++------------------------------------------------- m4/virt-audit.m4 | 10 ++++++++++ 2 files changed, 12 insertions(+), 49 deletions(-) create mode 100644 m4/virt-audit.m4
+++ b/m4/virt-audit.m4 @@ -0,0 +1,10 @@ +dnl The libaudit.so library +
ACK with copyright added. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake