* [Qemu-devel] [PATCH] Add compat eventfd header @ 2011-06-28 15:35 Michael S. Tsirkin 2011-06-29 13:02 ` Alexander Graf 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2011-06-28 15:35 UTC (permalink / raw) To: qemu-devel Cc: Stefan Hajnoczi, Michael S. Tsirkin, Alexander Graf, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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) +{ + return syscall(SYS_eventfd, count, flags); +} + +#endif diff --git a/configure b/configure index 856b41e..d757a6a 100755 --- a/configure +++ b/configure @@ -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 -- 1.7.5.53.gc233e ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-28 15:35 [Qemu-devel] [PATCH] Add compat eventfd header Michael S. Tsirkin @ 2011-06-29 13:02 ` Alexander Graf 2011-06-29 13:11 ` Michael S. Tsirkin 0 siblings, 1 reply; 10+ messages in thread From: Alexander Graf @ 2011-06-29 13:02 UTC (permalink / raw) To: Michael S.Tsirkin Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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. 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 13:02 ` Alexander Graf @ 2011-06-29 13:11 ` Michael S. Tsirkin 2011-06-29 13:22 ` Alexander Graf 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2011-06-29 13:11 UTC (permalink / raw) To: Alexander Graf Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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? > 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? -- MST ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 13:11 ` Michael S. Tsirkin @ 2011-06-29 13:22 ` Alexander Graf 2011-06-29 13:59 ` Michael S. Tsirkin 2011-06-29 15:52 ` Paolo Bonzini 0 siblings, 2 replies; 10+ messages in thread From: Alexander Graf @ 2011-06-29 13:22 UTC (permalink / raw) To: Michael S.Tsirkin Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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 :). > >> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 13:22 ` Alexander Graf @ 2011-06-29 13:59 ` Michael S. Tsirkin 2011-06-29 14:07 ` Alexander Graf 2011-06-29 15:52 ` Paolo Bonzini 1 sibling, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2011-06-29 13:59 UTC (permalink / raw) To: Alexander Graf Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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 3. now build starts - if we had trouble compiling stuff eventfd gets disabled I think the setup is highly safe. -- MST ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 13:59 ` Michael S. Tsirkin @ 2011-06-29 14:07 ` Alexander Graf 2011-06-29 15:28 ` Michael S. Tsirkin 0 siblings, 1 reply; 10+ messages in thread From: Alexander Graf @ 2011-06-29 14:07 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 14:07 ` Alexander Graf @ 2011-06-29 15:28 ` Michael S. Tsirkin 2011-06-29 15:30 ` Alexander Graf 0 siblings, 1 reply; 10+ messages in thread From: Michael S. Tsirkin @ 2011-06-29 15:28 UTC (permalink / raw) To: Alexander Graf Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 15:28 ` Michael S. Tsirkin @ 2011-06-29 15:30 ` Alexander Graf 2011-06-29 15:33 ` Michael S. Tsirkin 0 siblings, 1 reply; 10+ messages in thread From: Alexander Graf @ 2011-06-29 15:30 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno 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 <mst@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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 15:30 ` Alexander Graf @ 2011-06-29 15:33 ` Michael S. Tsirkin 0 siblings, 0 replies; 10+ messages in thread From: Michael S. Tsirkin @ 2011-06-29 15:33 UTC (permalink / raw) To: Alexander Graf Cc: Stefan Hajnoczi, qemu-devel, Blue Swirl, jan.kiszka, Paolo Bonzini, Aurelien Jarno On Wed, Jun 29, 2011 at 05:30:24PM +0200, Alexander Graf wrote: > > 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 <mst@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 Will do, thanks for the review. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] Add compat eventfd header 2011-06-29 13:22 ` Alexander Graf 2011-06-29 13:59 ` Michael S. Tsirkin @ 2011-06-29 15:52 ` Paolo Bonzini 1 sibling, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2011-06-29 15:52 UTC (permalink / raw) To: Alexander Graf Cc: Stefan Hajnoczi, Michael S.Tsirkin, qemu-devel, Blue Swirl, jan.kiszka, Aurelien Jarno On 06/29/2011 03:22 PM, Alexander Graf wrote: > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-06-29 15:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-28 15:35 [Qemu-devel] [PATCH] Add compat eventfd header Michael S. Tsirkin 2011-06-29 13:02 ` Alexander Graf 2011-06-29 13:11 ` Michael S. Tsirkin 2011-06-29 13:22 ` Alexander Graf 2011-06-29 13:59 ` Michael S. Tsirkin 2011-06-29 14:07 ` Alexander Graf 2011-06-29 15:28 ` Michael S. Tsirkin 2011-06-29 15:30 ` Alexander Graf 2011-06-29 15:33 ` Michael S. Tsirkin 2011-06-29 15:52 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).