[libvirt] [PATCH v2 0/3] qemu: Explicitly check for gnutls_rnd()
Patch 1 fixes a bug in configure. Patch 2 performs a minor cleanup. Patch 3 is the fix for the build issues currently experienced on CentOS 6 (see [1]). Changes from v1: * update CFLAGS and LIBS before performing the check, so that the compiler can actually find the function Cheers. [1] https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=... Andrea Bolognani (3): configure: Restore CFLAGS properly after GnuTLS checks configure: Always use old_CFLAGS and old_LIBS qemu: Explicitly check for gnutls_rnd() configure.ac | 31 +++++++++++++++++++------------ src/qemu/qemu_domain.c | 6 +++--- 2 files changed, 22 insertions(+), 15 deletions(-) -- 2.5.5
The previous value of CFLAGS was saved as old_cflags but later restored from old_CFLAGS, which is clearly not correct. Restore CFLAGS from the right variable. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 85fc6e1..6088f77 100644 --- a/configure.ac +++ b/configure.ac @@ -1289,8 +1289,8 @@ if test "x$with_gnutls" != "xno"; then with_gnutls=yes fi + CFLAGS="$old_cflags" LIBS="$old_libs" - CFLAGS="$old_CFLAGS" fi if test "x$with_gnutls" = "xyes" ; then -- 2.5.5
The variables used for storing CFLAGS and LIBS before temporarily modifying them was consistent when it comes to the name, but not when it comes to the case. Make sure names are completely consistent. --- configure.ac | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 6088f77..6442674 100644 --- a/configure.ac +++ b/configure.ac @@ -321,7 +321,7 @@ if test "x$lv_cv_pthread_sigmask_works" != xyes; then AC_DEFINE([FUNC_PTHREAD_SIGMASK_BROKEN], [1], [Define to 1 if pthread_sigmask is not a real function]) fi -LIBS=$old_libs +LIBS=$old_LIBS dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h regex.h sys/un.h \ @@ -1198,15 +1198,15 @@ AC_SUBST([LIBXML_CFLAGS]) AC_SUBST([LIBXML_LIBS]) dnl xmlURI structure has query_raw? -old_cflags="$CFLAGS" -old_libs="$LIBS" +old_CFLAGS="$CFLAGS" +old_LIBS="$LIBS" CFLAGS="$CFLAGS $LIBXML_CFLAGS" LIBS="$LIBS $LIBXML_LIBS" AC_CHECK_MEMBER([struct _xmlURI.query_raw], [AC_DEFINE([HAVE_XMLURI_QUERY_RAW], [], [Have query_raw field in libxml2 xmlURI structure])],, [#include <libxml/uri.h>]) -CFLAGS="$old_cflags" -LIBS="$old_libs" +CFLAGS="$old_CFLAGS" +LIBS="$old_LIBS" dnl GnuTLS library AC_ARG_WITH([gnutls], @@ -1222,8 +1222,8 @@ if test "x$with_gnutls" != "xno"; then GNUTLS_LIBS="-L$with_gnutls/lib" fi fail=0 - old_cflags="$CFLAGS" - old_libs="$LIBS" + old_CFLAGS="$CFLAGS" + old_LIBS="$LIBS" CFLAGS="$CFLAGS $GNUTLS_CFLAGS" LIBS="$LIBS $GNUTLS_LIBS" @@ -1289,8 +1289,8 @@ if test "x$with_gnutls" != "xno"; then with_gnutls=yes fi - CFLAGS="$old_cflags" - LIBS="$old_libs" + CFLAGS="$old_CFLAGS" + LIBS="$old_LIBS" fi if test "x$with_gnutls" = "xyes" ; then @@ -1431,8 +1431,8 @@ if test "$with_selinux" != "yes" ; then AC_MSG_ERROR([You must install the libselinux development package and enable SELinux with the --with-selinux=yes in order to compile libvirt --with-secdriver-selinux=yes]) fi elif test "$with_secdriver_selinux" != "no"; then - old_cflags="$CFLAGS" - old_libs="$LIBS" + old_CFLAGS="$CFLAGS" + old_LIBS="$LIBS" CFLAGS="$CFLAGS $SELINUX_CFLAGS" LIBS="$CFLAGS $SELINUX_LIBS" @@ -1440,8 +1440,8 @@ elif test "$with_secdriver_selinux" != "no"; then AC_CHECK_FUNC([selinux_virtual_domain_context_path], [], [fail=1]) AC_CHECK_FUNC([selinux_virtual_image_context_path], [], [fail=1]) AC_CHECK_FUNCS([selinux_lxc_contexts_path]) - CFLAGS="$old_cflags" - LIBS="$old_libs" + CFLAGS="$old_CFLAGS" + LIBS="$old_LIBS" if test "$fail" = "1" ; then if test "$with_secdriver_selinux" = "check" ; then -- 2.5.5
Our use of gnutls_rnd(), introduced with commit ad7520e8, is conditional to the availability of the <gnutls/crypto.h> header file. Such check, however, turns out not to be strict enough, as there are some versions of GnuTLS (eg. 2.8.5 from CentOS 6) that provide the header file, but not the function itself, which was introduced only in GnuTLS 2.12.0. Introduce an explicit check for the function. --- configure.ac | 7 +++++++ src/qemu/qemu_domain.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 6442674..c8c2895 100644 --- a/configure.ac +++ b/configure.ac @@ -1289,6 +1289,13 @@ if test "x$with_gnutls" != "xno"; then with_gnutls=yes fi + dnl GNUTLS_CFLAGS and GNUTLS_LIBS have probably been updated above, + dnl and we need the final values for function probing to work + CFLAGS="$old_CFLAGS $GNUTLS_CFLAGS" + LIBS="$old_LIBS $GNUTLS_LIBS" + + AC_CHECK_FUNCS([gnutls_rnd]) + CFLAGS="$old_CFLAGS" LIBS="$old_LIBS" fi diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fa7cfc9..55dcba8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -635,8 +635,8 @@ qemuDomainGenerateRandomKey(size_t nbytes) if (VIR_ALLOC_N(key, nbytes) < 0) return NULL; -#if HAVE_GNUTLS_CRYPTO_H - /* Generate a master key using gnutls if possible */ +#if HAVE_GNUTLS_RND + /* Generate a master key using gnutls_rnd() if possible */ if ((ret = gnutls_rnd(GNUTLS_RND_RANDOM, key, nbytes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to generate master key, ret=%d"), ret); @@ -644,7 +644,7 @@ qemuDomainGenerateRandomKey(size_t nbytes) return NULL; } #else - /* If we don't have gnutls, we will generate a less cryptographically + /* If we don't have gnutls_rnd(), we will generate a less cryptographically * strong master key from /dev/urandom. */ if ((ret = virRandomBytes(key, nbytes)) < 0) { -- 2.5.5
On 04/07/2016 09:29 AM, Andrea Bolognani wrote:
Patch 1 fixes a bug in configure.
Patch 2 performs a minor cleanup.
Patch 3 is the fix for the build issues currently experienced on CentOS 6 (see [1]).
Changes from v1:
* update CFLAGS and LIBS before performing the check, so that the compiler can actually find the function
Cheers.
[1] https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=...
Andrea Bolognani (3): configure: Restore CFLAGS properly after GnuTLS checks configure: Always use old_CFLAGS and old_LIBS qemu: Explicitly check for gnutls_rnd()
configure.ac | 31 +++++++++++++++++++------------ src/qemu/qemu_domain.c | 6 +++--- 2 files changed, 22 insertions(+), 15 deletions(-)
This does work for me.. Not sure I see the need for 1/3 since 2/3 essentially uses the old_CFLAGS to save and causes you to just undo what you did. I don't object to having both, just not sure it's necessary. ACK series though (and thanks for digging into this), John
On Thu, 2016-04-07 at 11:46 -0400, John Ferlan wrote:
configure: Restore CFLAGS properly after GnuTLS checks configure: Always use old_CFLAGS and old_LIBS qemu: Explicitly check for gnutls_rnd() configure.ac | 31 +++++++++++++++++++------------ src/qemu/qemu_domain.c | 6 +++--- 2 files changed, 22 insertions(+), 15 deletions(-)
This does work for me.. Not sure I see the need for 1/3 since 2/3 essentially uses the old_CFLAGS to save and causes you to just undo what you did. I don't object to having both, just not sure it's necessary.
Separate commits for separate changes. Squashing them together would yield the same end result, but the bug fix would be sort of lost among the noise generated by the cleanup.
ACK series though (and thanks for digging into this),
Pushed, let's see if that makes the CI happy :) Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
participants (2)
-
Andrea Bolognani -
John Ferlan