Re: [PATCH 4/6] misc: convert to v5 bulkstat_single call

[Dave Chinner <david@fromorbit.com>]

On Thu, Sep 05, 2019 at 08:35:22PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  spaceman/health.c  |    4 +-
>  7 files changed, 105 insertions(+), 32 deletions(-)
> 
> 
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index cc3cc93a..e8fa39ab 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -724,6 +724,7 @@ fsrfile(
>  	xfs_ino_t		ino)
>  {
>  	struct xfs_fd		fsxfd = XFS_FD_INIT_EMPTY;
> +	struct xfs_bulkstat	bulkstat;
>  	struct xfs_bstat	statbuf;
>  	jdm_fshandle_t		*fshandlep;
>  	int			fd = -1;
> @@ -748,12 +749,13 @@ fsrfile(
>  		goto out;
>  	}
>  
> -	error = xfrog_bulkstat_single(&fsxfd, ino, &statbuf);
> +	error = xfrog_bulkstat_single(&fsxfd, ino, 0, &bulkstat);
>  	if (error) {
>  		fsrprintf(_("unable to get bstat on %s: %s\n"),
>  			fname, strerror(error));
>  		goto out;
>  	}
> +	xfrog_bulkstat_to_bstat(&fsxfd, &statbuf, &bulkstat);

So this is so none of the rest of fsr needs to be converted to use
the new structure versions? When will this go away?

>  	do {
> -		struct xfs_bstat tbstat;
> +		struct xfs_bulkstat	tbstat;
>  		char		name[64];
>  		int		ret;
>  
> @@ -983,7 +985,7 @@ fsr_setup_attr_fork(
>  		 * this to compare against the target and determine what we
>  		 * need to do.
>  		 */
> -		ret = xfrog_bulkstat_single(&txfd, tstatbuf.st_ino, &tbstat);
> +		ret = xfrog_bulkstat_single(&txfd, tstatbuf.st_ino, 0, &tbstat);
>  		if (ret) {
>  			fsrprintf(_("unable to get bstat on temp file: %s\n"),
>  						strerror(ret));

Because this looks like we now have a combination of v1 and v5
structures being used...

>  
> diff --git a/io/swapext.c b/io/swapext.c
> index 2b4918f8..ca024b93 100644
> --- a/io/swapext.c
> +++ b/io/swapext.c
> @@ -28,6 +28,7 @@ swapext_f(
>  	char			**argv)
>  {
>  	struct xfs_fd		fxfd = XFS_FD_INIT(file->fd);
> +	struct xfs_bulkstat	bulkstat;
>  	int			fd;
>  	int			error;
>  	struct xfs_swapext	sx;
> @@ -48,12 +49,13 @@ swapext_f(
>  		goto out;
>  	}
>  
> -	error = xfrog_bulkstat_single(&fxfd, stat.st_ino, &sx.sx_stat);
> +	error = xfrog_bulkstat_single(&fxfd, stat.st_ino, 0, &bulkstat);
>  	if (error) {
>  		errno = error;
>  		perror("bulkstat");
>  		goto out;
>  	}
> +	xfrog_bulkstat_to_bstat(&fxfd, &sx.sx_stat, &bulkstat);

and this is required because bstat is part of the swapext ioctl ABI?

>  	sx.sx_version = XFS_SX_VERSION;
>  	sx.sx_fdtarget = file->fd;
>  	sx.sx_fdtmp = fd;
> diff --git a/libfrog/bulkstat.c b/libfrog/bulkstat.c
> index b4468243..2a70824e 100644
> --- a/libfrog/bulkstat.c
> +++ b/libfrog/bulkstat.c
> @@ -20,26 +20,99 @@ xfrog_bulkstat_prep_v1_emulation(
>  	return xfd_prepare_geometry(xfd);
>  }
>  
> +/* Bulkstat a single inode using v5 ioctl. */
> +static int
> +xfrog_bulkstat_single5(
> +	struct xfs_fd			*xfd,
> +	uint64_t			ino,
> +	unsigned int			flags,
> +	struct xfs_bulkstat		*bulkstat)
> +{
> +	struct xfs_bulkstat_req		*req;
> +	int				ret;
> +
> +	if (flags & ~(XFS_BULK_IREQ_SPECIAL))
> +		return EINVAL;

negative error returns, please.

> @@ -57,8 +57,6 @@ xfs_iterate_inodes_range_check(
>  	int			error;
>  
>  	for (i = 0, bs = bstat; i < XFS_INODES_PER_CHUNK; i++) {
> -		struct xfs_bstat bs1;
> -
>  		if (!(inogrp->xi_allocmask & (1ULL << i)))
>  			continue;
>  		if (bs->bs_ino == inogrp->xi_startino + i) {
> @@ -68,13 +66,11 @@ xfs_iterate_inodes_range_check(
>  
>  		/* Load the one inode. */
>  		error = xfrog_bulkstat_single(&ctx->mnt,
> -				inogrp->xi_startino + i, &bs1);
> -		if (error || bs1.bs_ino != inogrp->xi_startino + i) {
> +				inogrp->xi_startino + i, 0, bs);
> +		if (error || bs->bs_ino != inogrp->xi_startino + i) {
>  			memset(bs, 0, sizeof(struct xfs_bulkstat));
>  			bs->bs_ino = inogrp->xi_startino + i;
>  			bs->bs_blksize = ctx->mnt_sv.f_frsize;
> -		} else {
> -			xfrog_bstat_to_bulkstat(&ctx->mnt, bs, &bs1);
>  		}
>  		bs++;
>  	}

So this immediately tears down the confusing stuff that was set up
in the previous patch. Perhaps separate out the scrub changes and do
both bulkstat and bulkstat_single conversions in one patch?

-Dave.

-- 
Dave Chinner
david@fromorbit.com