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

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

On Fri, Sep 13, 2019 at 11:10:36AM +1000, Dave Chinner wrote:
> 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:

Ok, fixed.

> > +	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.

Fixed too.

> > +{
> > +	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?

Function adapter so we can use xfrog_bulk_req_teardown as part of using
the V1 inumbers ioctl to emulate the V5 inumbers ioctl.

> > +
> > +/* 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?

Yeah, I'll fix the names too.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com