Re: [PATCH 01/13] libfrog: fix workqueue error communication problems

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Mon, Sep 30, 2019 at 02:23:40PM -0500, Eric Sandeen wrote:
> On 9/25/19 4:33 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Convert all the workqueue functions to return positive error codes so
> > that we can move away from the libc-style indirect errno handling and
> > towards passing error codes directly back to callers.
> 
> This all looks fine, but it doesn't really do what the commit log says,
> right?

Urrrk... yes.  Clearly I stamped out the changelogs with a machine. :/

I /think/ most of the patches in that series actually have a return
conversion and a callsite conversion, but this one clearly is just...

"Convert workqueue functions to return errno errors from the C library,
then convert the callers to use str_liberror to report the runtime
error."

> The one spot where error return is changed, it was already
> positive; the rest is swapping str_liberror for str_info which is
> just cosmetic, right?

<shrug> Mostly cosmetic.  Before you'd get:

INFO: Could not create workqueue

Now you get:

ERROR: creating icount workqueue: Not enough frobs.

(and it actually records it as a runtime error :P)

--D

> -Eric
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  libfrog/workqueue.c |    4 ++--
> >  scrub/fscounters.c  |    5 ++---
> >  scrub/inodes.c      |    5 ++---
> >  scrub/phase2.c      |    8 +++-----
> >  scrub/phase4.c      |    6 +++---
> >  scrub/read_verify.c |    3 +--
> >  scrub/spacemap.c    |   11 ++++-------
> >  scrub/vfs.c         |    3 +--
> >  8 files changed, 18 insertions(+), 27 deletions(-)
> > 
> > 
> > diff --git a/libfrog/workqueue.c b/libfrog/workqueue.c
> > index 73114773..a806da3e 100644
> > --- a/libfrog/workqueue.c
> > +++ b/libfrog/workqueue.c
> > @@ -106,8 +106,8 @@ workqueue_add(
> >  	}
> >  
> >  	wi = malloc(sizeof(struct workqueue_item));
> > -	if (wi == NULL)
> > -		return ENOMEM;
> > +	if (!wi)
> > +		return errno;
> >  
> >  	wi->function = func;
> >  	wi->index = index;
> > diff --git a/scrub/fscounters.c b/scrub/fscounters.c
> > index ad467e0c..669c5ab0 100644
> > --- a/scrub/fscounters.c
> > +++ b/scrub/fscounters.c
> > @@ -115,15 +115,14 @@ xfs_count_all_inodes(
> >  			scrub_nproc_workqueue(ctx));
> >  	if (ret) {
> >  		moveon = false;
> > -		str_info(ctx, ctx->mntpoint, _("Could not create workqueue."));
> > +		str_liberror(ctx, ret, _("creating icount workqueue"));
> >  		goto out_free;
> >  	}
> >  	for (agno = 0; agno < ctx->mnt.fsgeom.agcount; agno++) {
> >  		ret = workqueue_add(&wq, xfs_count_ag_inodes, agno, ci);
> >  		if (ret) {
> >  			moveon = false;
> > -			str_info(ctx, ctx->mntpoint,
> > -_("Could not queue AG %u icount work."), agno);
> > +			str_liberror(ctx, ret, _("queueing icount work"));
> >  			break;
> >  		}
> >  	}
> ...
> 
>