Re: xfs_admin -O feature upgrade feedback

[Brian Foster <bfoster@redhat.com>]

On Tue, Mar 02, 2021 at 11:57:22PM +0100, Geert Hendrickx wrote:
> On Tue, Mar 02, 2021 at 07:19:37 -0500, Brian Foster wrote:
> > It's not clear to me if you're reporting that feature upgrades spuriously
> > report this "Conversion failed ..." message (i.e., feature upgrade
> > succeeded, but repair found and fixed things expected to be problems due
> > to the feature upgrade), or that this error is reported if there is
> > something independently wrong with the fs. If the former, that seems like
> > a bug. If the latter, I think that's reasonable/expected behavior.
> 
> 
> 
> There are sillier scenarios, like simply incorrect arguments.  For example
> "xfs_admin -O bigtypo=1 /dev/foo" responds with: "Conversion failed, is the
> filesystem unmounted?"
> 
> (where /dev/foo is the correct blockdevice, properly unmounted etc, but the
> options argument contains a typo)
> 
> The proper xfs_repair error "unknown option -c bigtypo=1" gets thrown away.
> 
> 
> Other examples include "-O bigtime" => "bigtime requires a parameter" (with
> Darrick's patch for the other issue applied), or "bigtime=0" => "bigtime
> only supports upgrades", all dropped on the floor by xfs_admin and replaced
> with the one generic message that gives no indication of the actual problem.
> (the user keeps verifying whether the filesystem is unmounted and clean...)
> 

Ok. I suppose in the scenario where xfs_repair runs on behalf of
xfs_admin and then fails immediately due to a usage error, it might be
more appropriate to dump whatever error xfs_repair exits with. I'm not
sure how best to filter that and/or deal with the issues Darrick points
out, but fair point...

Maybe a simple compromise is a verbose option for xfs_admin itself..?
I.e., the normal use case operates as it does now, but the failure case
would print something like:

  "Feature conversion failed. Retry with -v for detailed error output."

... and then 'xfs_admin -v ...' would just pass through xfs_repair
output. Eh?

Brian

> 
> 
> 	Geert
> 
>