On 12/05/2012 04:57 PM, Kir Kolyshkin wrote:
> On 11/26/2012 06:35 PM, Glauber Costa wrote:
>> The include file "vzsyscalls.h" have a comprehensive list of vz-syscalls
>> and which architectures support them. If that file is included in a not
>> supported architecture, it will trigger an #error, which is a very
>> reliable way to determine that vz-specific code should not be compiled
>> in at all.
>>
>> Signed-off-by: Glauber Costa <[email protected]>
>> ---
>>   configure.ac | 18 ++++++++++++++++++
>>   1 file changed, 18 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 3c04300..59e7384 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -65,6 +65,14 @@ CFLAGS="${CFLAGS} -Wall -Wpointer-arith -Wcast-qual
>> -Winline -Wextra"
>>   CFLAGS="${CFLAGS} -Wcast-align -Wno-unused-parameter"
>>   CFLAGS="${CFLAGS} -Wno-missing-field-initializers"
>>   CFLAGS="${CFLAGS} -D_FILE_OFFSET_BITS=64"
>> +# to test for the usability of some of our headers
>> +CFLAGS="${CFLAGS} -Iinclude/"
>> +
>> +# Those are the pre-processor flags. Failure to set it, can lead to
>> messages
>> +# like "accepted by the compiler, rejected by the preprocessor",
>> meaning the
>> +# compiler found the headers, but the pre-processor did not.
>> +CPPFLAGS="${CFLAGS}"
>> +
>>     # We need -m64 for ppc64 in order to get proper ioctls
>>   if test x$target_cpu = xppc64 -o x$target_cpu = xpowerpc64; then
>> @@ -188,6 +196,16 @@ AM_CONDITIONAL(HAVE_CGROUP, [test "x$with_cgroup"
>> = "x+cgroup"])
>>   AC_SUBST(CGROUP_LIBS)
>>   AC_SUBST(CGROUP_CFLAGS)
>>   +
>> +# vzsyscalls.h will encode the knowledge of which syscalls are
>> supported in
>> +# which archs, and determine whether or not the VZ_KERNEL is usable.
>> For now
>> +# we will only disable it if we can't really use it.
>> +AC_CHECK_HEADERS([vzsyscalls.h],
>> +    [AC_DEFINE(VZ_KERNEL_SUPPORTED)
>> +     AM_CONDITIONAL(HAVE_VZ_KERNEL, true)],
>> +    [AM_CONDITIONAL(HAVE_VZ_KERNEL, false)]
>> +)
>> +
> 
> This is dirty in multiple ways.
> 1. AC_CHECK_HEADERS should be used for _system_ headers, so it tries
> compiling
> a program with #include <vzsyscalls.h>, while we need it to have
> #include "vzsyscalls.h".
> 
> 2. You worked around that by adding -Iinclude setting to CFLAGS and
> CPPFLAGS, which
> itself is dirty in multiple ways.
> 
> 3. The output that this part of configure produces is very cryptic -- it
> shows us whether
> vzsyscalls.h is present and usable or not, instead of saying whether
> this architecture
> can possibly be supported by OpenVZ.
> 
> I suggest using AC_COMPILE_IFELSE or even AC_PREPROC_IFELSE to make this
> cleaner.
> 
> 2.
> 
>>   # A way to redefine /vz is ./configure vzdir=/some/dir
>>   AS_IF([test "x$vzdir" = "x"], vzdir=/vz)
>>   AC_ARG_VAR(vzdir, [Common prefix for a few OpenVZ-related directories,
> 
> 
All reasonable.

Please consider the following patch instead. Let me know if you would
like any of the others in the series to be changed.

>From 9b5715bfead816ca43f21b24a1d4a5fc2d2b40c3 Mon Sep 17 00:00:00 2001
From: Glauber Costa <[email protected]>
Date: Fri, 23 Nov 2012 17:25:48 +0400
Subject: [PATCH] test for vz-kernel support in the target architecture

The include file "vzsyscalls.h" have a comprehensive list of vz-syscalls
and which architectures support them. If that file is included in a not
supported architecture, it will trigger an #error, which is a very
reliable way to determine that vz-specific code should not be compiled
in at all.

Since this places some new infrastructure in place for presence of
OpenVZ kernel support, this comes with the ability of disabling this
from the command line as well.

Signed-off-by: Glauber Costa <[email protected]>
---
 configure.ac | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 3c04300..c49022b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -188,6 +188,36 @@ AM_CONDITIONAL(HAVE_CGROUP, [test "x$with_cgroup" = "x+cgroup"])
 AC_SUBST(CGROUP_LIBS)
 AC_SUBST(CGROUP_CFLAGS)
 
+
+AC_ARG_WITH([vz],
+            [AS_HELP_STRING([--with-vz],
+                            [Enable support for OpenVZ kernel])],
+            [case "${withval}" in
+              yes) with_vz="+vz";;
+              no)  with_vz="-vz";;
+              *)   AC_MSG_ERROR(bad value ${withval} for --with-vz);;
+            esac],
+            [with_vz="+vz"])
+
+
+AS_IF([test "x$with_vz" = "x+vz"],
+# vzsyscalls.h will encode the knowledge of which syscalls are supported in
+# which archs, and determine whether or not the VZ_KERNEL is usable. For now
+# we will only disable it if we can't really use it.
+
+[AC_MSG_CHECKING([if current architecture supported by OpenVZ])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include "${srcdir}/include/vzsyscalls.h"]])],
+	[AC_DEFINE(VZ_KERNEL_SUPPORTED)
+	 AM_CONDITIONAL(HAVE_VZ_KERNEL, true)
+	 AC_MSG_RESULT([yes])
+	],
+	[AM_CONDITIONAL(HAVE_VZ_KERNEL, false)
+	 with_vz="-vz"
+	AC_MSG_RESULT([no])]
+)],
+[AM_CONDITIONAL(HAVE_VZ_KERNEL, false)]
+)
+
 # A way to redefine /vz is ./configure vzdir=/some/dir
 AS_IF([test "x$vzdir" = "x"], vzdir=/vz)
 AC_ARG_VAR(vzdir, [Common prefix for a few OpenVZ-related directories,
@@ -205,9 +235,13 @@ AC_CONFIG_COMMANDS_PRE([SUMMARY="$PACKAGE_STRING configured successfully:
  sysconfdir: $sysconfdir
      libdir: $libdir
       vzdir: $vzdir
-   features: $enable_bashcomp $enable_logrotate $enable_udev $with_ploop $with_cgroup
+   features: $enable_bashcomp $enable_logrotate $enable_udev $with_ploop $with_cgroup $with_vz
 "])
 
+AS_IF([test "x$with_vz" = "x-vz" -a "x$with_cgroup" = "x-cgroup"],
+[AC_ERROR("At least one of cgroup or vz must be enabled")]
+)
+
 # Output
 AC_CONFIG_FILES([bin/Makefile
                  etc/bash_completion.d/Makefile
-- 
1.7.11.7

_______________________________________________
Devel mailing list
[email protected]
http://lists.openvz.org/mailman/listinfo/devel

Reply via email to