Re: [PATCH] cifs: add test that setfattr -x fails non-existing EAs

[Eryu Guan <guaneryu@gmail.com>]

On Mon, Feb 11, 2019 at 09:15:28AM +0200, Amir Goldstein wrote:
> Hi Ronnie,
> 
> CC the correct list <fstests@vger.kernel.org>
> 
> On Mon, Feb 11, 2019 at 7:21 AM Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> >
> > We just fixed a bug in cifs.ko where it would incorrectly return success
> > for setfattr -x user.does-not-exist.
> >
> > This patch adds a test case for this.
> >
> > Xfstests already have tests for setfattr -x in generic/097
> > but we can not yet use that test for cifs since we can only support
> > the user namespace.
> 
> Mmm... that's not a reason to write a cifs specific test.
> 1. Your test is not cifs specific so should be generic
> 2. There is a lot of other test coverage cifs is missing from generic/097
> 
> What I suggest is:
> - implement _require_trusted_attrs
> - replace _require_attrs with  _require_trusted_attrs in the few
>   generic tests that use trusted xattrs
> - I counted 5 generic tests and there is also generic/079 that
>   sets trusted xattr via t_immutable and doesn't currently _require_attrs
>   at all. Frankly, it looks like most of those test could use user.* xattrs,
>   but whatever.
> - Anyway, please stay away from the overlay trusted xattr tests.
> - clone generic/097 to a new test that only _require_attrs
>   leaving out the trusted xattrs

Yeah, these suggestions all look good to me,

> 
> After that change, cifs will not fail on the trusted xattr tests
> and instead those tests will be properly skipped for cifs.

and this would be the ideal situation for cifs :)

Thanks,
Eryu