Re: [PATCH] btrfs: sysfs: export dev stats in devinfo directory

[Omar Sandoval <osandov@osandov.com>]

On Thu, Jun 10, 2021 at 06:37:01PM +0200, David Sterba wrote:
> On Wed, Jun 09, 2021 at 05:55:05PM -0700, Omar Sandoval wrote:
> > > > The ioctl returns ENODEV is !dev_stats_valid, maybe this file should do
> > > > the same? It seems a little awkward to have a flag that means that the
> > > > rest of the file is meaningless.
> > > 
> > > You mean returning -ENODEV when reading the stats file? Or return 0 but
> > > the contents is something like 'stats invalid' or similar.
> > 
> > I'd vote for returning -ENODEV when reading the stats file, but I think
> > either one is fine.
> 
> Hm so I think this should reflect how the sysfs files are used. They all
> contain textual information, and errors are returned when eg. there are
> no permissions.
> 
> In a shell script it's IMHO more convenient to do
> 
> stats=$(cat $devicepath/stats)
> 
> and then validate contents of $stats rather then catching the error
> value and deciding based on that what happend. Not to say that this
> would also print an error message. I've found this in
> admin-guide/sysfs-rules.rst that's perhaps closest to a recommendation
> we could follow:
> 
> 172 - When reading and writing sysfs device attribute files, avoid dependency
> 173     on specific error codes wherever possible. This minimizes coupling to
> 174     the error handling implementation within the kernel.
> 
> So I take it as that error codes belong to the sysfs layer and the
> validity of the contents is up to the sysfs user, ie. btrfs in this
> case.

Good point. "Real" programs will just use the ioctl, so it makes sense
to cater this to shell scripts.