[libvirt] [PATCH] build: hoist qemu dependence on yajl to configure
Commit 6e769eba made it a runtime error if libvirt was compiled without yajl support but targets a new enough qemu. But enough users are hitting this on self-compiled libvirt that it is worth erroring out at compilation time, rather than an obscure failure when trying to use the built executable. * configure.ac: If qemu is requested and -version works, require yajl if qemu version is new enough. * src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Add comment. --- configure.ac | 19 ++++++++++++++++++- src/qemu/qemu_capabilities.c | 3 ++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index dda764d..0ba0598 100644 --- a/configure.ac +++ b/configure.ac @@ -991,6 +990,24 @@ AC_ARG_WITH([yajl], [], [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"` + AS_VERSION_COMPARE([$qemu_version], [0.15], + [], [with_yajl=yes], [with_yajl=yes]) + fi + fi +fi + YAJL_CFLAGS= YAJL_LIBS= with_yajl2=no diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b410648..f263375 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1206,7 +1206,8 @@ qemuCapsComputeCmdFlags(const char *help, * forgot to include YAJL libraries when building their own * libvirt but is targetting a newer qemu, we are better off * telling them to recompile (the spec file includes the - * dependency, so distros won't hit this). */ + * dependency, so distros won't hit this). This check is + * also in configure.ac (see $with_yajl). */ if (version >= 15000 || (version >= 12000 && strstr(help, "libvirt"))) { if (check_yajl) { -- 1.7.10.2
On 06/13/2012 11:00 AM, Eric Blake wrote:
Commit 6e769eba made it a runtime error if libvirt was compiled without yajl support but targets a new enough qemu. But enough users are hitting this on self-compiled libvirt that it is worth erroring out at compilation time, rather than an obscure failure when trying to use the built executable.
* configure.ac: If qemu is requested and -version works, require yajl if qemu version is new enough. * src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Add comment. ---
I did test that 'yum erase yajl-devel' followed by a fresh configure with this patch did complain on my F17 box; then reinstalling yajl-devel let configure proceed. Caveat - my RHEL 5 box (more precisely, the disk that hosts my RHEL 5 disk image) is currently in storage, so I haven't actually tested that this works on that older setup. Since RHEL 5 doesn't have yajl-devel, I would appreciate if someone can test that this change doesn't break ./configure with no options on that older setup. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Jun 13, 2012 at 12:00 PM, Eric Blake <eblake@redhat.com> wrote:
Commit 6e769eba made it a runtime error if libvirt was compiled without yajl support but targets a new enough qemu. But enough users are hitting this on self-compiled libvirt that it is worth erroring out at compilation time, rather than an obscure failure when trying to use the built executable.
* configure.ac: If qemu is requested and -version works, require yajl if qemu version is new enough. * src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Add comment. --- configure.ac | 19 ++++++++++++++++++- src/qemu/qemu_capabilities.c | 3 ++- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac index dda764d..0ba0598 100644 --- a/configure.ac +++ b/configure.ac @@ -991,6 +990,24 @@ AC_ARG_WITH([yajl], [], [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"` + AS_VERSION_COMPARE([$qemu_version], [0.15], + [], [with_yajl=yes], [with_yajl=yes]) + fi + fi +fi + YAJL_CFLAGS= YAJL_LIBS= with_yajl2=no diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b410648..f263375 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1206,7 +1206,8 @@ qemuCapsComputeCmdFlags(const char *help, * forgot to include YAJL libraries when building their own * libvirt but is targetting a newer qemu, we are better off * telling them to recompile (the spec file includes the - * dependency, so distros won't hit this). */ + * dependency, so distros won't hit this). This check is + * also in configure.ac (see $with_yajl). */ if (version >= 15000 || (version >= 12000 && strstr(help, "libvirt"))) { if (check_yajl) { -- 1.7.10.2
FWIW, visually inspecting this code I can ACK it. I'll do some test builds with it later. -- Doug Goldstein
On 06/15/2012 10:24 AM, Doug Goldstein wrote:
On Wed, Jun 13, 2012 at 12:00 PM, Eric Blake <eblake@redhat.com> wrote:
Commit 6e769eba made it a runtime error if libvirt was compiled without yajl support but targets a new enough qemu. But enough users are hitting this on self-compiled libvirt that it is worth erroring out at compilation time, rather than an obscure failure when trying to use the built executable.
* configure.ac: If qemu is requested and -version works, require yajl if qemu version is new enough. * src/qemu/qemu_capabilities.c (qemuCapsComputeCmdFlags): Add comment. --- configure.ac | 19 ++++++++++++++++++- src/qemu/qemu_capabilities.c | 3 ++- 2 files changed, 20 insertions(+), 2 deletions(-)
FWIW, visually inspecting this code I can ACK it. I'll do some test builds with it later.
Thanks for the review. I've gone ahead and pushed this. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Doug Goldstein -
Eric Blake