Re: [LTP] [PATCH v2] mount_setattr02.c: Check mount_setattr attr.propagation

[Wei Gao via ltp <ltp@lists.linux.it>]

On Fri, Feb 21, 2025 at 01:54:58PM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> nit: You did not change commit message subject (we talked about it).
Oh, if you check time of message, i sent v2 patch before we talked about this :)
> 
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> > @@ -0,0 +1,103 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> NOTE: this '[Description]' should no longer be added.
> 
> See:
> https://github.com/linux-test-project/ltp/commit/a3e27df34d6cb49477c9bd6f9bbaa1ce4de155f9
> (it updated also examples in doc)
> 
> and follow ups
> https://github.com/linux-test-project/ltp/commit/63eceaa2a83bca41997edcd649eb62272622a100
> https://github.com/linux-test-project/ltp/commit/824f66ca72fc7505a31c30792334905b646c9d37
Got it, thanks your information.
> > + *
> > + * Basic mount_setattr() test.
> nit: here is better a blank line.
> > + * Test basic propagation mount attributes are set correctly.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +
> > +#include <sys/statvfs.h>
> > +#include "tst_test.h"
> > +#include "lapi/fsmount.h"
> > +#include "tst_safe_stdio.h"
> > +
> > +#define DIRA "/DIRA_PROPAGATION_CHECK"
> 
> Also, I found a way to test into TMPDIR. There is no need to touch a real root.
> It works even on separate /tmp.
> 
> If you agree, I merge with following diff.
> https://github.com/pevik/ltp/blob/wei/mount_setattr02.v2.fixes/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
I agree. Great!
> 
> With changes below.
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> Kind regards,
> Petr
> 
> +++ testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> @@ -4,9 +4,8 @@
>   */
>  
>  /*\
> - * [Description]
> - *
>   * Basic mount_setattr() test.
> + *
>   * Test basic propagation mount attributes are set correctly.
>   */
>  
> @@ -17,9 +16,9 @@
>  #include "lapi/fsmount.h"
>  #include "tst_safe_stdio.h"
>  
> -#define DIRA "/DIRA_PROPAGATION_CHECK"
> +static char *tmpdir;
>  
> -static int dir_created, mounted;
> +static int mounted;
>  
>  static bool is_shared_mount(const char *path)
>  {
> @@ -52,19 +51,15 @@ static bool is_shared_mount(const char *path)
>  static void cleanup(void)
>  {
>  	if (mounted)
> -		SAFE_UMOUNT(DIRA);
> -
> -	if (dir_created)
> -		SAFE_RMDIR(DIRA);
> +		SAFE_UMOUNT(tmpdir);
>  }
>  
>  static void setup(void)
>  {
> -	SAFE_MKDIR(DIRA, 0777);
> +	tmpdir = tst_tmpdir_path();
>  	SAFE_UNSHARE(CLONE_NEWNS);
> -	dir_created = 1;
>  	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> -	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
> +	SAFE_MOUNT("testing", tmpdir, "tmpfs", MS_NOATIME | MS_NODEV, "");
>  	mounted = 1;
>  }
>  
> @@ -75,24 +70,24 @@ static void run(void)
>  		.attr_clr       = MOUNT_ATTR__ATIME,
>  	};
>  
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  
>  	attr.propagation = -1;
> -	TST_EXP_FAIL_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)), EINVAL);
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_FAIL_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)), EINVAL);
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  
>  	attr.propagation = MS_SHARED;
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 1);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 1);
>  
>  	attr.propagation = MS_PRIVATE;
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  
>  	attr.propagation = MS_SLAVE;
> -	TST_EXP_PASS_SILENT(mount_setattr(-1, DIRA, 0, &attr, sizeof(attr)));
> -	TST_EXP_EQ_LI(is_shared_mount(DIRA), 0);
> +	TST_EXP_PASS_SILENT(mount_setattr(-1, tmpdir, 0, &attr, sizeof(attr)));
> +	TST_EXP_EQ_LI(is_shared_mount(tmpdir), 0);
>  }
>  
>  static struct tst_test test = {
> @@ -100,4 +95,5 @@ static struct tst_test test = {
>  	.setup = setup,
>  	.cleanup = cleanup,
>  	.needs_root = 1,
> +	.needs_tmpdir = 1,
>  };

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp