[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Add compat eventfd header
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH] Add compat eventfd header |
Date: |
Wed, 29 Jun 2011 17:30:24 +0200 |
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 <address@hidden>
>>>>>>> ---
>>>>>>> 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
- [Qemu-devel] [PATCH] Add compat eventfd header, Michael S. Tsirkin, 2011/06/28
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Alexander Graf, 2011/06/29
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Michael S. Tsirkin, 2011/06/29
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Alexander Graf, 2011/06/29
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Michael S. Tsirkin, 2011/06/29
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Alexander Graf, 2011/06/29
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Michael S. Tsirkin, 2011/06/29
- Re: [Qemu-devel] [PATCH] Add compat eventfd header,
Alexander Graf <=
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Michael S. Tsirkin, 2011/06/29
- Re: [Qemu-devel] [PATCH] Add compat eventfd header, Paolo Bonzini, 2011/06/29