Re: [PATCH 2/4] generic: ensure we drop suid after fallocate

[Amir Goldstein <amir73il@gmail.com>]

On Sat, Apr 16, 2022 at 5:02 PM Zorro Lang <zlang@redhat.com> wrote:
>
> On Fri, Apr 15, 2022 at 04:42:33PM +0300, Amir Goldstein wrote:
> > > Hi Darrick, that's another story, you don't need to worry about that in this case :)
> > > I'd like to ack this patch, but hope to move it from generic/ to shared/ . Maybe
> > > Eryu can help to move it, or I can do that after I get the push permission.
> > >
> > > The reason why I intend moving it to shared is:
> > > Although we are trying to get rid of tests/shared/, but the tests/shared/ still help to
> > > remind us what cases are still not real generic cases. We'll try to help all shared
> > > cases to be generic. When the time is ready, I'd like to move this case to generic/
> > > and change _supported_fs from "xfs btrfs ext4" to "generic".
> > >
> >
> > Sorry, but I have to object to this move.
> > I do not think that is what tests/shared should be used for.
> >
> > My preferences are:
> > 1. _suppoted_fs generic && _require_xfs_io_command "finsert"
>
> There is:
> "verb=finsert
>  _require_xfs_io_command $verb"
>
> This patch has not only one case, different cases test different mode of fallocate,
> and I think Darrick has given them different _require_xfs_io_command.
>

I know. I meant that the tests for verbs finsert/fcollapse can definitely use
generic as there are very few fs that support those verbs and those fs
should be fixed, not hide the failure.

cifs maintainer btw is using whitelists of tests for CI, so new
failing tests  are
not likely to affect cifs testing.

> > 2. _suppoted_fs generic
> > 3. _supported_fs xfs btrfs ext4 (without moving to tests/shared)
>
> There's not any generic cases write like this, only shared cases like that. My personal
> opinion is *(2)* or make it shared if it insists "_supported_fs xfs btrfs ext4" (then
> will move it back to generic and "_suppoted_fs generic" when Darrick think it's time).
>

Let's wait to hear what Darrick has to say.
I just don't understand the incentive to hide test failures from buggy fs even
if this is a change of long time behavior.

BTW, here is untested draft of what I started working on:
https://github.com/amir73il/xfstests/commits/hints

With the new helpers, this test could also be classified as:

_suppoted_fs generic
_known_issue_on_fs ^xfs ^btrfs ^ext4

Meaning that the test runs on all fs, unless tester opts-out with
-x known_issues
and if test is run an fails an hint is printed:
"You _MAY_ be hit by a known issue on $FSTYP."

I'll probably be done with testing those patches and post them tomorrow.

Thanks,
Amir.