Re: [Qemu-devel] [PULL v2 00/27] ivshmem deprecation, qtests, typedefs and gnu99

[Daniel P . Berrangé]

On Tue, Jan 15, 2019 at 03:32:10PM +0100, Thomas Huth wrote:
> On 2019-01-15 14:16, Peter Maydell wrote:
> > On Mon, 14 Jan 2019 at 17:45, Thomas Huth <address@hidden> wrote:
> >>
> >>  Hi Peter!
> >>
> >> The following changes since commit 
> >> 7260438b7056469610ee166f7abe9ff8a26b8b16:
> >>
> >>   Merge remote-tracking branch 
> >> 'remotes/palmer/tags/riscv-for-master-3.2-part2' into staging (2019-01-14 
> >> 11:41:43 +0000)
> >>
> >> are available in the git repository at:
> >>
> >>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-01-14v2
> >>
> >> for you to fetch changes up to 650db715681ad1a042705484776e1974f288f3d4:
> >>
> >>   tests/hexloader-test: Don't pass -nographic to the QEMU under test 
> >> (2019-01-14 18:21:29 +0100)
> >>
> >> ----------------------------------------------------------------
> >> - Remove deprecated "ivshmem" legacy device
> >> - Bug fix for vhost-user-test
> >> - Use more CONFIG Makefile switches for qtests
> >> - Get rid of global_qtests in some more qtests
> >> - typedef cleanups
> >> - Fixes for compiling with Clang
> >> - Force C standard to gnu99
> >> ----------------------------------------------------------------
> > 
> > Hi; another compile failure on that ppc64 system, I'm afraid:
> > 
> > /home/pm215/qemu/qemu-seccomp.c:45:1: error: initializer element is not 
> > constant
> >  };
> >  ^
> > /home/pm215/qemu/qemu-seccomp.c:45:1: error: (near initialization for
> > ‘sched_setscheduler_arg[0]’)
> > 
> > (I did a quick check with 'make -k' and it looks like there aren't
> > any more lurking after that one.)
> > 
> > The system libseccomp is libseccomp-2.3.1-3.el7.
> 
> Darn, I think this time it is a compiler bug:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63567
> 
> It's only fixed in GCC >= v5.0 :-(
> 
> I see two options:
> 
> 1) Expand the macro manually:
> 
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -41,7 +41,7 @@ struct QemuSeccompSyscall {
>  };
> 
>  const struct scmp_arg_cmp sched_setscheduler_arg[] = {
> -    SCMP_A1(SCMP_CMP_NE, SCHED_IDLE)
> +    { .arg = 1, .op = SCMP_CMP_NE, .datum_a = SCHED_IDLE }
>  };
> 
> ... then it compiles fine for me in gnu99 mode, too.
> 
> 2) Scratch the whole idea with gnu99 again ...

I'd prefer 1, since I think its important for us to actually have a
consistent compiler standard to target, even if we have to do some
short terms hacks like the one you show.  As long as a put a comment
next to it, we can revert the hack when we eventually ditch support for the
GCC <= 5.0

Also, I'd probably argue that the seccomp syntax without the macro
is clearer to understand regardless :-)

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|