Re: [PATCH 5/5] xfs_io: set exitcode on failure appropriately

[Dave Chinner <david@fromorbit.com>]

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@redhat.com>
> > > > 
> > > > 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@redhat.com>
> > > 
> > > 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@fromorbit.com