On 29.06.2011, at 17:28, Michael S. Tsirkin wrote: > 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.
I wouldn't call it needed, but I'd usually say better safe than sorry :). > We should also check ifdef __linux__. Good point, yes :) Alex
