Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable

Cyril Hrubis
2022-11-22T12:41:16Z

Hi!@
> > > + for (i = 0; i < max_file_multi; i++) {
> > > + char path[PATH_MAX];
> > > +
> > > + sprintf(path, FILE_PATH_MULTIDIR, i);
> > > + SAFE_UNLINK(path);
> > > + sprintf(path, DIR_PATH_MULTI, i);
> > > + SAFE_RMDIR(path);
> > > + sprintf(path, FILE_PATH_MULTI, i);
> > > + SAFE_UNLINK(path);
> > > + }
> > > SAFE_UNLINK(FILE_PATH);
> > > - SAFE_UNLINK(FILE2_PATH);
> > > SAFE_RMDIR(SUBDIR_PATH);
> > > SAFE_RMDIR(DIR_PATH);
> > > SAFE_RMDIR(MNT2_PATH);
>
> > Do we have to unlink anything at all?
>
> > As far as I can tell we create these files on a device that is
> > reformatted after the test anyways.
>
> It looks it's not cleared, because tmpdir is removed in do_cleanup(),
> which is called just before the end of testing, not for each filesystem:
>
> https://github.com/linux-test-project/ltp/tree/master/lib
>
> Files created for testing in test's setup() are called for each filesystem
> (do_test_setup), therefore files would remain created:
>
> fanotify10.c:894: TBROK: mkdir(fs_mnt/testdir, 0755) failed: EEXIST (17)
>
> Summary:
> passed 492
> failed 0
> broken 1
> skipped 3
> warnings 0
>
> And even if we disable just SAFE_UNLINK(FILE_PATH); we get errors
> due our rmdir() implementation wanting directory being empty:
>
> fanotify10.c:953: TWARN: rmdir(fs_mnt/testdir) failed: ENOTEMPTY (39)
> fanotify10.c:894: TBROK: mkdir(fs_mnt/testdir, 0755) failed: EEXIST (17)
> fanotify10.c:952: TWARN: rmdir(fs_mnt/testdir/testdir2) failed: ENOENT (2)
> fanotify10.c:953: TWARN: rmdir(fs_mnt/testdir) failed: ENOTEMPTY (39)
> fanotify10.c:954: TWARN: rmdir(mntpoint) failed: ENOENT (2)
>
> IMHO it's safer to remove these files instead of creating them just for first
> filesystem. Having unique path for each filesystem (e.g. use filesystem name in
> the directory path would result would solve the need to remove files but
> probably not worth of implementing.

Hmm, I was commenting under an impression that we create all files on
the device and it looks like that is the case, but we do not run with
.all_filesystems but rather than that with .variants = 2 and of course
we call do_test_setup() for each test variant. So yes we run the test
twice with the same device and the cleanup is required.

--
Cyril Hrubis
chrubis@suse.cz