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

[Topi Miettinen]

On 01/02/15 23:53, Djalal Harouni wrote:
> 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 ?

I'll make it return errno, for general use it's of course good to know
if there was a problem.

> 
> Thanks!
>