On Tue, Apr 04, 2017 at 07:31:46PM +0300, Amir Goldstein wrote: > On Tue, Apr 4, 2017 at 2:19 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Thu, Feb 23, 2017 at 11:42 AM, Xiong Zhou <xzhou@xxxxxxxxxx> wrote: > [...] > >> + > >> +# Modify as appropriate. > >> +_supported_fs overlay > >> +_supported_os Linux > >> +_require_scratch > >> +_require_chattr > >> + > > > > Eryu, > > > > A way-post-merge bug spotting: > > Should be "_require_chattr i". > > _require_chattr alone makes no sense (and should probably be reported > > as an error) > > > > I may get to send a fix, but not too soon.. > > > > Apparently, _require_chattr with no arguments does do what it is expected to do, > or as David wrote in the new documentation patch: > "The test requires that the chattr command be available and supported by > the $TEST_DEV filesystem" > > But it appears to be doing the right thing by coincidence?? > Without any arguments to _require_chattr, it ends up executing > > chattr + $file > > and > > chattr - file > > The former, surprisingly does do FS_IOC_GETFLAGS and FS_IOC_SETFLAGS > on the file, so the check is correct. > > The latter spews this error into full output "Must use '-v', =, - or +" > and ignored the failure - again correct, but seems unintentional. > > I wonder if we should forbid the use of _require_chattr with no args? > It's only user right now is test overlay/027 and that seems like a bug > (test should _require_chattr i). Yes, it's a bug, my bad. Thanks for catching this, good job! -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html