Re: [systemd-devel] [PATCH 2/4] mount-setup: introduce mount_setup_run_dirs()

2014-10-08 Thread Lennart Poettering
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()

2014-10-08 Thread Daniel J Walsh

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()

2014-10-07 Thread Michal Sekletar
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()

2014-10-02 Thread Michal Sekletar
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()

2014-10-02 Thread Lennart Poettering
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