Re: [LTP] [PATCH 3/4] fanotify24: Add test for FAN_PRE_ACCESS and FAN_DENY_ERRNO

[Petr Vorel <pvorel@suse.cz>]

> On Mon, Feb 10, 2025 at 4:43 PM Jan Kara <jack@suse.cz> wrote:

> > On Mon 10-02-25 16:13:15, Amir Goldstein wrote:
> > > Fork the test fanotify24 from test fanotify03, replacing the
> > > permission event FAN_ACCESS_PERM with the new pre-content event
> > > FAN_PRE_ACCESS.

> > > The test is changed to use class FAN_CLASS_PRE_CONTENT, which is
> > > required for FAN_PRE_ACCESS and this class also enabled the response
> > > with cutomer error code FAN_DENY_ERRNO.

> > > Unlike FAN_ACCESS_PERM, FAN_PRE_ACCESS is also created on write()
> > > system call.  The test case expected results are adjusted to
> > > respond with the default error (EPERM) to open() and write() and
> > > to respond with custom errors (EIO, EBUSY) to read() and execve().

> > > Not all fs support pre-content events, so run on all filesystems
> > > to excercise FAN_PRE_ACCESS on all supported filesystems.

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

> > Looks good to me. I was just wondering whether some bits like
> > generate_events(), mark setup, child setup, main test loop could not be
> > factored out into a helper functions used by both old and new tests?

> Yes, I agree that forking the tests is bad and that we need much
> more common helpers.

> IIUC, LTP developers are going to try to come up with some proposals
> for refactoring helpers to split some large fanotify tests [1][2].

> My opinion is that factoring out helpers that are useful only for
> fanotify03,fanotify24 is suboptimal and we need to see if we can
> create much more generic helpers that could be shared by more tests.

> BTW, if you look closer, you will see that generate_events() is quite
> different between fanotify03 and fanotify24, although it is true that
> fanotify24 has a more generalized version that follows the expected
> events more closely.

> I did start with extending fanotify03 before I forked it and before the
> fork generate_events() was even more hard to follow because of
> the difference in expected events for write() between permission
> and pre-content events.

I'm not happy that tests are nearly identical, but agree that merging them would
make readability even harder. Also if there is really no value to run
FAN_ACCESS_PERM (fanotify03.c) on all filesystems it would prolong testing.

The only downside of not factoring out common code is that fix in one test will
not appear in the other test (it looks to me from what you wrote that you
improved generate_events() for fanotify24.c but not for fanotify03.c. Also now
is for me to remember remove else if (errno == exp_errno) [1] also for
fanotify24.c). We have long history of this in LTP.  But I'm also not sure if
it's worth factoring out code just for 2 tests. We might reconsider factoring
out, but unless Martin or Cyril objects I would keep it for now.

I was looking briefly for code which could be turned into more generic helpers,
but so far I haven't found anything. Maybe you have better idea.

FYI there is not only test code and test output readability, but also ability to
filter out certain fix. If particular fix is not backported to enterprise kernel
(WONTFIX). With one of many tests fails we have no way to distinguish it in the
current code (tst_kvercmp2() does not always help). Therefore we prefer to have
regression tests in separate files (don't mix them with tests for basic
functionality). But that would probably lead to even more code duplicity.

I was thinking whether we could try allow to run only subset of struct tcase
items, based on getopt parameter. E.g. run only "inode" marks (first two items),
or mount marks (3rd and 4th), ...  runtest/syscalls would get more items of
particular test. It would prolong testing (for all_filesystems quite significantly)
but besides better test output readability + allow to filter out fixes.

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/CAOQ4uxgu16dOsU4uuq66CGqXw6wY8c8jK7sL1QheB8kTPU=X+g@mail.gmail.com/

> Thanks,
> Amir.

> [1] https://lore.kernel.org/ltp/71d4414b-802f-4019-8527-e8886e2d1aeb@suse.cz/
> [2] https://lore.kernel.org/ltp/20250131164217.GA1135694@pevik/

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