[LTP] [PATCH v2 2/3] Add fanotify_get_supported_init_flags() helper function

[Martin Doucha]

On 25. 10. 22 11:48, Jan Kara wrote:
> On Tue 25-10-22 10:51:01, Martin Doucha wrote:
>> Imagine two init flags, A and B (doesn't matter which ones) that are not
>> supposed to conflict in any way according to documentation. And we'll add 3
>> fanotify14 test cases with the following init calls:
>> - fanotify_init(A)
>> - fanotify_init(B)
>> - fanotify_init(A|B)
>>
>> All 3 init calls are supposed to pass and then fanotify_mark() is supposed
>> to fail. Now imagine that we have a buggy kernel where both flags are
>> implemented but fanotify_init(A|B) hits a weird corner case and returns
>> EINVAL. In your version of the code, the test will assume that it's due to a
>> missing feature and report the test case as skipped. In my version of the
>> code, the test will report a bug because it knows that all the required
>> features are present.
> 
> Yeah, but AFAIU you are trading expanded testing for possibility of false
> test failures (because the situation that for some features A and B, both A
> and B are implemented but A|B got implemented only later seems equally
> probable scenario).
> 
> Since I don't find this critical to test (it seems like a relatively
> unlikely mistake to do), I'd prefer less testing against false test
> failures. If we want to be more precise, we should be spelling out in the
> testcase (ideally using some common infrastructure) that if A & B is
> supported, we also expect A|B (or even some C) to work.

This kind of failure may be highly unlikely on a vanilla kernel but it 
can easily happen due to incorrectly backported patches. IMHO it's 
better to get a failure and find out that the test case was invalid than 
to ignore a bug that may indicate deeper issues. We can always fix a 
broken test and possibly also update documentation of past changes in 
syscall behavior.

On 25. 10. 22 13:11, Amir Goldstein wrote:
> It is a valid test case to assert that the support for two flags is
> independent,
> but this is not the job of fanotify14.
> fanotify14 checks for *illegal* flag combinations.
> 
> If you feel that there should be a test that verifies that
> support of flag A is independent of support of flag B,
> then please write a different test for that.
> 
> But then would you test all possible permutations of flags?
> Not only flags that are used in fanotify14?
> Not only flag pairs? but more concurrent flags?
> I don't know if other APIs have such rigorous tests (except API fuzzers).
> 
> I agree with Jan that the value of such a test would be questionable,
> but it does have a value, so I won't object to having this test, as
> long as it does not blindly check for all the known fanotify init bits
> are independent.

We find a fair amount of kernel bugs not because we have a specific 
targeted testcase for them but because they break test setup for 
something else. The subtests in fanotify14 may not comprehensively test 
all combinations of init flags but it's still "free" test coverage that 
will be useful for catching bugs.

> Asserting flag combination independence should be opt-in by the test
> not out-out like you did with REPORT_FID and REPORT_NAME.

I don't understand this sentence, especially which patch it's referring to.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic