Add compat eventfd header

PATCH Superseded Michael S. Tsirkin 3 participants 10 messages Jun 28, 2011 – Jun 29, 2011

lore

Share & tools

lore

Michael S. Tsirkin

author

2011-06-28 15:35 UTC #0 lore raw

PATCH 1/1 Add compat eventfd header

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 <[email protected]>

---
 compat/sys/eventfd.h |   13 +++++++++++++
 configure            |    4 +++
 2 files changed, 16 insertions(+), 0 deletions(-)
 create mode 100644 compat/sys/eventfd.h

compat/sys/eventfd.h new file +13 -0

new file mode 100644
index 0000000..f55d96a
@@ -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)
+{
+    return syscall(SYS_eventfd, count, flags);
+}
+
+#endif

configure +3 -0

index 856b41e..d757a6a 100755
@@ -891,6 +891,9 @@ sparc64-bsd-user \
 "
 fi
 
+#compat headers
+QEMU_CFLAGS="$QEMU_CFLAGS -idirafter $source_path/compat"
+
 if test x"$show_help" = x"yes" ; then
 cat << EOF

Signature · 1 line

1.7.5.53.gc233e

Alexander Graf

2011-06-29 13:02 UTC #1 lore raw

Quoted 19 lines · Michael S. Tsirkin

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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a

compat/sys/eventfd.h line 8

Quoted 8 lines

compat/sys/eventfd.h new file +8 -0

@@ -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.

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

Michael S. Tsirkin

author

2011-06-29 13:11 UTC #2 lore raw

Quoted 16 lines · Alexander Graf

Quoted 13 lines · Michael S. Tsirkin
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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h

Quoted 16 lines

Quoted 6 lines
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a
compat/sys/eventfd.h line 8
Quoted 8 lines
compat/sys/eventfd.h new file +8 -0
@@ -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?

Quoted 4 lines

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?

Signature · 1 line

MST

Alexander Graf

2011-06-29 13:22 UTC #3 lore raw

Quoted 34 lines · Michael S. Tsirkin

Quoted 16 lines · Alexander Graf
Quoted 13 lines · Michael S. Tsirkin
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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
Quoted 16 lines
Quoted 6 lines
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a
compat/sys/eventfd.h line 8
Quoted 8 lines
compat/sys/eventfd.h new file +8 -0
@@ -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.

Quoted 1 line

What exactly? Two empty lines?

The space between d and ( I'd say. Just put it in checkpatch and verify it :).

Quoted 9 lines

Quoted 5 lines
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.

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

Michael S. Tsirkin

author

2011-06-29 13:59 UTC #4 lore raw

Quoted 37 lines · Alexander Graf

Quoted 34 lines · Michael S. Tsirkin
Quoted 32 lines · Alexander Graf
Quoted 19 lines · Michael S. Tsirkin
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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a
compat/sys/eventfd.h line 8
Quoted 8 lines
compat/sys/eventfd.h new file +8 -0
@@ -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.

Quoted 3 lines

Quoted 1 line
What exactly? Two empty lines?
The space between d and ( I'd say. Just put it in checkpatch and verify it :).

Will fix.

Quoted 11 lines

Quoted 9 lines
Quoted 5 lines
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.

Quoted 4 lines

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

3. now build starts
- if we had trouble compiling stuff eventfd gets disabled

I think the setup is highly safe.

Signature · 1 line

MST

Alexander Graf

2011-06-29 14:07 UTC #5 lore raw

Quoted 64 lines · Michael S. Tsirkin

Quoted 37 lines · Alexander Graf
Quoted 34 lines · Michael S. Tsirkin
Quoted 32 lines · Alexander Graf
Quoted 19 lines · Michael S. Tsirkin
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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a
compat/sys/eventfd.h line 8
Quoted 8 lines
compat/sys/eventfd.h new file +8 -0
@@ -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.
Quoted 3 lines
Quoted 1 line
What exactly? Two empty lines?
The space between d and ( I'd say. Just put it in checkpatch and verify it :).
Will fix.
Quoted 11 lines
Quoted 9 lines
Quoted 5 lines
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.
Quoted 4 lines
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

Quoted 7 lines

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

Michael S. Tsirkin

author

2011-06-29 15:28 UTC #6 lore raw

Quoted 75 lines · Alexander Graf

Quoted 64 lines · Michael S. Tsirkin
Quoted 37 lines · Alexander Graf
Quoted 34 lines · Michael S. Tsirkin
Quoted 32 lines · Alexander Graf
Quoted 19 lines · Michael S. Tsirkin
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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a
compat/sys/eventfd.h line 8
Quoted 8 lines
compat/sys/eventfd.h new file +8 -0
@@ -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.
Quoted 3 lines
Quoted 1 line
What exactly? Two empty lines?
The space between d and ( I'd say. Just put it in checkpatch and verify it :).
Will fix.
Quoted 11 lines
Quoted 9 lines
Quoted 5 lines
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.
Quoted 4 lines
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
Quoted 7 lines
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

Quoted 2 lines

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.

Quoted 1 line

Then it should be safe.

Good.

Quoted 6 lines

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__.

Signature · 1 line

MST

Alexander Graf

2011-06-29 15:30 UTC #7 lore raw

Quoted 90 lines · Michael S. Tsirkin

Quoted 75 lines · Alexander Graf
Quoted 64 lines · Michael S. Tsirkin
Quoted 37 lines · Alexander Graf
Quoted 34 lines · Michael S. Tsirkin
Quoted 32 lines · Alexander Graf
Quoted 19 lines · Michael S. Tsirkin
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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a
compat/sys/eventfd.h line 8
Quoted 8 lines
compat/sys/eventfd.h new file +8 -0
@@ -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.
Quoted 3 lines
Quoted 1 line
What exactly? Two empty lines?
The space between d and ( I'd say. Just put it in checkpatch and verify it :).
Will fix.
Quoted 11 lines
Quoted 9 lines
Quoted 5 lines
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.
Quoted 4 lines
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
Quoted 7 lines
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
Quoted 2 lines
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.
Quoted 1 line
Then it should be safe.
Good.
Quoted 5 lines
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.

Quoted 5 lines

Quoted 1 line
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 :).

Quoted 1 line

We should also check ifdef __linux__.

Good point, yes :)

Alex

Michael S. Tsirkin

author

2011-06-29 15:33 UTC #8 lore raw

Quoted 99 lines · Alexander Graf

Quoted 90 lines · Michael S. Tsirkin
Quoted 75 lines · Alexander Graf
Quoted 64 lines · Michael S. Tsirkin
Quoted 37 lines · Alexander Graf
Quoted 34 lines · Michael S. Tsirkin
Quoted 32 lines · Alexander Graf
Quoted 19 lines · Michael S. Tsirkin
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 <[email protected]>
---
compat/sys/eventfd.h | 13 +++++++++++++
configure | 4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
compat/sys/eventfd.h new file +0 -0
new file mode 100644
index 0000000..f55d96a
compat/sys/eventfd.h line 8
Quoted 8 lines
compat/sys/eventfd.h new file +8 -0
@@ -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.
Quoted 3 lines
Quoted 1 line
What exactly? Two empty lines?
The space between d and ( I'd say. Just put it in checkpatch and verify it :).
Will fix.
Quoted 11 lines
Quoted 9 lines
Quoted 5 lines
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.
Quoted 4 lines
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
Quoted 7 lines
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
Quoted 2 lines
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.
Quoted 1 line
Then it should be safe.
Good.
Quoted 5 lines
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.
Quoted 5 lines
Quoted 1 line
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.

Quoted 8 lines

I wouldn't call it needed, but I'd usually say better safe than sorry :).
Quoted 1 line
We should also check ifdef __linux__.
Good point, yes :)
Alex

Will do, thanks for the review.

Paolo Bonzini

2011-06-29 15:52 UTC #9 lore raw

In reply to Alexander Graf

Quoted 8 lines · Alexander Graf

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.
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?

No, he patched configure to use the compat header too, so detection would fail at configure time and eventfd would not be used.

Paolo