On Wed, Jun 29, 2011 at 04:07:50PM +0200, Alexander Graf wrote: > > On 29.06.2011, at 15:59, Michael S. Tsirkin wrote: > > > On Wed, Jun 29, 2011 at 03:22:33PM +0200, Alexander Graf wrote: > >> > >> On 29.06.2011, at 15:11, Michael S. Tsirkin wrote: > >> > >>> On Wed, Jun 29, 2011 at 03:02:46PM +0200, Alexander Graf wrote: > >>>> > >>>> On 28.06.2011, at 17:35, Michael S. Tsirkin wrote: > >>>> > >>>>> Support build on RHEL 5.X where we have syscall for eventfd but not > >>>>> userspace wrapper. > >>>>> > >>>>> (cherry-picked from commit 9e3269181e9bc56feb43bcd4e8ce0b82cd543e65 > >>>>> in qemu-kvm.git). > >>>>> > >>>>> Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > >>>>> --- > >>>>> compat/sys/eventfd.h | 13 +++++++++++++ > >>>>> configure | 4 +++ > >>>>> 2 files changed, 16 insertions(+), 0 deletions(-) > >>>>> create mode 100644 compat/sys/eventfd.h > >>>>> > >>>>> diff --git a/compat/sys/eventfd.h b/compat/sys/eventfd.h > >>>>> new file mode 100644 > >>>>> index 0000000..f55d96a > >>>>> --- /dev/null > >>>>> +++ b/compat/sys/eventfd.h > >>>>> @@ -0,0 +1,13 @@ > >>>>> +#ifndef _COMPAT_SYS_EVENTFD > >>>>> +#define _COMPAT_SYS_EVENTFD > >>>>> + > >>>>> +#include <unistd.h> > >>>>> +#include <syscall.h> > >>>>> + > >>>>> + > >>>>> +static inline int eventfd (int count, int flags) > >>>> > >>>> coding style seems wrong. > >>> > >>> What exactly? Two empty lines? > >> > >> The space between d and ( I'd say. Just put it in checkpatch and verify it > >> :). > > > > Will fix. > > > >>> > >>>> However, I'm not sure I like the idea of adding this code in qemu. > >>>> Wouldn't the RHEL5 libc be a better place for such a wrapper? > >>>> > >>>> > >>>> Alex > >>> > >>> My guess (I don't speak for red hat here) is that's unlikely to be > >>> patched anytime soon. It helps me when I need to use such a box, > >>> and the cost seems negligeable. What's the drawback? > >> > >> Well, you need to make sure that it only gets included on Linux systems > >> and if there's ever some more compatibility wrapping around the syscall > >> (unlikely, but you never know), this could potentially break. > > > > Nope, this gets included last (-idirafter) so if it breaks it's broken > > anyway. > > > >> Also, who defines SYS_eventfd? What if you're trying to build this code on > >> SLES10 for example, which does not have the syscall and thus doesn't have > >> it defined? Would compilation simply break? > >> > >> > >> Alex > > > > Here's what happens: > > 1. configure runs > > 2. configure tries to compile a test program > > - if eventfd.h exists in system compat is ignored > > - if eventfd.h does not exist in system compat is used > > - if compat is used but does build program does not compile > > - if program does not compile eventfd is disabled > > Sure, but the cflags is added nevertheless, right? So you end up including a > header file that uses undefined constants or even includes random header > files that don't necessarily exist on your OS. > Or is sys/eventfd.h only #include'd when the config option is set?
Yes. > Then it should be safe. Good. > However, it might make sense to double-check > that inside the header itself and #error out in case the config option > is not set, so that this gets caught easily. > > > Alex That makes the original testing that the header works a bit trickier: we have to add -DCONFIG_EVENTFD. But I can do that if you think it's needed. We should also check ifdef __linux__. -- MST
