Re: [Qemu-devel] [PATCH] seccomp: change configure to avoid arm 32 to break

[Eduardo Otubo <eduardo.otubo@profitbricks.com>]

On Thu, Nov 06, 2014 at 03:49:04PM +0000, Peter Maydell wrote:
> On 6 November 2014 14:49, Eduardo Otubo <eduardo.otubo@profitbricks.com> wrote:
> > Right now seccomp is breaking the compilation of Qemu on armv7l due
> > to libsecomp current lack of support for this arch. This problem is
> > already fixed on libseccomp upstream but no release date for that is
> > scheduled to far. This patch disables support for seccomp on armv7l
> > temporarily until libseccomp does a new release. Then I'll remove the
> > hack and update libseccomp dependency on configure script.
> >
> > Related bug: https://bugs.launchpad.net/qemu/+bug/1363641
> >
> > Signed-off-by: Eduardo Otubo <eduardo.otubo@profitbricks.com>
> > ---
> >  configure | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 2f17bf3..16fd7f5 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1823,15 +1823,17 @@ fi
> >  # libseccomp check
> >
> >  if test "$seccomp" != "no" ; then
> > -    if $pkg_config --atleast-version=2.1.0 libseccomp; then
> > -        libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
> > -        QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
> > -       seccomp="yes"
> > -    else
> > -       if test "$seccomp" = "yes"; then
> > -            feature_not_found "libseccomp" "Install libseccomp devel >= 2.1.0"
> > -       fi
> > -       seccomp="no"
> > +    if test "$cpu" = "i386" || test "$cpu" = "x86_64"; then
> > +        if $pkg_config --atleast-version=2.1.0 libseccomp; then
> > +            libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
> > +            QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
> > +            seccomp="yes"
> > +        else
> > +            if test "$seccomp" = "yes"; then
> > +                feature_not_found "libseccomp" "Install libseccomp devel >= 2.1.0"
> > +            fi
> > +            seccomp="no"
> > +        fi
> >      fi
> 
> This is missing the logic for turning "" into "no" or printing the
> feature_not_found message if the probe failed because of the CPU
> being wrong. The easiest fix for that is just to roll the whole check
> into one if:
>     if { test "$cpu" = "i386" || test "$cpu" = "x86_64"; } &&
>         $pkg_config --atleast-version=2.1.0 libseccomp; then
> 

Yep, I think that does make a lot of sense looking from the perspective
of feature_not_found. I'll just roll out a v3 if you don't mind (I guess
this was supposed to be the v2)

Thanks for the review,

-- 
Eduardo Otubo
ProfitBricks GmbH