Re: [PATCH v2 2/6] fstests: consolidate _require_fs_sysfs and _require_fs_sysfs_dev_attr

[Zorro Lang <zlang@kernel.org>]

On Wed, Oct 28, 2026 at 08:57:26AM -0700, Christoph Hellwig wrote:
> On Sat, Oct 24, 2026 at 04:31:10AM +0800, Zorro Lang wrote:
> > There is heavy overlap between _require_fs_sysfs and
> > _require_fs_sysfs_dev_attr, and their ambiguous names make it
> > difficult to tell them apart.
> > 
> > Teach _require_fs_sysfs_dev_attr to default to $TEST_DEV when only
> > an attribute is given, remove _has_fs_sysfs and _require_fs_sysfs,
> > and update their callers.
> 
> I find the optional device argument a bit confusing.  What speaks against
> making it mandatory and having _require_{test,scratch}_sysfs_dev_attr
> wrappers?

Thanks for reviewing! Sure, that makes more sense.

> 
> > +	_has_fs_sysfs_dev_attr $dev "$ctlfile" && _set_fs_sysfs_dev_attr $dev "$ctlfile" 1
> 
> Please turn this into an if to make it more readable and avoid the
> long line.

Sure, I'll send v3 with this patch updated.

Thanks,
Zorro

> 
> >  
> >  	# Disable retry of metadata writes that fail with EIO
> >  	for ctl in max_retries retry_timeout_seconds; do
> >  		ctlfile="error/metadata/EIO/$ctl"
> >  
> > -		_has_fs_sysfs "$ctlfile" && _set_fs_sysfs_dev_attr $dev "$ctlfile" 0
> > +		_has_fs_sysfs_dev_attr $dev "$ctlfile" && _set_fs_sysfs_dev_attr $dev "$ctlfile" 0
> 
> Same.
>