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 > >
