[PATCH v2] kmod: Handle undefined O_CLOEXEC

[PATCH v2] kmod: Handle undefined O_CLOEXEC

[Radu Moisan]

Close-on-exec seems to be unsuported on some architectures like CentOS 5.8
and thus causing some packages to fail to build successfully. Future kernel
version will probably fix this, but for now this patch works around this
problem.

Signed-off-by: Radu Moisan <radu.moisan@intel.com>
---
 meta/recipes-kernel/kmod/kmod.inc                  |    3 +-
 .../Handle-unsupported-close-on-exec-flag.patch    |   60 ++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 meta/recipes-kernel/kmod/kmod/Handle-unsupported-close-on-exec-flag.patch

diff --git a/meta/recipes-kernel/kmod/kmod.inc b/meta/recipes-kernel/kmod/kmod.inc
index adba4d4..c992ad8 100644
--- a/meta/recipes-kernel/kmod/kmod.inc
+++ b/meta/recipes-kernel/kmod/kmod.inc
@@ -8,7 +8,7 @@ LICENSE = "GPL-2.0+ & LGPL-2.1+"
 LICENSE_libkmod = "LGPL-2.1+"
 SECTION = "base"
 PV = "8"
-INC_PR = "r1"
+INC_PR = "r2"
 
 DEPENDS += "pkgconfig-native"
 
@@ -20,6 +20,7 @@ inherit autotools gtk-doc
 SRC_URI = "git://git.profusion.mobi/kmod.git;protocol=git;branch=master \
            file://depmod-search.conf \
            file://0001-man-disable-man-page-generation-because-we-don-t-hav.patch \
+           file://Handle-unsupported-close-on-exec-flag.patch \
           "
 
 SRCREV = "819f79a24d58e3c8429f1631df2f8f85a2f95d4a"
diff --git a/meta/recipes-kernel/kmod/kmod/Handle-unsupported-close-on-exec-flag.patch b/meta/recipes-kernel/kmod/kmod/Handle-unsupported-close-on-exec-flag.patch
new file mode 100644
index 0000000..f0820d9
--- /dev/null
+++ b/meta/recipes-kernel/kmod/kmod/Handle-unsupported-close-on-exec-flag.patch
@@ -0,0 +1,60 @@
+Index: git/libkmod/libkmod-config.c
+===================================================================
+--- git.orig/libkmod/libkmod-config.c	2012-07-23 16:13:44.000000000 +0300
++++ git/libkmod/libkmod-config.c	2012-07-23 16:15:53.000000000 +0300
+@@ -33,6 +33,10 @@
+ #include "libkmod.h"
+ #include "libkmod-private.h"
+ 
++#ifndef O_CLOEXEC
++#define O_CLOEXEC 0
++#endif
++
+ struct kmod_alias {
+ 	char *name;
+ 	char modname[];
+Index: git/libkmod/libkmod-file.c
+===================================================================
+--- git.orig/libkmod/libkmod-file.c	2012-07-23 16:13:44.000000000 +0300
++++ git/libkmod/libkmod-file.c	2012-07-23 16:15:57.000000000 +0300
+@@ -31,6 +31,10 @@
+ #include "libkmod.h"
+ #include "libkmod-private.h"
+ 
++#ifndef O_CLOEXEC
++#define O_CLOEXEC 0
++#endif
++
+ #ifdef ENABLE_XZ
+ #include <lzma.h>
+ #endif
+Index: git/libkmod/libkmod-index.c
+===================================================================
+--- git.orig/libkmod/libkmod-index.c	2012-07-23 16:13:44.000000000 +0300
++++ git/libkmod/libkmod-index.c	2012-07-23 16:16:00.000000000 +0300
+@@ -31,6 +31,10 @@
+ #include "libkmod-index.h"
+ #include "macro.h"
+ 
++#ifndef O_CLOEXEC
++#define O_CLOEXEC 0
++#endif
++
+ /* index.c: module index file shared functions for modprobe and depmod */
+ 
+ #define INDEX_CHILDMAX 128
+Index: git/libkmod/libkmod-module.c
+===================================================================
+--- git.orig/libkmod/libkmod-module.c	2012-07-23 16:13:44.000000000 +0300
++++ git/libkmod/libkmod-module.c	2012-07-23 16:16:04.000000000 +0300
+@@ -40,6 +40,10 @@
+ #include "libkmod.h"
+ #include "libkmod-private.h"
+ 
++#ifndef O_CLOEXEC
++#define O_CLOEXEC 0
++#endif
++
+ /**
+  * SECTION:libkmod-module
+  * @short_description: operate on kernel modules
-- 
1.7.9.5

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Enrico Scholz]

Radu Moisan <radu.moisan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
writes:

> Close-on-exec seems to be unsuported on some architectures like CentOS 5.8
> and thus causing some packages to fail to build successfully. 

Have you verified that making O_CLOEXEC a noop does not break the
applications?


Enrico

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Radu Moisan]

As far as kmod package is concerned O_CLOEXEC is used in constructs like 
"O_RDONLY|O_CLOEXEC". O_CLOEXEC can be used (is defined) starting with 
Linux kernel ≥2.6.23 and glibc ≥2.7 case in which the patch does not 
logically changing anything. However, prior Linux kernel ≥2.6.23 
O_CLOEXEC is not defined (the case for CentOS 5.8 - kernel 2.6.18) and 
using it in code will cause build errors. My patch provides a workaround 
for those distributions that do not have O_CLOEXEC define, just to be 
able to build stuff. I have not tested on CentOS 5.8 if the applications 
are not broken in some way, but that's not in the scope of this patch. 
If something does indeed break, then a totally different patch is 
required, targeting a backport of kmod for kernel older than 2.6.23.

Radu

On 07/23/2012 05:34 PM, Enrico Scholz wrote:
> Radu Moisan <radu.moisan-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> writes:
>
>> Close-on-exec seems to be unsuported on some architectures like CentOS 5.8
>> and thus causing some packages to fail to build successfully.
> Have you verified that making O_CLOEXEC a noop does not break the
> applications?
>
>
> Enrico
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Chris Larson]

On Tue, Jul 24, 2012 at 12:37 AM, Radu Moisan <radu.moisan@intel.com> wrote:
> I have not tested on CentOS 5.8 if the applications are not broken in some
> way, but that's not in the scope of this patch. If something does indeed
> break, then a totally different patch is required, targeting a backport of
> kmod for kernel older than 2.6.23.

Personally, I'd rather see the build fail than have the tools behave
incorrectly in some inexplicable way. If you haven't tested it, the
patch shouldn't go in.
-- 
Christopher Larson

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Burton, Ross]

On 24 July 2012 14:27, Chris Larson <clarson@kergoth.com> wrote:
> On Tue, Jul 24, 2012 at 12:37 AM, Radu Moisan <radu.moisan@intel.com> wrote:
>> I have not tested on CentOS 5.8 if the applications are not broken in some
>> way, but that's not in the scope of this patch. If something does indeed
>> break, then a totally different patch is required, targeting a backport of
>> kmod for kernel older than 2.6.23.
>
> Personally, I'd rather see the build fail than have the tools behave
> incorrectly in some inexplicable way. If you haven't tested it, the
> patch shouldn't go in.

I was curious...

There are two commits in kmod where the cloexec changes were made:

http://git.profusion.mobi/cgit.cgi/kmod.git/log/?qt=grep&q=cloexec

The changes were a simple addition of the O_CLOEXEC flag, so this
patch is simply the union of those two commits.  A release of kmod
that doesn't require O_CLOEXEC has the same behaviour as this patch.
The problem O_CLOEXEC is solving isn't possible to solve cleanly
without it.  Using an older version of kmod instead of patching kmod
to work on older systems would result in more bugs and less features
for no win.

Ross

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[McClintock Matthew-B29882]

On Tue, Jul 24, 2012 at 8:40 AM, Burton, Ross <ross.burton@intel.com> wrote:
> On 24 July 2012 14:27, Chris Larson <clarson@kergoth.com> wrote:
>> On Tue, Jul 24, 2012 at 12:37 AM, Radu Moisan <radu.moisan@intel.com> wrote:
>>> I have not tested on CentOS 5.8 if the applications are not broken in some
>>> way, but that's not in the scope of this patch. If something does indeed
>>> break, then a totally different patch is required, targeting a backport of
>>> kmod for kernel older than 2.6.23.
>>
>> Personally, I'd rather see the build fail than have the tools behave
>> incorrectly in some inexplicable way. If you haven't tested it, the
>> patch shouldn't go in.
>
> I was curious...
>
> There are two commits in kmod where the cloexec changes were made:
>
> http://git.profusion.mobi/cgit.cgi/kmod.git/log/?qt=grep&q=cloexec
>
> The changes were a simple addition of the O_CLOEXEC flag, so this
> patch is simply the union of those two commits.  A release of kmod
> that doesn't require O_CLOEXEC has the same behaviour as this patch.
> The problem O_CLOEXEC is solving isn't possible to solve cleanly
> without it.  Using an older version of kmod instead of patching kmod
> to work on older systems would result in more bugs and less features
> for no win.

Was there any conclusion on this? I'm seeing the same problems. This
would only effect kmod-native (unless the target was using the older
stuff as well which is uncommon at this point).

Looking the commit that adds this as a dep:

commit f22cf1bedf2aa7fb41f98d6165401eb8a8bad17d
Author: Khem Raj <raj.khem@gmail.com>
Date:   Tue Jan 31 00:35:02 2012 -0800

    image.bbclass,kernel.bbclass: Use kmod-native instead of
module-init-tools-cross

We are just using depmod from this recipe for the kernel build, which
is a non-threaded user of this libraries built within kmod - therefore
we should not encounter any issues [1](after reading what O_CLOEXEC
does) with this patch except that it should only be applied to -native
builds. Also, if issues do present themselves they will be during the
build and not later at runtime so we can identify any issues that
arise.

So in summary, the patch should (IMO) be:

SRC_URI_append_virtclass-native =
"file://Handle-unsupported-close-on-exec-flag.patch"

Thoughts?

[1] http://lwn.net/Articles/249006/

-M

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Chris Larson]

On Wed, Aug 15, 2012 at 11:37 AM, McClintock Matthew-B29882
<B29882@freescale.com> wrote:
> On Tue, Jul 24, 2012 at 8:40 AM, Burton, Ross <ross.burton@intel.com> wrote:
>> On 24 July 2012 14:27, Chris Larson <clarson@kergoth.com> wrote:
>>> On Tue, Jul 24, 2012 at 12:37 AM, Radu Moisan <radu.moisan@intel.com> wrote:
>>>> I have not tested on CentOS 5.8 if the applications are not broken in some
>>>> way, but that's not in the scope of this patch. If something does indeed
>>>> break, then a totally different patch is required, targeting a backport of
>>>> kmod for kernel older than 2.6.23.
>>>
>>> Personally, I'd rather see the build fail than have the tools behave
>>> incorrectly in some inexplicable way. If you haven't tested it, the
>>> patch shouldn't go in.
>>
>> I was curious...
>>
>> There are two commits in kmod where the cloexec changes were made:
>>
>> http://git.profusion.mobi/cgit.cgi/kmod.git/log/?qt=grep&q=cloexec
>>
>> The changes were a simple addition of the O_CLOEXEC flag, so this
>> patch is simply the union of those two commits.  A release of kmod
>> that doesn't require O_CLOEXEC has the same behaviour as this patch.
>> The problem O_CLOEXEC is solving isn't possible to solve cleanly
>> without it.  Using an older version of kmod instead of patching kmod
>> to work on older systems would result in more bugs and less features
>> for no win.
>
> Was there any conclusion on this? I'm seeing the same problems. This
> would only effect kmod-native (unless the target was using the older
> stuff as well which is uncommon at this point).

For what it's worth, we applied this in our layer and things do seem
to work fine with this applied. It was either this or what we did
before (reverted the switch to kmod-native and retained
module-init-tools-cross), as we require CentOS5/RHEL5 support still.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[McClintock Matthew-B29882]

On Wed, Aug 15, 2012 at 2:10 PM, Chris Larson <clarson@kergoth.com> wrote:
> On Wed, Aug 15, 2012 at 11:37 AM, McClintock Matthew-B29882
> <B29882@freescale.com> wrote:
>> On Tue, Jul 24, 2012 at 8:40 AM, Burton, Ross <ross.burton@intel.com> wrote:
>>> On 24 July 2012 14:27, Chris Larson <clarson@kergoth.com> wrote:
>>>> On Tue, Jul 24, 2012 at 12:37 AM, Radu Moisan <radu.moisan@intel.com> wrote:
>>>>> I have not tested on CentOS 5.8 if the applications are not broken in some
>>>>> way, but that's not in the scope of this patch. If something does indeed
>>>>> break, then a totally different patch is required, targeting a backport of
>>>>> kmod for kernel older than 2.6.23.
>>>>
>>>> Personally, I'd rather see the build fail than have the tools behave
>>>> incorrectly in some inexplicable way. If you haven't tested it, the
>>>> patch shouldn't go in.
>>>
>>> I was curious...
>>>
>>> There are two commits in kmod where the cloexec changes were made:
>>>
>>> http://git.profusion.mobi/cgit.cgi/kmod.git/log/?qt=grep&q=cloexec
>>>
>>> The changes were a simple addition of the O_CLOEXEC flag, so this
>>> patch is simply the union of those two commits.  A release of kmod
>>> that doesn't require O_CLOEXEC has the same behaviour as this patch.
>>> The problem O_CLOEXEC is solving isn't possible to solve cleanly
>>> without it.  Using an older version of kmod instead of patching kmod
>>> to work on older systems would result in more bugs and less features
>>> for no win.
>>
>> Was there any conclusion on this? I'm seeing the same problems. This
>> would only effect kmod-native (unless the target was using the older
>> stuff as well which is uncommon at this point).
>
> For what it's worth, we applied this in our layer and things do seem
> to work fine with this applied. It was either this or what we did
> before (reverted the switch to kmod-native and retained
> module-init-tools-cross), as we require CentOS5/RHEL5 support still.

I've been doing something similar and it's been working OK. -  I think
we should apply Ross's patch.

-M

diff --git a/meta/recipes-kernel/kmod/kmod-native_git.bb
b/meta/recipes-kernel/kmod/kmod-native_git.bb
index 96de8b8..054a842 100644
--- a/meta/recipes-kernel/kmod/kmod-native_git.bb
+++ b/meta/recipes-kernel/kmod/kmod-native_git.bb
@@ -4,7 +4,9 @@
 require kmod.inc
 inherit native

-PR = "${INC_PR}.0"
+PR = "${INC_PR}.1"
+
+CFLAGS += "-D O_CLOEXEC=0"

 do_install_append (){
        for tool in depmod insmod lsmod modinfo modprobe rmmod

-M

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Burton, Ross]

On 15 August 2012 20:27, McClintock Matthew-B29882 <B29882@freescale.com> wrote:
> I've been doing something similar and it's been working OK. -  I think
> we should apply Ross's patch.

Radu's, not mine.

Ross

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Radu Moisan]

On 08/15/2012 10:32 PM, Burton, Ross wrote:
> On 15 August 2012 20:27, McClintock Matthew-B29882 <B29882@freescale.com> wrote:
>> I've been doing something similar and it's been working OK. -  I think
>> we should apply Ross's patch.
> Radu's, not mine.
>
> Ross
What's happening with this one? Is it going to me merged or something 
needs to be changed. As I see the comments, it should be fine.

Radu

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Saul Wold]

On 08/29/2012 06:48 AM, Radu Moisan wrote:
>
> On 08/15/2012 10:32 PM, Burton, Ross wrote:
>> On 15 August 2012 20:27, McClintock Matthew-B29882
>> <B29882@freescale.com> wrote:
>>> I've been doing something similar and it's been working OK. -  I think
>>> we should apply Ross's patch.
>> Radu's, not mine.
>>
>> Ross
> What's happening with this one? Is it going to me merged or something
> needs to be changed. As I see the comments, it should be fine.
>
Radu,

I think that Matthew's patch was merged

[OE-core] [PATCH v2] kmod-native_git.bb: fix builds for hosts with older 
libc

Did this not fix this issue?

Sau!


> Radu
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>

Re: [PATCH v2] kmod: Handle undefined O_CLOEXEC

[Radu Moisan]

On 08/29/2012 08:05 PM, Saul Wold wrote:
> On 08/29/2012 06:48 AM, Radu Moisan wrote:
>>
>> On 08/15/2012 10:32 PM, Burton, Ross wrote:
>>> On 15 August 2012 20:27, McClintock Matthew-B29882
>>> <B29882@freescale.com> wrote:
>>>> I've been doing something similar and it's been working OK. -  I think
>>>> we should apply Ross's patch.
>>> Radu's, not mine.
>>>
>>> Ross
>> What's happening with this one? Is it going to me merged or something
>> needs to be changed. As I see the comments, it should be fine.
>>
> Radu,
>
> I think that Matthew's patch was merged
>
> [OE-core] [PATCH v2] kmod-native_git.bb: fix builds for hosts with 
> older libc
>
> Did this not fix this issue?
>
> Sau!
>
>
That's fine Soul, thanks for pointing that out. I just wanted to get it 
of my list of pending patches.

Radu