Re: [PATCH 1/1] xfs_spaceman: report health problems

[Dave Chinner <david@fromorbit.com>]

On Thu, Sep 05, 2019 at 08:33:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Use the fs and ag geometry ioctls to report health problems to users.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  libfrog/fsgeom.c        |   16 ++
>  libfrog/fsgeom.h        |    1 
>  man/man8/xfs_spaceman.8 |   28 +++
>  spaceman/Makefile       |    2 
>  spaceman/health.c       |  459 +++++++++++++++++++++++++++++++++++++++++++++++
>  spaceman/init.c         |    1 
>  spaceman/space.h        |    1 
>  7 files changed, 507 insertions(+), 1 deletion(-)
>  create mode 100644 spaceman/health.c

Looks good. Minor nit below, but otherwise:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

> +static int
> +report_bulkstat_health(
> +	xfs_agnumber_t		agno)
> +{
> +	struct xfs_bstat	bstat[BULKSTAT_NR];
> +	char			descr[256];
> +	uint64_t		startino = 0;
> +	uint64_t		lastino = -1ULL;
> +	uint32_t		ocount;
> +	uint32_t		i;
> +	int			error;
> +
> +	if (agno != NULLAGNUMBER) {
> +		startino = cvt_agino_to_ino(&file->xfd, agno, 0);
> +		lastino = cvt_agino_to_ino(&file->xfd, agno + 1, 0) - 1;
> +	}
> +
> +	while ((error = xfrog_bulkstat(&file->xfd, &startino, BULKSTAT_NR,
> +			bstat, &ocount) == 0) && ocount > 0) {
> +		for (i = 0; i < ocount; i++) {
> +			if (bstat[i].bs_ino > lastino)
> +				goto out;
> +			snprintf(descr, sizeof(descr) - 1, _("inode %llu"),
> +					bstat[i].bs_ino);
> +			report_sick(descr, inode_flags, bstat[i].bs_sick,
> +					bstat[i].bs_checked);
> +		}
> +	}

This could be done as a do { } while loop:

	do {
		error = xfrog_bulkstat(&file->xfd, &startino, BULKSTAT_NR,
					bstat, &ocount);
		if (error)
			break;
		for (i = 0; i < ocount; i++) {
			[....]
		}
	} while (ocount > 0);

This could be done as a followup patch as it's not critical, just
a little bit of cleanup...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com