[systemd-devel] [PATCH v2] Do not clear parent mount flags when setting up namespaces

[Djalal Harouni]

Hi,

On Thu, Jan 01, 2015 at 10:36:39PM +0200, Topi Miettinen wrote:
> Copy parent directory mount flags when setting up a namespace and
> don't accidentally clear mount flags later.
As noted by Colin in the other email, there should be a git log message
for why the change.

Yes thank you, I see that in one of the replies of v1 of the patch you
say why, so just perhaps use it in the commit log and code comment ?


> ---
>  src/core/namespace.c |  4 ++--
>  src/shared/util.c    | 19 +++++++++++++++++--
>  src/shared/util.h    |  2 ++
>  3 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/src/core/namespace.c b/src/core/namespace.c
> index 4b8dbdd..6859b6a 100644
> --- a/src/core/namespace.c
> +++ b/src/core/namespace.c
> @@ -159,7 +159,7 @@ static int mount_dev(BindMount *m) {
>  
>          dev = strappenda(temporary_mount, "/dev");
>          (void)mkdir(dev, 0755);
> -        if (mount("tmpfs", dev, "tmpfs", MS_NOSUID|MS_STRICTATIME, "mode=755") < 0) {
> +        if (mount("tmpfs", dev, "tmpfs", get_mount_flags("/dev")|MS_NOSUID|MS_STRICTATIME, "mode=755") < 0) {
There is no need for this function to be a parameter


>                  r = -errno;
>                  goto fail;
>          }
> @@ -282,7 +282,7 @@ static int mount_kdbus(BindMount *m) {
>  
>          root = strappenda(temporary_mount, "/kdbus");
>          (void)mkdir(root, 0755);
> -        if (mount("tmpfs", root, "tmpfs", MS_NOSUID|MS_STRICTATIME, "mode=777") < 0) {
> +        if (mount("tmpfs", root, "tmpfs", get_mount_flags("/sys/fs/kdbus")|MS_NOSUID|MS_STRICTATIME, "mode=777") < 0) {
>                  r = -errno;
>                  goto fail;
>          }
> diff --git a/src/shared/util.c b/src/shared/util.c
> index dfaf7f7..8ff5073 100644
> --- a/src/shared/util.c
> +++ b/src/shared/util.c
> @@ -61,6 +61,7 @@
>  #include <sys/personality.h>
>  #include <sys/xattr.h>
>  #include <libgen.h>
> +#include <sys/statvfs.h>
>  #undef basename
>  
>  #ifdef HAVE_SYS_AUXV_H
> @@ -6858,6 +6859,15 @@ int umount_recursive(const char *prefix, int flags) {
>          return r ? r : n;
>  }
>  
> +unsigned long get_mount_flags(const char *path) {
> +        struct statvfs buf;
> +
> +        if (statvfs(path, &buf) < 0)
> +                return 0;
IMO here it should return an errno since this is a helper. In that case
perhaps just open code the statvfs() or improve the helper ?

Thanks!

-- 
Djalal Harouni
http://opendz.org