build, enter: Fixes for old centos #1529
Conversation
| @@ -27,6 +27,12 @@ | |||
| #include <sys/wait.h> | |||
| #include <unistd.h> | |||
|
|
|||
| #ifdef USE_NR_SETNS_MACRO_INSTEAD | |||
There was a problem hiding this comment.
Would it be better to use
#ifndef setns
#include <linux/unistd.h>
static int setns(int fd, int nstype) {
return syscall(__NR_setns, fd, nstype);
}
#endif
instead of declaring USE_NR_SETNS_MACRO_INSTEAD in configure.ac.
Then, no need to modify the call sites of setns()
There was a problem hiding this comment.
That won't work - preprocessor does not tell you if the function is defined or not.
What I could do is to use your code, but replace the #ifndef setns with #USE_NR_SETNS_MACRO_INSTEAD. This macro is defined only when we don't have setns function.
|
/cc @jaybuff |
|
How about not using setns from glibc at all? It's only used in one place and the "if not defined define it" logic is odd. Let's just have our own setns and use it all the time. It's a one line function. |
|
I'm not sure if going for the lowest common denominator is the way to go. I would rather insist on dropping support for too old distros (there is already CentOS 7.0 with glibc 2.17 and autoconf 2.69 released almost half year ago), so I would consider adding a support for CentOS 6.7 a rather exceptional situation. I just don't see too much sense in making new version of software running on some really old software (5 years since glibc 2.12 or 7 years since autoconf 2.63 were released are a lot of time). True, your suggestion would make the fix easier/shorter, but I'd prefer to avoid following that way. |
|
@krnowak are you suggesting dropping this PR and officially not supporting builds on centos 6.x? IMHO replacing |
|
No, we are definitely going to try to support builds on CentOS 6.x On Mon, Oct 5, 2015 at 2:00 PM, Jay Buffington notifications@github.com
|
|
+1 for the simple always use our own setns syscall wrapper approach, no need for the added complexity of detecting the glibc wrapper. |
Centos 6.7 has buggy autoconf/m4/whatever - it is getting confused with empty conditional bodies preceded with m4 comments, which results in generating shell code with empty conditional bodies, which in turn is an invalid code. Just put some colons (which serve as an explicit noop in shell) to be safe. Also, lower autoconf required version.
Another fix for CentOS 6.7 - setns function was added to glibc 2.14, but CentOS has only 2.12.
|
I'm against the said simple solution - I want compatibility stuff to stick out like a sore thumb. I updated the PR, so it does not modify setns call site. |
|
I think the solution implemented now is simple enough. |
|
Anyway, |
|
@jaybuff hmm, were you able to successfully run with this? |
|
I was able to successfully build this (help reports version 0.8.1+gita65939d) and rkt run succeeded for me. I was under the impression that the |
|
OK, I'm going to land this and we can revisit if there are runtime issues. |
build, enter: Fixes for old centos
|
👍 |
|
I'm not sure either, why I got the error. Anyway, I tried mounting it manually and got basically the same error. It's probably because the stock kernel in CentOS is quite ancient (2.6.32). On some howtos about running Docker in CentOS 6 there was a hint about using newer kernel (3.10 or something). |
|
Oh, yes, I am using a 3.18 kernel with CentOS 6u5, so that would explain it. |
This fixes generation of invalid shell script by autotools and the lack of setns function.
Fixes #1443.