Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nspawn: beef up netns checking a bit, for compat with old kernels #10589

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

poettering
Copy link
Member

Fixes: #10544

@evverx
Copy link
Member

evverx commented Oct 31, 2018

If a file referring to a non-network namespace is passed to nspawn, it fails to start (which is good and expected), but the message it prints doesn't seem to be helpful:

# ../../build/systemd-nspawn -D ./CONT/ --network-namespace-path=/proc/1/ns/ipc ip a
Spawning container CONT on /root/systemd-centos-ci/systemd/test/TEST-13-NSPAWN-SMOKE/CONT.
Press ^] three times within 1s to kill container.
Failed to register machine: Interrupted system call

I'm not sure what to do about it. It could be fixed or we could probably safely assume that people who try to run containers on CentOS with nspawn know what they're doing and leave everything as is :-)

@poettering
Copy link
Member Author

I'm not sure what to do about it. It could be fixed or we could probably safely assume that people who try to run containers on CentOS with nspawn know what they're doing and leave everything as is :-)

hmm, is that on an old kernel or a new one?

i am a bit surprised about the EINTR i must say... can you strace -f -s 500 -o log that and paste the output somewhere please?

@evverx
Copy link
Member

evverx commented Oct 31, 2018

strace slows everything down so when run under it nspawn fails with

Spawning container CONT on /root/systemd-centos-ci/systemd/test/TEST-13-NSPAWN-SMOKE/CONT.
Press ^] three times within 1s to kill container.
Child died too early.

The most important part is probably that setns fails and the process exits without logging anything:

2996  setns(22<ipc:[4026531839]>, CLONE_NEWNET <unfinished ...>
2994  recvmsg(11<UNIX:[32416->32423]>,  <unfinished ...>
2996  <... setns resumed> )             = -1 EINVAL (Invalid argument)
2994  <... recvmsg resumed> {msg_name(0)=NULL, msg_iov(1)=[{":1.84\0\0\0\5\1u\0\2\0\0\0\7\1s\0\24\0\0\0org.freedesktop.DBus\0\0\0\0", 48}], msg_controllen=0, msg_flags=MSG_CMSG_CLOEXEC}, MSG_DONTWAIT|MSG_CMSG_CLOEXEC) = 48
2996  close(5<UNIX:[32402,"/run/systemd/nspawn/notify"]> <unfinished ...>
2994  recvmsg(11<UNIX:[32416->32423]>,  <unfinished ...>
2996  <... close resumed> )             = 0
2994  <... recvmsg resumed> {msg_name(0)=NULL, msg_iov(1)=[{"l\2\1\1+\0\0\0A\1\0\0.\0\0\0\5\1u\0\3\0\0\0", 24}], msg_controllen=0, msg_flags=MSG_CMSG_CLOEXEC}, MSG_DONTWAIT|MSG_CMSG_CLOEXEC) = 24
2996  exit_group(1)                     = ?
2994  recvmsg(11<UNIX:[32416->32423]>,  <unfinished ...>
2996  +++ exited with 1 +++

@evverx
Copy link
Member

evverx commented Oct 31, 2018

Forgot to write what I'm using. It's CentOS 7 with Linux 3.10.0-862.14.4.el7.x86_64.

@poettering
Copy link
Member Author

ok, thanks, we can definitely improve that error logging in that case...

@mrc0mmand
Copy link
Member

Both testsuites (nspawn and QEMU) finally pass on CentOS 7 with this PR (full TEST-13 log for reference).

Btw, if you have any idea how to speed-up the CentOS ticket process so the testing on CentOS is finally fully automated, I'd be really glad to know.

@poettering
Copy link
Member Author

@evverx i posted #10591 for you, ptal

@evverx
Copy link
Member

evverx commented Oct 31, 2018

Btw, if you have any idea how to speed-up the CentOS ticket process so the testing on CentOS is finally fully automated, I'd be really glad to know.

A full disclosure: I have no idea :-), but maybe it would make sense to ask the previous maintainer how long it took to get access to the testing infrastructure and whether it's possible to speed up the process a little bit. Unfortunately, I don't know who was the previous maintainer either.

@evverx evverx merged commit 6619ad8 into systemd:master Oct 31, 2018
@@ -67,11 +67,26 @@ static void test_path_is_temporary_fs(void) {
assert_se(path_is_temporary_fs("/i-dont-exist") == -ENOENT);
}

static void test_fd_is_network_ns(void) {
_cleanup_close_ int fd = -1;
assert_se(fd_is_network_ns(STDIN_FILENO) == 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope nobody is going to run ./build/test-stat-util </proc/1/ns/net :-)

keszybz pushed a commit to keszybz/systemd-stable that referenced this pull request Dec 17, 2018
keszybz pushed a commit to keszybz/systemd-stable that referenced this pull request Dec 17, 2018
keszybz pushed a commit to systemd/systemd-stable that referenced this pull request Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants