Re: xfs_admin -O feature upgrade feedback

["Darrick J. Wong" <djwong@kernel.org>]

[adding linux-xfs to cc]

On Mon, Mar 01, 2021 at 11:13:14AM +0100, Geert Hendrickx wrote:
> Gentlemen
> 
> 
> I've been testing xfsprogs 5.11.0-rc1 for XFS feature upgrades (inobtcount
> and bigtime), and have a couple of small nits with it - besides the actual
> functionality working as expected:
> 
> 1/ xfs_admin responds to every xfs_repair failure with the very generic
> "Conversion failed, is the filesystem unmounted?"  This isn't very helpful
> (and left me scratching my head in a number of scenarios), whereas calling
> xfs_repair directly shows a relevant error message in all cases.  This
> output should somehow be relayed through xfs_admin - without just dumping
> the whole xfs_repair output which I know you wanted to avoid.  Maybe by
> distinguishing more carefully between stderr and stdout?  (Currently, it
> seems xfs_repair sends its errors to stdout and "normal output" to stderr,
> and xfs_admin discards xfs_repair's stderr.)

That's a difficult project -- some of the things repair will complain
about are a result of whatever the upgrade is (e.g. complaining about
incorrect inode btree counters when you're in the process of enabling
the counters) but then there are other things that it probably should
not be dropping on the floor.

> 2/ minor, but xfs_admin(8) manpage documents `xfs_admin -O feature=status`,
> however xfs_admin itself appends another `=1`, resulting in `xfs_repair -c
> feature=1=1`.  This works, but looks ugly, and is not consistent with the
> option to enable multiple features at once.  I think either the xfs_admin
> script or its manpage should be adjusted to be consistent?

Yeah, xfs_admin should not add '=1'. Thanks for pointing that out.

> Apart from this, the actual upgrade functionality is working as expected,
> great job!

Thx :)

> Btw, do you have an idea from which release onwards mkfs.xfs will enable
> the new features by default?  Are there fixed rules for this (like feature
> must be X releases old, or supported by the latest LTS kernel), or is this
> judged on a case-by-case basis?

~6mo after we hear that people are using the feature /and/ we haven't
heard of any serious complaints.

Or some distro makes a business case and enables it by default. <cough>

--D

> 
> Thanks!
> 
> 
> 	Geert
> 
>