Re: [systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()
On Tue, 07.10.14 14:14, Michal Sekletar (msekl...@redhat.com) wrote: Hence, if a container manager mounts everything properly, then mount_setup() should be a NOP anyway... In theory yes, but in fact not having /run mounted as tmpfs is default in the docker container. I have no strong opinion on whether this is sensible or not, however I think that systemd can be made more resilient and handle such cases. Sorry, but no. /run should be pre-mounted, and if it isn't we need the rights to mount it. We will not boot up a system without /run. That's part of the API for programs, and we will not avoid it. Please ask Docker to premount /run. All distros need /run anyway these days, Debian does, Ubuntu does, Fedora does. Now systemd will try to mount /run on tmpfs, such attempt will fail because of missing capability and then systemd will just hang. Well, just sticking the head in the sand won't help. If we don't have /run mounted, then things will break later on. We cannot ignore that. Sorry, Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()
On 10/08/2014 07:40 AM, Lennart Poettering wrote: On Tue, 07.10.14 14:14, Michal Sekletar (msekl...@redhat.com) wrote: Hence, if a container manager mounts everything properly, then mount_setup() should be a NOP anyway... In theory yes, but in fact not having /run mounted as tmpfs is default in the docker container. I have no strong opinion on whether this is sensible or not, however I think that systemd can be made more resilient and handle such cases. Sorry, but no. /run should be pre-mounted, and if it isn't we need the rights to mount it. We will not boot up a system without /run. That's part of the API for programs, and we will not avoid it. Please ask Docker to premount /run. All distros need /run anyway these days, Debian does, Ubuntu does, Fedora does. Now systemd will try to mount /run on tmpfs, such attempt will fail because of missing capability and then systemd will just hang. Well, just sticking the head in the sand won't help. If we don't have /run mounted, then things will break later on. We cannot ignore that. Sorry, Lennart We have a patch for this. In the past docker has bocked/removed the patch because there is no concept of systemd-tmpfs inside a container to pre-populate /run. So images came with content in their /run. Alex wrote a patch to scan the /run on the image and create the content in a tmpfs /run. I will attempt to push this patch again to docker. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()
On Thu, Oct 02, 2014 at 11:43:22AM +0200, Lennart Poettering wrote: On Thu, 02.10.14 09:57, Michal Sekletar (msekl...@redhat.com) wrote: In cases when we are running as system manager, but we don't have the capability to mount filesystems don't call mount_setup(). However we assume that some directories (e.g. /run/systemd) are always around. Hence don't create those directories in mount_setup(). --- src/core/main.c| 7 ++- src/core/mount-setup.c | 20 src/core/mount-setup.h | 1 + 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 1a62e04..fcd9471 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1393,10 +1393,15 @@ int main(int argc, char *argv[]) { /* Mount /proc, /sys and friends, so that /proc/cmdline and * /proc/$PID/fd is available. */ -if (getpid() == 1) { +if (getpid() == 1 have_effective_cap(CAP_SYS_ADMIN)) { r = mount_setup(loaded_policy); if (r 0) goto finish; Hmm, is this really necessary? I mean, the code in mount_setup() will anyway only mount what is missing, but not overmount what is already mounted. I think it is necessary to make possible to run systemd in a docker container. Hence, if a container manager mounts everything properly, then mount_setup() should be a NOP anyway... In theory yes, but in fact not having /run mounted as tmpfs is default in the docker container. I have no strong opinion on whether this is sensible or not, however I think that systemd can be made more resilient and handle such cases. Now systemd will try to mount /run on tmpfs, such attempt will fail because of missing capability and then systemd will just hang. Michal Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()
In cases when we are running as system manager, but we don't have the capability to mount filesystems don't call mount_setup(). However we assume that some directories (e.g. /run/systemd) are always around. Hence don't create those directories in mount_setup(). --- src/core/main.c| 7 ++- src/core/mount-setup.c | 20 src/core/mount-setup.h | 1 + 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 1a62e04..fcd9471 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1393,10 +1393,15 @@ int main(int argc, char *argv[]) { /* Mount /proc, /sys and friends, so that /proc/cmdline and * /proc/$PID/fd is available. */ -if (getpid() == 1) { +if (getpid() == 1 have_effective_cap(CAP_SYS_ADMIN)) { r = mount_setup(loaded_policy); if (r 0) goto finish; +} else if (getpid() == 1 detect_container(NULL) 0) { +/* Running inside the container as PID 1 but without capability + to mount filesystems. Create at least directories we always + expect to be around */ +mount_setup_run_dirs(); } /* Reset all signal handlers. */ diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index 23a66d2..cd2991d 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -373,6 +373,17 @@ static int nftw_cb( return FTW_CONTINUE; }; +void mount_setup_run_dirs(void) { +/* Create a few directories we always want around, Note that + * sd_booted() checks for /run/systemd/system, so this mkdir + * really needs to stay for good, otherwise software that + * copied sd-daemon.c into their sources will misdetect + * systemd. */ +mkdir_label(/run/systemd, 0755); +mkdir_label(/run/systemd/system, 0755); +mkdir_label(/run/systemd/inaccessible, ); +} + int mount_setup(bool loaded_policy) { int r; unsigned i; @@ -418,14 +429,7 @@ int mount_setup(bool loaded_policy) { if (mount(NULL, /, NULL, MS_REC|MS_SHARED, NULL) 0) log_warning(Failed to set up the root directory for shared mount propagation: %m); -/* Create a few directories we always want around, Note that - * sd_booted() checks for /run/systemd/system, so this mkdir - * really needs to stay for good, otherwise software that - * copied sd-daemon.c into their sources will misdetect - * systemd. */ -mkdir_label(/run/systemd, 0755); -mkdir_label(/run/systemd/system, 0755); -mkdir_label(/run/systemd/inaccessible, ); +mount_setup_run_dirs(); return 0; } diff --git a/src/core/mount-setup.h b/src/core/mount-setup.h index 4b521ad..bfe92b1 100644 --- a/src/core/mount-setup.h +++ b/src/core/mount-setup.h @@ -25,6 +25,7 @@ int mount_setup_early(void); +void mount_setup_run_dirs(void); int mount_setup(bool loaded_policy); int mount_cgroup_controllers(char ***join_controllers); -- 2.0.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()
On Thu, 02.10.14 09:57, Michal Sekletar (msekl...@redhat.com) wrote: In cases when we are running as system manager, but we don't have the capability to mount filesystems don't call mount_setup(). However we assume that some directories (e.g. /run/systemd) are always around. Hence don't create those directories in mount_setup(). --- src/core/main.c| 7 ++- src/core/mount-setup.c | 20 src/core/mount-setup.h | 1 + 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 1a62e04..fcd9471 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1393,10 +1393,15 @@ int main(int argc, char *argv[]) { /* Mount /proc, /sys and friends, so that /proc/cmdline and * /proc/$PID/fd is available. */ -if (getpid() == 1) { +if (getpid() == 1 have_effective_cap(CAP_SYS_ADMIN)) { r = mount_setup(loaded_policy); if (r 0) goto finish; Hmm, is this really necessary? I mean, the code in mount_setup() will anyway only mount what is missing, but not overmount what is already mounted. Hence, if a container manager mounts everything properly, then mount_setup() should be a NOP anyway... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel