Re: [LTP] [PATCH 3/5] syscalls/fanotify10: Watch directory that is not the mount path

From: Petr Vorel <pvorel@suse.cz>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Matthew Bobrowski <repnop@google.com>, Jan Kara <jack@suse.cz>,
	LTP List <ltp@lists.linux.it>
Subject: Re: [LTP] [PATCH 3/5] syscalls/fanotify10: Watch directory that is not the mount path
Date: Thu, 16 Jun 2022 15:31:01 +0200	[thread overview]
Message-ID: <YqswlUo82GnEzEnE@pevik> (raw)
In-Reply-To: <CAOQ4uxitmPwwGHiAonZjyqJKG=+SvzraGG9ot1=a2iNrcscAAQ@mail.gmail.com>

> On Tue, Jun 14, 2022 at 1:26 PM Jan Kara <jack@suse.cz> wrote:

> > On Mon 13-06-22 17:38:24, Amir Goldstein wrote:
> > > Move the test file into a test directory, so we won't set a mark
> > > on the mount path when watching the children of a directory.

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

> > One question below...

> > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > index 067dd65ae..efef25135 100644
> > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > @@ -74,16 +74,19 @@ static int filesystem_mark_unsupported;

> > >  #define MOUNT_PATH "fs_mnt"
> > >  #define MNT2_PATH "mntpoint"
> > > +#define DIR_NAME "testdir"
> > >  #define FILE_NAME "testfile"
> > >  #define FILE2_NAME "testfile2"
> > >  #define TEST_APP "fanotify_child"
> > >  #define TEST_APP2 "fanotify_child2"
> > > -#define FILE_PATH MOUNT_PATH"/"FILE_NAME
> > > -#define FILE2_PATH MOUNT_PATH"/"FILE2_NAME
> > > +#define DIR_PATH MOUNT_PATH"/"DIR_NAME
> > > +#define FILE_PATH DIR_PATH"/"FILE_NAME
> > > +#define FILE2_PATH DIR_PATH"/"FILE2_NAME
> > >  #define FILE_EXEC_PATH MOUNT_PATH"/"TEST_APP
> > >  #define FILE2_EXEC_PATH MOUNT_PATH"/"TEST_APP2
> > > -#define FILE_MNT2 MNT2_PATH"/"FILE_NAME
> > > -#define FILE2_MNT2 MNT2_PATH"/"FILE2_NAME
> > > +#define DIR_MNT2 MNT2_PATH"/"DIR_NAME
> > > +#define FILE_MNT2 DIR_MNT2"/"FILE_NAME
> > > +#define FILE2_MNT2 DIR_MNT2"/"FILE2_NAME
> > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2

> > > @@ -239,50 +242,50 @@ static struct tcase {
> > >       },
> > >       {
> > >               "ignore events on children of directory created on a specific file",
> > > -             MNT2_PATH, FANOTIFY_INODE,
> > > -             FILE_PATH, FANOTIFY_INODE,
> > > +             DIR_MNT2, FANOTIFY_INODE,
> > > +             DIR_PATH, FANOTIFY_INODE,
> > >               FAN_EVENT_ON_CHILD,
> > >               FILE_PATH, 0, FAN_OPEN
> > >       },

> > I'm somewhat confused here why you remove the FILE_PATH test here... In all
> > the other tests you just replace MNT2_PATH with DIR_PATH which makes sense.

> Good question.
> I guess while doing the conversion, I realized that the test case definition
> was wrong, so I fixed it and forgot to mention it in the commit message.

> The test case (#17) should be placing an ignored mask on a directory
> (DIR_PATH) with FAN_EVENT_ON_CHILD and see that the event on
> a child (FILE_PATH) is ignored.

> But what the test case was doing is setting an ignored mask
> with FAN_EVENT_ON_CHILD on the child (FILE_PATH).

> Petr,

> Do you want to add this to the commit message?
> Or split to a separate patch?

Both ways work for me. Could you please rebase your LTP fork, do either of these
changes and ping me to pull?

Thx!

Petr

> Thanks,
> Amir.

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