On Mon, Feb 11, 2019 at 09:15:28AM +0200, Amir Goldstein wrote: > Hi Ronnie, > > CC the correct list <fstests@xxxxxxxxxxxxxxx> > > On Mon, Feb 11, 2019 at 7:21 AM Ronnie Sahlberg <lsahlber@xxxxxxxxxx> 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
