Re: [PATCH 5/6] misc: convert from XFS_IOC_FSINUMBERS to XFS_IOC_INUMBERS

[Dave Chinner <david@fromorbit.com>]

On Thu, Sep 05, 2019 at 08:35:28PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Convert all programs to use the v5 inumbers ioctl.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  io/imap.c          |   26 +++++-----
>  io/open.c          |   27 +++++++----
>  libfrog/bulkstat.c |  132 ++++++++++++++++++++++++++++++++++++++++++++++------
>  libfrog/bulkstat.h |   10 +++-
>  scrub/fscounters.c |   21 +++++---
>  scrub/inodes.c     |   36 ++++++++------
>  6 files changed, 189 insertions(+), 63 deletions(-)

....
> diff --git a/io/open.c b/io/open.c
> index e1979501..e198bcd8 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -681,39 +681,46 @@ static __u64
>  get_last_inode(void)
>  {
>  	struct xfs_fd		xfd = XFS_FD_INIT(file->fd);
> -	uint64_t		lastip = 0;
> +	struct xfs_inumbers_req	*ireq;
>  	uint32_t		lastgrp = 0;
> -	uint32_t		ocount = 0;
>  	__u64			last_ino;

	__u64			last_ino = 0;

> -	struct xfs_inogrp	igroup[IGROUP_NR];
> +
> +	ireq = xfrog_inumbers_alloc_req(IGROUP_NR, 0);
> +	if (!ireq) {
> +		perror("alloc req");
> +		return 0;
> +	}
>  
>  	for (;;) {
>  		int		ret;
>  
> -		ret = xfrog_inumbers(&xfd, &lastip, IGROUP_NR, igroup,
> -				&ocount);
> +		ret = xfrog_inumbers(&xfd, ireq);
>  		if (ret) {
>  			errno = ret;
>  			perror("XFS_IOC_FSINUMBERS");
> +			free(ireq);
>  			return 0;

			goto out;
>  		}
>  
>  		/* Did we reach the last inode? */
> -		if (ocount == 0)
> +		if (ireq->hdr.ocount == 0)
>  			break;
>  
>  		/* last inode in igroup table */
> -		lastgrp = ocount;
> +		lastgrp = ireq->hdr.ocount;
>  	}
>  
> -	if (lastgrp == 0)
> +	if (lastgrp == 0) {
> +		free(ireq);
>  		return 0;

		goto out;
> +	}
>  
>  	lastgrp--;
>  
>  	/* The last inode number in use */
> -	last_ino = igroup[lastgrp].xi_startino +
> -		  libxfs_highbit64(igroup[lastgrp].xi_allocmask);
> +	last_ino = ireq->inumbers[lastgrp].xi_startino +
> +		  libxfs_highbit64(ireq->inumbers[lastgrp].xi_allocmask);

out:
> +	free(ireq);
>  
>  	return last_ino;
>  }
> diff --git a/libfrog/bulkstat.c b/libfrog/bulkstat.c
> index 2a70824e..748d0f32 100644
> --- a/libfrog/bulkstat.c
> +++ b/libfrog/bulkstat.c
> @@ -387,6 +387,86 @@ xfrog_bulkstat_alloc_req(
>  	return breq;
>  }
>  
> +/* Convert an inumbers (v5) struct to a inogrp (v1) struct. */
> +void
> +xfrog_inumbers_to_inogrp(
> +	struct xfs_inogrp		*ig1,
> +	const struct xfs_inumbers	*ig)
> +{
> +	ig1->xi_startino = ig->xi_startino;
> +	ig1->xi_alloccount = ig->xi_alloccount;
> +	ig1->xi_allocmask = ig->xi_allocmask;

Same thing - inumbers_v5_to_v1(from, to);

> +}
> +
> +/* Convert an inogrp (v1) struct to a inumbers (v5) struct. */
> +void
> +xfrog_inogrp_to_inumbers(
> +	struct xfs_inumbers		*ig,
> +	const struct xfs_inogrp		*ig1)

ditto.

> +{
> +	memset(ig, 0, sizeof(*ig));
> +	ig->xi_version = XFS_INUMBERS_VERSION_V1;
> +
> +	ig->xi_startino = ig1->xi_startino;
> +	ig->xi_alloccount = ig1->xi_alloccount;
> +	ig->xi_allocmask = ig1->xi_allocmask;
> +}
> +
> +static uint64_t xfrog_inum_ino(void *v1_rec)
> +{
> +	return ((struct xfs_inogrp *)v1_rec)->xi_startino;
> +}
> +
> +static void xfrog_inum_cvt(struct xfs_fd *xfd, void *v5, void *v1)
> +{
> +	xfrog_inogrp_to_inumbers(v5, v1);
> +}

what's the point of this wrapper?

> +
> +/* Query inode allocation bitmask information using v5 ioctl. */
> +static int
> +xfrog_inumbers5(
> +	struct xfs_fd		*xfd,
> +	struct xfs_inumbers_req	*req)
> +{
> +	int			ret;
> +
> +	ret = ioctl(xfd->fd, XFS_IOC_INUMBERS, req);
> +	if (ret)
> +		return errno;
> +	return 0;

negative errors.

> +}
> +
> +/* Query inode allocation bitmask information using v1 ioctl. */
> +static int
> +xfrog_inumbers1(
> +	struct xfs_fd		*xfd,
> +	struct xfs_inumbers_req	*req)
> +{
> +	struct xfs_fsop_bulkreq	bulkreq = { 0 };
> +	int			error;
> +
> +	error = xfrog_bulkstat_prep_v1_emulation(xfd);
> +	if (error)
> +		return error;
> +
> +	error = xfrog_bulk_req_setup(xfd, &req->hdr, &bulkreq,
> +			sizeof(struct xfs_inogrp));
> +	if (error == ECANCELED)
> +		goto out_teardown;
> +	if (error)
> +		return error;
> +
> +	error = ioctl(xfd->fd, XFS_IOC_FSINUMBERS, &bulkreq);
> +	if (error)
> +		error = errno;

negative errors.

> +
> +out_teardown:
> +	return xfrog_bulk_req_teardown(xfd, &req->hdr, &bulkreq,
> +			sizeof(struct xfs_inogrp), xfrog_inum_ino,
> +			&req->inumbers, sizeof(struct xfs_inumbers),
> +			xfrog_inum_cvt, 64, error);
> +}
....

>  	struct xfs_bulkstat	*bs;
> -	uint64_t		igrp_ino;
> -	uint32_t		igrplen = 0;
> +	struct xfs_inumbers	*inogrp;

Isn't that mixing v1 structure names with v5 operations? Aren't we
pulling infomration out in inode records?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com