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

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

On Tue, Feb 18, 2025 at 04:18:58PM +0100, Petr Vorel wrote:
> Hi Wei,
> 
> nit: I guess you want to replace dot with space in subject.
You mean i s/attr.propagation/attr propagation ?
> 
> > +++ b/testcases/kernel/syscalls/mount_setattr/mount_setattr02.c
> > @@ -0,0 +1,102 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2025 SUSE LLC Wei Gao <wegao@suse.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * Basic mount_setattr() test.
> > + * 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"
> 
> Is it necessary to to use directory under root?
Yes. Otherwise failed will happen during mount_setattr.
But i have not check for detail.
> 
> ...
> > +static void cleanup(void)
> > +{
> 
> I guess this is due result of:
> SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> There should be either a proper detection whether this works or 
> 
> ../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
> mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)
> 
> 
> > +	SAFE_RMDIR(DIRA);
> 
> When running on old kernel (e.g. SLES based 4.12) it fails due TCONF:
> 
> ../../../../include/lapi/fsmount.h:197: TCONF: Test not supported on kernel version < v5.2
> mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: ENOENT (2)
> 
> There should be a flag to remove dir only when it was created.
> 
> > +
> nit: please remove this new line (I have to keep asking this :( ).
Sorry, i need pay attention, thanks for point this out with great patient :).
> > +}
> > +
> > +static void setup(void)
> > +{
> > +	fsopen_supported_by_kernel();
> I wonder if this needed for detecting new mount API support. Because second
> SAFE_MOUNT also runs code which detects code unsupported:
> 
> ../../../../include/lapi/fsmount.h:113: TCONF: syscall(442) __NR_mount_setattr not supported on your arch
> mount_setattr02.c:52: TWARN: rmdir(/DIRA_PROPAGATION_CHECK) failed: EBUSY (16)
> 
> But I have no idea what would be needed to be done to cleanup result of the
> first SAFE_MOUNT().
> 
> > +
> > +	SAFE_MKDIR(DIRA, 0777);
> > +}
> > +
> > +static void run(void)
> > +{
> > +
> and here new line.
> > +	SAFE_UNSHARE(CLONE_NEWNS);
> > +	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> > +	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
> Do these 2 needs to be in the run()? How about move them to setup()?
> 
> static int dir_created, mounted;
> 
> static void setup(void)
> {
> 	fsopen_supported_by_kernel();
> 
> 	SAFE_MKDIR(DIRA, 0777);
> 	dir_created = 1;
> 	SAFE_UNSHARE(CLONE_NEWNS);
> 	SAFE_MOUNT(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0);
> 	SAFE_MOUNT("testing", DIRA, "tmpfs", MS_NOATIME | MS_NODEV, "");
> 	mounted = 1;
> }
> 
> static void cleanup(void)
> {
> 	if (mounted)
> 		SAFE_UMOUNT(DIRA);
> 
> 	if (dir_created)
> 		SAFE_RMDIR(DIRA);
> }
> 
> + I later create generic helper from is_shared_mount().
I sent v2 patch now, once your create generic helper i can do another patch together with
your generic helper.
> 
> Kind regards,
> Petr
> 

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