On Tue, Mar 24, 2020 at 04:24:01PM -0700, Darrick J. Wong wrote: > On Wed, Mar 25, 2020 at 10:12:55AM +1100, Dave Chinner wrote: > > On Tue, Mar 24, 2020 at 03:57:26PM -0500, Eric Sandeen wrote: > > > On 3/23/20 7:19 PM, Dave Chinner wrote: > > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > > > Many operations don't set the exitcode when they fail, resulting > > > > in xfs_io exiting with a zero (no failure) exit code despite the > > > > command failing and returning an error. The command return code is > > > > really a boolean to tell the libxcmd command loop whether to > > > > continue processing or not, while exitcode is the actual xfs_io exit > > > > code returned to the parent on exit. > > > > > > > > This patchset just makes the code do the right thing. It's not the > > > > nicest code, but it's a start at producing correct behaviour. > > > > > > > > Signed-Off-By: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > I wonder if there somewhere we could formally document these conventions... > > > > > > Like maybe at least near the "exitcode" global declaration? > > > > I really think we need to rework the way we do the error handling > > in the command line parsing for these utilities. One of the things I > > found in doing this is that most of the code does return error codes > > to the main function, only then to drop it on the floor and turn it > > into "exitcode = 1; return 0;" pair. > > > > So I'm pondering how to make this much simpler - returning error > > codes from the command functions would be a much better idea, > > then have a command flag to indicate whether we continue on error or > > terminate. > > > > That moves all the exit code handling out of the commands and > > provides consistent error handling for all commands and > > infrastructure - 0 = success, failure returns negative errno - and > > so should enable much more reliable and consistent error handling > > across all the utilities.... > > It seems reasonable to me, though I wonder how fstests will react to > that. Then again, a lot of xfs_io error handling seems to be done via > grep so maybe it wouldn't be that bad. :) Pretty much. I don't think it will have much impact at all. This patch hasn't caused any new failures in my fstests runs at all, so I don't think further work to simplify this functionality will cause new problems, either... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
