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? Then it should be safe. 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
