diff mbox series

fanotify01: Test setting two marks on different filesystems

Message ID 20240125110510.751445-1-amir73il@gmail.com
State Accepted
Headers show
Series fanotify01: Test setting two marks on different filesystems | expand

Commit Message

Amir Goldstein Jan. 25, 2024, 11:05 a.m. UTC
When tested fs has zero fsid (e.g. fuse) and events are reported
with fsid+fid, watching different filesystems with the same group is
not supported.

Try to setup a bogus mark on test tmp dir, in addition to the mark
on the tested filesystem, to check if marks on different filesystems
are supported.

Run on all filesystem to test both fs with zero fsid (e.g. fuse) and
fs with valid fsid.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Petr,

The fanotify tests changes that you already picked from my github [1]
have to do with fixing new test failures in v6.7 and v6.8-rc1, dues to
the fact that each of those kernel versions added new filesystems that
support fanotify events with fid+fsid.

This is the only change to test new functionality in v6.8-rc1, namely,
that for fs with zero fsid (e.g. fuse), an fanotify group can watch a
single fs instance.

To fix the problem that you reported with this test on exfat [2],
I needed to make a distiction between the fs that do not support mount
mark with fid due to having zero fsid (e.g. fuse) and those fs that
do not support decoding fid (e.g. exfat).

It is not urgent to merge this change to the upcoming code freeze -
it's up to you, but since I already tested it I am posting it now.

Thanks,
Amir.

[1] https://github.com/amir73il/ltp/commits/fanotify_fsid/
[2] https://lore.kernel.org/ltp/CAOQ4uxh1VwoMK_ssjdcxo_sk4cw0pD_FcXZ6Lb2=XHLf21kGAw@mail.gmail.com/T/#mf15d751e8f77a497ee4387b0791219e800cde7ea

 testcases/kernel/syscalls/fanotify/fanotify.h |  6 +++++-
 .../kernel/syscalls/fanotify/fanotify01.c     | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

Comments

Petr Vorel Jan. 25, 2024, 12:23 p.m. UTC | #1
> When tested fs has zero fsid (e.g. fuse) and events are reported
> with fsid+fid, watching different filesystems with the same group is
> not supported.

> Try to setup a bogus mark on test tmp dir, in addition to the mark
> on the tested filesystem, to check if marks on different filesystems
> are supported.

> Run on all filesystem to test both fs with zero fsid (e.g. fuse) and
> fs with valid fsid.

> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

Hi Amir,

> Petr,

> The fanotify tests changes that you already picked from my github [1]
> have to do with fixing new test failures in v6.7 and v6.8-rc1, dues to
> the fact that each of those kernel versions added new filesystems that
> support fanotify events with fid+fsid.

> This is the only change to test new functionality in v6.8-rc1, namely,
> that for fs with zero fsid (e.g. fuse), an fanotify group can watch a
> single fs instance.

This patch would have to be rebased, to be applicable of the previous one (which
touches more tests) in your branch.

I also wonder what I did wrong, the branch works after reboot, tested on:
1) mainline kernel 6.8.0-rc1 in rapido-linux
2) openSUSE kernel 6.8.0-rc1-2.gf4ba5db-default
3) 6.7.0-9.gaedda80-default

> To fix the problem that you reported with this test on exfat [2],
> I needed to make a distiction between the fs that do not support mount
> mark with fid due to having zero fsid (e.g. fuse) and those fs that
> do not support decoding fid (e.g. exfat).

> It is not urgent to merge this change to the upcoming code freeze -
> it's up to you, but since I already tested it I am posting it now.

I'll probably merge it after LTP release.

Kind regards,
Petr

> Thanks,
> Amir.

> [1] https://github.com/amir73il/ltp/commits/fanotify_fsid/
> [2] https://lore.kernel.org/ltp/CAOQ4uxh1VwoMK_ssjdcxo_sk4cw0pD_FcXZ6Lb2=XHLf21kGAw@mail.gmail.com/T/#mf15d751e8f77a497ee4387b0791219e800cde7ea

>  testcases/kernel/syscalls/fanotify/fanotify.h |  6 +++++-
>  .../kernel/syscalls/fanotify/fanotify01.c     | 19 +++++++++++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)

> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index e0d178bcc..554940a7e 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -166,20 +166,23 @@ static inline int fanotify_flags_supported_on_fs(unsigned int init_flags,
>  {
>  	int fd;
>  	int rval = 0;
> +	int err = 0;

>  	fd = fanotify_init(init_flags, O_RDONLY);
> -
>  	if (fd < 0) {
> +		err = errno;
>  		if (errno == ENOSYS)
>  			tst_brk(TCONF, "fanotify not configured in kernel");
>  		if (errno != EINVAL)
>  			tst_brk(TBROK | TERRNO,
>  				"fanotify_init(%x, O_RDONLY) failed",
>  				init_flags);
> +		errno = err;
>  		return -1;
>  	}

>  	if (fname && fanotify_mark(fd, FAN_MARK_ADD | mark_flags, event_flags, AT_FDCWD, fname) < 0) {
> +		err = errno;
>  		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
>  			rval = strcmp(fname, OVL_MNT) ? -2 : -3;
>  		} else if (errno != EINVAL) {
> @@ -194,6 +197,7 @@ static inline int fanotify_flags_supported_on_fs(unsigned int init_flags,

>  	SAFE_CLOSE(fd);

> +	errno = err;
>  	return rval;
>  }

> diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
> index e4398f236..ba09f309d 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify01.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> @@ -77,6 +77,7 @@ static char buf[BUF_SIZE];
>  static int fd_notify;
>  static int fan_report_fid_unsupported;
>  static int mount_mark_fid_unsupported;
> +static int inode_mark_fid_xdev;
>  static int filesystem_mark_unsupported;

>  static unsigned long long event_set[EVENT_MAX];
> @@ -328,6 +329,17 @@ pass:

>  	}

> +
> +	/*
> +	 * Try to setup a bogus mark on test tmp dir, to check if marks on
> +	 * different filesystems are supported.
> +	 * When tested fs has zero fsid (e.g. fuse) and events are reported
> +	 * with fsid+fid, watching different filesystems is not supported.
> +	 */
> +	ret = report_fid ? inode_mark_fid_xdev : 0;
> +	TST_EXP_FD_OR_FAIL(fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_CLOSE_WRITE,
> +					 AT_FDCWD, "."), ret);
> +
>  	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
>  	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | mark->flag,
>  			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> @@ -352,6 +364,12 @@ static void setup(void)
>  	mount_mark_fid_unsupported = fanotify_flags_supported_on_fs(FAN_REPORT_FID,
>  								    FAN_MARK_MOUNT,
>  								    FAN_OPEN, fname);
> +	/* When mount mark is not supported due to zero fsid, multi fs inode marks are not supported */
> +	if (mount_mark_fid_unsupported && errno == ENODEV) {
> +		tst_res(TINFO, "filesystem %s does not support reporting events with fid from multi fs",
> +				tst_device->fs_type);
> +		inode_mark_fid_xdev = EXDEV;
> +	}
>  }

>  static void cleanup(void)
> @@ -368,6 +386,7 @@ static struct tst_test test = {
>  	.needs_root = 1,
>  	.mount_device = 1,
>  	.mntpoint = MOUNT_PATH,
> +	.all_filesystems = 1,
>  };

>  #else
Amir Goldstein Jan. 25, 2024, 1:45 p.m. UTC | #2
On Thu, Jan 25, 2024 at 2:23 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > When tested fs has zero fsid (e.g. fuse) and events are reported
> > with fsid+fid, watching different filesystems with the same group is
> > not supported.
>
> > Try to setup a bogus mark on test tmp dir, in addition to the mark
> > on the tested filesystem, to check if marks on different filesystems
> > are supported.
>
> > Run on all filesystem to test both fs with zero fsid (e.g. fuse) and
> > fs with valid fsid.
>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
>
> Hi Amir,
>
> > Petr,
>
> > The fanotify tests changes that you already picked from my github [1]
> > have to do with fixing new test failures in v6.7 and v6.8-rc1, dues to
> > the fact that each of those kernel versions added new filesystems that
> > support fanotify events with fid+fsid.
>
> > This is the only change to test new functionality in v6.8-rc1, namely,
> > that for fs with zero fsid (e.g. fuse), an fanotify group can watch a
> > single fs instance.
>
> This patch would have to be rebased, to be applicable of the previous one (which
> touches more tests) in your branch.

I don't understand - this patch is already rebased.
I have it in my branch after the fix patches:

7db2dac9f (HEAD -> fanotify_fsid) fanotify01: Test setting two marks
on different filesystems
f80dc512e fanotify{01,09,10}: Check for report fid support with mount mark
9062824a7 (master) fanotify16: Fix test failure on FUSE
ea085713f fanotify{14,15,16}: Check for filesystem mark support on filesystem
921f0ce86 fanotify13: Use generic check for mark type support on filesystem

>
> I also wonder what I did wrong, the branch works after reboot, tested on:
> 1) mainline kernel 6.8.0-rc1 in rapido-linux
> 2) openSUSE kernel 6.8.0-rc1-2.gf4ba5db-default
> 3) 6.7.0-9.gaedda80-default
>

Which branch works? LTP master branch?

My claim is that with the current LTP master branch, fanotify01,09,10
would fail on kernel 6.8.0-rc1 when running with
LTP_DEV_FS_TYPE=exfat (not LTP_SINGLE_FS_TYPE=exfat)

So at least you should consider applying this fix for the release:
f80dc512e fanotify{01,09,10}: Check for report fid support with mount mark

Thanks,
Amir.
Petr Vorel Jan. 25, 2024, 2:44 p.m. UTC | #3
> On Thu, Jan 25, 2024 at 2:23 PM Petr Vorel <pvorel@suse.cz> wrote:

> > > When tested fs has zero fsid (e.g. fuse) and events are reported
> > > with fsid+fid, watching different filesystems with the same group is
> > > not supported.

> > > Try to setup a bogus mark on test tmp dir, in addition to the mark
> > > on the tested filesystem, to check if marks on different filesystems
> > > are supported.

> > > Run on all filesystem to test both fs with zero fsid (e.g. fuse) and
> > > fs with valid fsid.

> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---

> > Hi Amir,

> > > Petr,

> > > The fanotify tests changes that you already picked from my github [1]
> > > have to do with fixing new test failures in v6.7 and v6.8-rc1, dues to
> > > the fact that each of those kernel versions added new filesystems that
> > > support fanotify events with fid+fsid.

> > > This is the only change to test new functionality in v6.8-rc1, namely,
> > > that for fs with zero fsid (e.g. fuse), an fanotify group can watch a
> > > single fs instance.

> > This patch would have to be rebased, to be applicable of the previous one (which
> > touches more tests) in your branch.

> I don't understand - this patch is already rebased.
I meant, you sent just this 2nd commit, without 1st commit
f80dc512e ("fanotify{01,09,10}: Check for report fid support with mount mark")
Therefore this commit is not applicable. I got just confused you send only
this commit with without the previous one...

> I have it in my branch after the fix patches:

> 7db2dac9f (HEAD -> fanotify_fsid) fanotify01: Test setting two marks
> on different filesystems
> f80dc512e fanotify{01,09,10}: Check for report fid support with mount mark
> 9062824a7 (master) fanotify16: Fix test failure on FUSE
> ea085713f fanotify{14,15,16}: Check for filesystem mark support on filesystem
> 921f0ce86 fanotify13: Use generic check for mark type support on filesystem


> > I also wonder what I did wrong, the branch works after reboot, tested on:
> > 1) mainline kernel 6.8.0-rc1 in rapido-linux
> > 2) openSUSE kernel 6.8.0-rc1-2.gf4ba5db-default
> > 3) 6.7.0-9.gaedda80-default


> Which branch works? LTP master branch?

I meant HEAD of your fanotify_fsid branch.

> My claim is that with the current LTP master branch, fanotify01,09,10
> would fail on kernel 6.8.0-rc1 when running with
> LTP_DEV_FS_TYPE=exfat (not LTP_SINGLE_FS_TYPE=exfat)

I'm sorry, I overlooked the different variable.
I see LTP_DEV_FS_TYPE=exfat gets EOPNOTSUPP on LTP master on 6.8.0-rc1:
fanotify01.c:106: TBROK: fanotify_mark(3, 0x11, 0x3b, ..., fs_mnt/tfile_4503) failed: EOPNOTSUPP (95)

> So at least you should consider applying this fix for the release:
> f80dc512e fanotify{01,09,10}: Check for report fid support with mount mark

I see. Yes, I reproduced the problem and thus cherry-picked and merged this one
as bae8ec9a3.

Although this patch (the 2nd commit) should also work everywhere, I'll backport
it just after the release.

Thanks a lot for explanation and sorry for the confusion.

Kind regards,
Petr

> Thanks,
> Amir.
Amir Goldstein Jan. 26, 2024, 7:38 a.m. UTC | #4
On Thu, Jan 25, 2024 at 4:44 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > On Thu, Jan 25, 2024 at 2:23 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > > When tested fs has zero fsid (e.g. fuse) and events are reported
> > > > with fsid+fid, watching different filesystems with the same group is
> > > > not supported.
>
> > > > Try to setup a bogus mark on test tmp dir, in addition to the mark
> > > > on the tested filesystem, to check if marks on different filesystems
> > > > are supported.
>
> > > > Run on all filesystem to test both fs with zero fsid (e.g. fuse) and
> > > > fs with valid fsid.
>
> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > ---
>
> > > Hi Amir,
>
> > > > Petr,
>
> > > > The fanotify tests changes that you already picked from my github [1]
> > > > have to do with fixing new test failures in v6.7 and v6.8-rc1, dues to
> > > > the fact that each of those kernel versions added new filesystems that
> > > > support fanotify events with fid+fsid.
>
> > > > This is the only change to test new functionality in v6.8-rc1, namely,
> > > > that for fs with zero fsid (e.g. fuse), an fanotify group can watch a
> > > > single fs instance.
>
> > > This patch would have to be rebased, to be applicable of the previous one (which
> > > touches more tests) in your branch.
>
> > I don't understand - this patch is already rebased.
> I meant, you sent just this 2nd commit, without 1st commit
> f80dc512e ("fanotify{01,09,10}: Check for report fid support with mount mark")
> Therefore this commit is not applicable. I got just confused you send only
> this commit with without the previous one...
>
> > I have it in my branch after the fix patches:
>
> > 7db2dac9f (HEAD -> fanotify_fsid) fanotify01: Test setting two marks
> > on different filesystems
> > f80dc512e fanotify{01,09,10}: Check for report fid support with mount mark
> > 9062824a7 (master) fanotify16: Fix test failure on FUSE
> > ea085713f fanotify{14,15,16}: Check for filesystem mark support on filesystem
> > 921f0ce86 fanotify13: Use generic check for mark type support on filesystem
>
>
> > > I also wonder what I did wrong, the branch works after reboot, tested on:
> > > 1) mainline kernel 6.8.0-rc1 in rapido-linux
> > > 2) openSUSE kernel 6.8.0-rc1-2.gf4ba5db-default
> > > 3) 6.7.0-9.gaedda80-default
>
>
> > Which branch works? LTP master branch?
>
> I meant HEAD of your fanotify_fsid branch.
>
> > My claim is that with the current LTP master branch, fanotify01,09,10
> > would fail on kernel 6.8.0-rc1 when running with
> > LTP_DEV_FS_TYPE=exfat (not LTP_SINGLE_FS_TYPE=exfat)
>
> I'm sorry, I overlooked the different variable.
> I see LTP_DEV_FS_TYPE=exfat gets EOPNOTSUPP on LTP master on 6.8.0-rc1:
> fanotify01.c:106: TBROK: fanotify_mark(3, 0x11, 0x3b, ..., fs_mnt/tfile_4503) failed: EOPNOTSUPP (95)
>
> > So at least you should consider applying this fix for the release:
> > f80dc512e fanotify{01,09,10}: Check for report fid support with mount mark
>
> I see. Yes, I reproduced the problem and thus cherry-picked and merged this one
> as bae8ec9a3.
>
> Although this patch (the 2nd commit) should also work everywhere, I'll backport
> it just after the release.

Sounds good.
Better also wait for ACK from Jan on this new test.

>
> Thanks a lot for explanation and sorry for the confusion.
>

No worries.

The change of behavior that many fs started supporting fanotify
with fid events was a bit challenging for those tests.

Sorry for not foreseeing this outcome. Glad it all worked out.

Thanks,
Amir.
Jan Kara Jan. 26, 2024, 11:44 a.m. UTC | #5
On Thu 25-01-24 13:05:10, Amir Goldstein wrote:
> When tested fs has zero fsid (e.g. fuse) and events are reported
> with fsid+fid, watching different filesystems with the same group is
> not supported.
> 
> Try to setup a bogus mark on test tmp dir, in addition to the mark
> on the tested filesystem, to check if marks on different filesystems
> are supported.
> 
> Run on all filesystem to test both fs with zero fsid (e.g. fuse) and
> fs with valid fsid.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

The test looks good to me so feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> 
> Petr,
> 
> The fanotify tests changes that you already picked from my github [1]
> have to do with fixing new test failures in v6.7 and v6.8-rc1, dues to
> the fact that each of those kernel versions added new filesystems that
> support fanotify events with fid+fsid.
> 
> This is the only change to test new functionality in v6.8-rc1, namely,
> that for fs with zero fsid (e.g. fuse), an fanotify group can watch a
> single fs instance.
> 
> To fix the problem that you reported with this test on exfat [2],
> I needed to make a distiction between the fs that do not support mount
> mark with fid due to having zero fsid (e.g. fuse) and those fs that
> do not support decoding fid (e.g. exfat).
> 
> It is not urgent to merge this change to the upcoming code freeze -
> it's up to you, but since I already tested it I am posting it now.
> 
> Thanks,
> Amir.
> 
> [1] https://github.com/amir73il/ltp/commits/fanotify_fsid/
> [2] https://lore.kernel.org/ltp/CAOQ4uxh1VwoMK_ssjdcxo_sk4cw0pD_FcXZ6Lb2=XHLf21kGAw@mail.gmail.com/T/#mf15d751e8f77a497ee4387b0791219e800cde7ea
> 
>  testcases/kernel/syscalls/fanotify/fanotify.h |  6 +++++-
>  .../kernel/syscalls/fanotify/fanotify01.c     | 19 +++++++++++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index e0d178bcc..554940a7e 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -166,20 +166,23 @@ static inline int fanotify_flags_supported_on_fs(unsigned int init_flags,
>  {
>  	int fd;
>  	int rval = 0;
> +	int err = 0;
>  
>  	fd = fanotify_init(init_flags, O_RDONLY);
> -
>  	if (fd < 0) {
> +		err = errno;
>  		if (errno == ENOSYS)
>  			tst_brk(TCONF, "fanotify not configured in kernel");
>  		if (errno != EINVAL)
>  			tst_brk(TBROK | TERRNO,
>  				"fanotify_init(%x, O_RDONLY) failed",
>  				init_flags);
> +		errno = err;
>  		return -1;
>  	}
>  
>  	if (fname && fanotify_mark(fd, FAN_MARK_ADD | mark_flags, event_flags, AT_FDCWD, fname) < 0) {
> +		err = errno;
>  		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
>  			rval = strcmp(fname, OVL_MNT) ? -2 : -3;
>  		} else if (errno != EINVAL) {
> @@ -194,6 +197,7 @@ static inline int fanotify_flags_supported_on_fs(unsigned int init_flags,
>  
>  	SAFE_CLOSE(fd);
>  
> +	errno = err;
>  	return rval;
>  }
>  
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
> index e4398f236..ba09f309d 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify01.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> @@ -77,6 +77,7 @@ static char buf[BUF_SIZE];
>  static int fd_notify;
>  static int fan_report_fid_unsupported;
>  static int mount_mark_fid_unsupported;
> +static int inode_mark_fid_xdev;
>  static int filesystem_mark_unsupported;
>  
>  static unsigned long long event_set[EVENT_MAX];
> @@ -328,6 +329,17 @@ pass:
>  
>  	}
>  
> +
> +	/*
> +	 * Try to setup a bogus mark on test tmp dir, to check if marks on
> +	 * different filesystems are supported.
> +	 * When tested fs has zero fsid (e.g. fuse) and events are reported
> +	 * with fsid+fid, watching different filesystems is not supported.
> +	 */
> +	ret = report_fid ? inode_mark_fid_xdev : 0;
> +	TST_EXP_FD_OR_FAIL(fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_CLOSE_WRITE,
> +					 AT_FDCWD, "."), ret);
> +
>  	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
>  	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | mark->flag,
>  			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> @@ -352,6 +364,12 @@ static void setup(void)
>  	mount_mark_fid_unsupported = fanotify_flags_supported_on_fs(FAN_REPORT_FID,
>  								    FAN_MARK_MOUNT,
>  								    FAN_OPEN, fname);
> +	/* When mount mark is not supported due to zero fsid, multi fs inode marks are not supported */
> +	if (mount_mark_fid_unsupported && errno == ENODEV) {
> +		tst_res(TINFO, "filesystem %s does not support reporting events with fid from multi fs",
> +				tst_device->fs_type);
> +		inode_mark_fid_xdev = EXDEV;
> +	}
>  }
>  
>  static void cleanup(void)
> @@ -368,6 +386,7 @@ static struct tst_test test = {
>  	.needs_root = 1,
>  	.mount_device = 1,
>  	.mntpoint = MOUNT_PATH,
> +	.all_filesystems = 1,
>  };
>  
>  #else
> -- 
> 2.34.1
>
Petr Vorel Jan. 30, 2024, 1:07 p.m. UTC | #6
Hi Amir, Jan,

I was going to merge, but I suspect this does not work on TMPDIR on btrfs.

Tested on SLE 15-SP6 (kernel 6.4), on 15-SP4 (kernel 5.14), Tumbleweed (kernel
6.8.0-rc1), Alpine Linux (kernel 6.4) I get:

fanotify01.c:341: TFAIL: fanotify_mark(fd_notify, 0x00000001, 0x00000008, -100, ".") failed: EXDEV (18)

for tests #3, #4 and #5 on all filesystems.

Testing on other on other filesystem it works: Debian kernel 5.10, 6.1 on ext4,
Alpine Linux kernel 6.4 on tmpfs, Tumbleweed kernel 6.8.0-rc1 on tmpfs.

Should be btrfs handled differently or skipped? (below)
Or test EXDEV for #3, #4 and #5? (not sure how handle just half of the tests on
btrfs differently).

Kind regards,
Petr

diff --git testcases/kernel/syscalls/fanotify/fanotify01.c testcases/kernel/syscalls/fanotify/fanotify01.c
index ba09f309d..97ade1829 100644
--- testcases/kernel/syscalls/fanotify/fanotify01.c
+++ testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -335,8 +335,15 @@ pass:
 	 * different filesystems are supported.
 	 * When tested fs has zero fsid (e.g. fuse) and events are reported
 	 * with fsid+fid, watching different filesystems is not supported.
+	 * Not supported on Btrfs.
 	 */
+	if (tst_fs_type(".") == TST_BTRFS_MAGIC) {
+		tst_res(TCONF, "skipped on Btrfs");
+		return;
+	}
+
 	ret = report_fid ? inode_mark_fid_xdev : 0;
+
 	TST_EXP_FD_OR_FAIL(fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_CLOSE_WRITE,
 					 AT_FDCWD, "."), ret);
Amir Goldstein Jan. 30, 2024, 2:58 p.m. UTC | #7
On Tue, Jan 30, 2024 at 3:07 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir, Jan,
>
> I was going to merge, but I suspect this does not work on TMPDIR on btrfs.
>

This is a problem because the test result depends on the type of TMPDIR.
The failure in your case is because TMPDIR is not only btrfs, but a
btrfs subvol.
Jan has dealt with several related fanotify tests failures lately.

> Tested on SLE 15-SP6 (kernel 6.4), on 15-SP4 (kernel 5.14), Tumbleweed (kernel
> 6.8.0-rc1), Alpine Linux (kernel 6.4) I get:
>
> fanotify01.c:341: TFAIL: fanotify_mark(fd_notify, 0x00000001, 0x00000008, -100, ".") failed: EXDEV (18)
>
> for tests #3, #4 and #5 on all filesystems.
>
> Testing on other on other filesystem it works: Debian kernel 5.10, 6.1 on ext4,
> Alpine Linux kernel 6.4 on tmpfs, Tumbleweed kernel 6.8.0-rc1 on tmpfs.
>
> Should be btrfs handled differently or skipped? (below)
> Or test EXDEV for #3, #4 and #5? (not sure how handle just half of the tests on
> btrfs differently).
>
> Kind regards,
> Petr
>
> diff --git testcases/kernel/syscalls/fanotify/fanotify01.c testcases/kernel/syscalls/fanotify/fanotify01.c
> index ba09f309d..97ade1829 100644
> --- testcases/kernel/syscalls/fanotify/fanotify01.c
> +++ testcases/kernel/syscalls/fanotify/fanotify01.c
> @@ -335,8 +335,15 @@ pass:
>          * different filesystems are supported.
>          * When tested fs has zero fsid (e.g. fuse) and events are reported
>          * with fsid+fid, watching different filesystems is not supported.
> +        * Not supported on Btrfs.
>          */
> +       if (tst_fs_type(".") == TST_BTRFS_MAGIC) {
> +               tst_res(TCONF, "skipped on Btrfs");
> +               return;
> +       }
> +

Note that btrfs is not the FS under test. It is the FS of TMPFS,
so even if you did skip, this message would have been wrong.

Please try the patch below.

Thanks,
Amir.

--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -364,12 +364,19 @@ static void setup(void)
        mount_mark_fid_unsupported =
fanotify_flags_supported_on_fs(FAN_REPORT_FID,

FAN_MARK_MOUNT,

FAN_OPEN, fname);
-       /* When mount mark is not supported due to zero fsid, multi fs
inode marks are not supported */
+       /*
+        * When mount mark is not supported due to zero fsid (e.g.
fuse) or if TMPDIR has non-uniform
+        * fsid (e.g. btrfs subvol), multi fs inode marks are not supported.
+        */
        if (mount_mark_fid_unsupported && errno == ENODEV) {
                tst_res(TINFO, "filesystem %s does not support
reporting events with fid from multi fs",
                                tst_device->fs_type);
                inode_mark_fid_xdev = EXDEV;
        }
+       if (fanotify_flags_supported_on_fs(FAN_REPORT_FID,
FAN_MARK_MOUNT, FAN_OPEN, ".")) {
+               inode_mark_fid_xdev = errno;
+               tst_res(TINFO, "TMPDIR does not support reporting
events with fid from multi fs");
+       }
 }
Petr Vorel Jan. 30, 2024, 6:44 p.m. UTC | #8
Hi Amir,

> On Tue, Jan 30, 2024 at 3:07 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Amir, Jan,

> > I was going to merge, but I suspect this does not work on TMPDIR on btrfs.


> This is a problem because the test result depends on the type of TMPDIR.
> The failure in your case is because TMPDIR is not only btrfs, but a
> btrfs subvol.
> Jan has dealt with several related fanotify tests failures lately.

> > Tested on SLE 15-SP6 (kernel 6.4), on 15-SP4 (kernel 5.14), Tumbleweed (kernel
> > 6.8.0-rc1), Alpine Linux (kernel 6.4) I get:

> > fanotify01.c:341: TFAIL: fanotify_mark(fd_notify, 0x00000001, 0x00000008, -100, ".") failed: EXDEV (18)

> > for tests #3, #4 and #5 on all filesystems.

> > Testing on other on other filesystem it works: Debian kernel 5.10, 6.1 on ext4,
> > Alpine Linux kernel 6.4 on tmpfs, Tumbleweed kernel 6.8.0-rc1 on tmpfs.

> > Should be btrfs handled differently or skipped? (below)
> > Or test EXDEV for #3, #4 and #5? (not sure how handle just half of the tests on
> > btrfs differently).

> > Kind regards,
> > Petr

> > diff --git testcases/kernel/syscalls/fanotify/fanotify01.c testcases/kernel/syscalls/fanotify/fanotify01.c
> > index ba09f309d..97ade1829 100644
> > --- testcases/kernel/syscalls/fanotify/fanotify01.c
> > +++ testcases/kernel/syscalls/fanotify/fanotify01.c
> > @@ -335,8 +335,15 @@ pass:
> >          * different filesystems are supported.
> >          * When tested fs has zero fsid (e.g. fuse) and events are reported
> >          * with fsid+fid, watching different filesystems is not supported.
> > +        * Not supported on Btrfs.
> >          */
> > +       if (tst_fs_type(".") == TST_BTRFS_MAGIC) {
> > +               tst_res(TCONF, "skipped on Btrfs");
> > +               return;
> > +       }
> > +

> Note that btrfs is not the FS under test. It is the FS of TMPFS,
> so even if you did skip, this message would have been wrong.

> Please try the patch below.

Great, works as expected.

I can merge amended commit [1], or feel free to send v2 if you want to mention
btrfs subvol in the commit message.

Kind regards,
Petr

[1] https://github.com/pevik/ltp/commit/359047c97151d87b4e709bbabaaa529a31bcc50b

> Thanks,
> Amir.

> --- a/testcases/kernel/syscalls/fanotify/fanotify01.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
> @@ -364,12 +364,19 @@ static void setup(void)
>         mount_mark_fid_unsupported =
> fanotify_flags_supported_on_fs(FAN_REPORT_FID,

> FAN_MARK_MOUNT,

> FAN_OPEN, fname);
> -       /* When mount mark is not supported due to zero fsid, multi fs
> inode marks are not supported */
> +       /*
> +        * When mount mark is not supported due to zero fsid (e.g.
> fuse) or if TMPDIR has non-uniform
> +        * fsid (e.g. btrfs subvol), multi fs inode marks are not supported.
> +        */
>         if (mount_mark_fid_unsupported && errno == ENODEV) {
>                 tst_res(TINFO, "filesystem %s does not support
> reporting events with fid from multi fs",
>                                 tst_device->fs_type);
>                 inode_mark_fid_xdev = EXDEV;
>         }
> +       if (fanotify_flags_supported_on_fs(FAN_REPORT_FID,
> FAN_MARK_MOUNT, FAN_OPEN, ".")) {
> +               inode_mark_fid_xdev = errno;
> +               tst_res(TINFO, "TMPDIR does not support reporting
> events with fid from multi fs");
> +       }
>  }
Amir Goldstein Jan. 30, 2024, 7:22 p.m. UTC | #9
On Tue, Jan 30, 2024 at 8:44 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> > On Tue, Jan 30, 2024 at 3:07 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Amir, Jan,
>
> > > I was going to merge, but I suspect this does not work on TMPDIR on btrfs.
>
>
> > This is a problem because the test result depends on the type of TMPDIR.
> > The failure in your case is because TMPDIR is not only btrfs, but a
> > btrfs subvol.
> > Jan has dealt with several related fanotify tests failures lately.
>
> > > Tested on SLE 15-SP6 (kernel 6.4), on 15-SP4 (kernel 5.14), Tumbleweed (kernel
> > > 6.8.0-rc1), Alpine Linux (kernel 6.4) I get:
>
> > > fanotify01.c:341: TFAIL: fanotify_mark(fd_notify, 0x00000001, 0x00000008, -100, ".") failed: EXDEV (18)
>
> > > for tests #3, #4 and #5 on all filesystems.
>
> > > Testing on other on other filesystem it works: Debian kernel 5.10, 6.1 on ext4,
> > > Alpine Linux kernel 6.4 on tmpfs, Tumbleweed kernel 6.8.0-rc1 on tmpfs.
>
> > > Should be btrfs handled differently or skipped? (below)
> > > Or test EXDEV for #3, #4 and #5? (not sure how handle just half of the tests on
> > > btrfs differently).
>
> > > Kind regards,
> > > Petr
>
> > > diff --git testcases/kernel/syscalls/fanotify/fanotify01.c testcases/kernel/syscalls/fanotify/fanotify01.c
> > > index ba09f309d..97ade1829 100644
> > > --- testcases/kernel/syscalls/fanotify/fanotify01.c
> > > +++ testcases/kernel/syscalls/fanotify/fanotify01.c
> > > @@ -335,8 +335,15 @@ pass:
> > >          * different filesystems are supported.
> > >          * When tested fs has zero fsid (e.g. fuse) and events are reported
> > >          * with fsid+fid, watching different filesystems is not supported.
> > > +        * Not supported on Btrfs.
> > >          */
> > > +       if (tst_fs_type(".") == TST_BTRFS_MAGIC) {
> > > +               tst_res(TCONF, "skipped on Btrfs");
> > > +               return;
> > > +       }
> > > +
>
> > Note that btrfs is not the FS under test. It is the FS of TMPFS,
> > so even if you did skip, this message would have been wrong.
>
> > Please try the patch below.
>
> Great, works as expected.
>
> I can merge amended commit [1], or feel free to send v2 if you want to mention
> btrfs subvol in the commit message.
>

Amended commit looks fine.

I don't think there is a need to specify btrfs subvol TMPDIR in the
commit message.
It is a minor implementation detail, not the main thing.

Thanks,
Amir.
Petr Vorel Jan. 30, 2024, 8:26 p.m. UTC | #10
> On Tue, Jan 30, 2024 at 8:44 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Amir,

> > > On Tue, Jan 30, 2024 at 3:07 PM Petr Vorel <pvorel@suse.cz> wrote:

> > > > Hi Amir, Jan,

> > > > I was going to merge, but I suspect this does not work on TMPDIR on btrfs.


> > > This is a problem because the test result depends on the type of TMPDIR.
> > > The failure in your case is because TMPDIR is not only btrfs, but a
> > > btrfs subvol.
> > > Jan has dealt with several related fanotify tests failures lately.

> > > > Tested on SLE 15-SP6 (kernel 6.4), on 15-SP4 (kernel 5.14), Tumbleweed (kernel
> > > > 6.8.0-rc1), Alpine Linux (kernel 6.4) I get:

> > > > fanotify01.c:341: TFAIL: fanotify_mark(fd_notify, 0x00000001, 0x00000008, -100, ".") failed: EXDEV (18)

> > > > for tests #3, #4 and #5 on all filesystems.

> > > > Testing on other on other filesystem it works: Debian kernel 5.10, 6.1 on ext4,
> > > > Alpine Linux kernel 6.4 on tmpfs, Tumbleweed kernel 6.8.0-rc1 on tmpfs.

> > > > Should be btrfs handled differently or skipped? (below)
> > > > Or test EXDEV for #3, #4 and #5? (not sure how handle just half of the tests on
> > > > btrfs differently).

> > > > Kind regards,
> > > > Petr

> > > > diff --git testcases/kernel/syscalls/fanotify/fanotify01.c testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > index ba09f309d..97ade1829 100644
> > > > --- testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > +++ testcases/kernel/syscalls/fanotify/fanotify01.c
> > > > @@ -335,8 +335,15 @@ pass:
> > > >          * different filesystems are supported.
> > > >          * When tested fs has zero fsid (e.g. fuse) and events are reported
> > > >          * with fsid+fid, watching different filesystems is not supported.
> > > > +        * Not supported on Btrfs.
> > > >          */
> > > > +       if (tst_fs_type(".") == TST_BTRFS_MAGIC) {
> > > > +               tst_res(TCONF, "skipped on Btrfs");
> > > > +               return;
> > > > +       }
> > > > +

> > > Note that btrfs is not the FS under test. It is the FS of TMPFS,
> > > so even if you did skip, this message would have been wrong.

> > > Please try the patch below.

> > Great, works as expected.

> > I can merge amended commit [1], or feel free to send v2 if you want to mention
> > btrfs subvol in the commit message.


> Amended commit looks fine.

> I don't think there is a need to specify btrfs subvol TMPDIR in the
> commit message.
> It is a minor implementation detail, not the main thing.

Good, merged!

Thanks!
Petr

> Thanks,
> Amir.
Jan Kara Jan. 30, 2024, 9:31 p.m. UTC | #11
On Tue 30-01-24 16:58:28, Amir Goldstein wrote:
> On Tue, Jan 30, 2024 at 3:07 PM Petr Vorel <pvorel@suse.cz> wrote:
> > Tested on SLE 15-SP6 (kernel 6.4), on 15-SP4 (kernel 5.14), Tumbleweed (kernel
> > 6.8.0-rc1), Alpine Linux (kernel 6.4) I get:
> >
> > fanotify01.c:341: TFAIL: fanotify_mark(fd_notify, 0x00000001, 0x00000008, -100, ".") failed: EXDEV (18)
> >
> > for tests #3, #4 and #5 on all filesystems.
> >
> > Testing on other on other filesystem it works: Debian kernel 5.10, 6.1 on ext4,
> > Alpine Linux kernel 6.4 on tmpfs, Tumbleweed kernel 6.8.0-rc1 on tmpfs.
> >
> > Should be btrfs handled differently or skipped? (below)
> > Or test EXDEV for #3, #4 and #5? (not sure how handle just half of the tests on
> > btrfs differently).
> >
> > Kind regards,
> > Petr
> >
> > diff --git testcases/kernel/syscalls/fanotify/fanotify01.c testcases/kernel/syscalls/fanotify/fanotify01.c
> > index ba09f309d..97ade1829 100644
> > --- testcases/kernel/syscalls/fanotify/fanotify01.c
> > +++ testcases/kernel/syscalls/fanotify/fanotify01.c
> > @@ -335,8 +335,15 @@ pass:
> >          * different filesystems are supported.
> >          * When tested fs has zero fsid (e.g. fuse) and events are reported
> >          * with fsid+fid, watching different filesystems is not supported.
> > +        * Not supported on Btrfs.
> >          */
> > +       if (tst_fs_type(".") == TST_BTRFS_MAGIC) {
> > +               tst_res(TCONF, "skipped on Btrfs");
> > +               return;
> > +       }
> > +
> 
> Note that btrfs is not the FS under test. It is the FS of TMPFS,
> so even if you did skip, this message would have been wrong.
> 
> Please try the patch below.

Thanks for fixing this so quickly! The fix looks good to me.

								Honza
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index e0d178bcc..554940a7e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -166,20 +166,23 @@  static inline int fanotify_flags_supported_on_fs(unsigned int init_flags,
 {
 	int fd;
 	int rval = 0;
+	int err = 0;
 
 	fd = fanotify_init(init_flags, O_RDONLY);
-
 	if (fd < 0) {
+		err = errno;
 		if (errno == ENOSYS)
 			tst_brk(TCONF, "fanotify not configured in kernel");
 		if (errno != EINVAL)
 			tst_brk(TBROK | TERRNO,
 				"fanotify_init(%x, O_RDONLY) failed",
 				init_flags);
+		errno = err;
 		return -1;
 	}
 
 	if (fname && fanotify_mark(fd, FAN_MARK_ADD | mark_flags, event_flags, AT_FDCWD, fname) < 0) {
+		err = errno;
 		if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
 			rval = strcmp(fname, OVL_MNT) ? -2 : -3;
 		} else if (errno != EINVAL) {
@@ -194,6 +197,7 @@  static inline int fanotify_flags_supported_on_fs(unsigned int init_flags,
 
 	SAFE_CLOSE(fd);
 
+	errno = err;
 	return rval;
 }
 
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index e4398f236..ba09f309d 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -77,6 +77,7 @@  static char buf[BUF_SIZE];
 static int fd_notify;
 static int fan_report_fid_unsupported;
 static int mount_mark_fid_unsupported;
+static int inode_mark_fid_xdev;
 static int filesystem_mark_unsupported;
 
 static unsigned long long event_set[EVENT_MAX];
@@ -328,6 +329,17 @@  pass:
 
 	}
 
+
+	/*
+	 * Try to setup a bogus mark on test tmp dir, to check if marks on
+	 * different filesystems are supported.
+	 * When tested fs has zero fsid (e.g. fuse) and events are reported
+	 * with fsid+fid, watching different filesystems is not supported.
+	 */
+	ret = report_fid ? inode_mark_fid_xdev : 0;
+	TST_EXP_FD_OR_FAIL(fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_CLOSE_WRITE,
+					 AT_FDCWD, "."), ret);
+
 	/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
 	SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | mark->flag,
 			  FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
@@ -352,6 +364,12 @@  static void setup(void)
 	mount_mark_fid_unsupported = fanotify_flags_supported_on_fs(FAN_REPORT_FID,
 								    FAN_MARK_MOUNT,
 								    FAN_OPEN, fname);
+	/* When mount mark is not supported due to zero fsid, multi fs inode marks are not supported */
+	if (mount_mark_fid_unsupported && errno == ENODEV) {
+		tst_res(TINFO, "filesystem %s does not support reporting events with fid from multi fs",
+				tst_device->fs_type);
+		inode_mark_fid_xdev = EXDEV;
+	}
 }
 
 static void cleanup(void)
@@ -368,6 +386,7 @@  static struct tst_test test = {
 	.needs_root = 1,
 	.mount_device = 1,
 	.mntpoint = MOUNT_PATH,
+	.all_filesystems = 1,
 };
 
 #else