diff mbox series

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

Message ID 156774092210.2643497.7118033849671297049.stgit@magnolia (mailing list archive)
State New, archived
Headers show
Series xfsprogs: port utilities to bulkstat v5 | expand

Commit Message

Darrick J. Wong Sept. 6, 2019, 3:35 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fsr/xfs_fsr.c      |    8 +++-
 io/open.c          |    6 ++-
 io/swapext.c       |    4 ++
 libfrog/bulkstat.c |  103 ++++++++++++++++++++++++++++++++++++++++++++--------
 libfrog/bulkstat.h |    4 +-
 scrub/inodes.c     |    8 +---
 spaceman/health.c  |    4 +-
 7 files changed, 105 insertions(+), 32 deletions(-)

Comments

Dave Chinner Sept. 13, 2019, 1:02 a.m. UTC | #1 
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.
Darrick J. Wong Sept. 16, 2019, 10:02 p.m. UTC | #2 
On Fri, Sep 13, 2019 at 11:02:37AM +1000, Dave Chinner wrote:
> 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?

Yes.  I think a lof of the retained bulkstat weirdness in fsr could go
away if (a) we maintained an open xfs_fd to the filesystem and (b)
wrapped the swapext ioctl... but there's already too much here so I
stopped short of refactoring fsr.

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

Ok.

--D

> -Dave.
> 
> -- 
> Dave Chinner
> david@fromorbit.com
diff mbox series

Patch

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);
 
 	fd = jdm_open(fshandlep, &statbuf, O_RDWR|O_DIRECT);
 	if (fd < 0) {
@@ -974,7 +976,7 @@  fsr_setup_attr_fork(
 
 	i = 0;
 	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));
diff --git a/io/open.c b/io/open.c
index e1aac7d1..e1979501 100644
--- a/io/open.c
+++ b/io/open.c
@@ -723,7 +723,7 @@  inode_f(
 	int			argc,
 	char			**argv)
 {
-	struct xfs_bstat	bstat;
+	struct xfs_bulkstat	bulkstat;
 	uint64_t		result_ino = 0;
 	uint64_t		userino = NULLFSINO;
 	char			*p;
@@ -803,7 +803,7 @@  inode_f(
 		struct xfs_fd	xfd = XFS_FD_INIT(file->fd);
 
 		/* get this inode */
-		ret = xfrog_bulkstat_single(&xfd, userino, &bstat);
+		ret = xfrog_bulkstat_single(&xfd, userino, 0, &bulkstat);
 		if (ret == EINVAL) {
 			/* Not in use */
 			result_ino = 0;
@@ -813,7 +813,7 @@  inode_f(
 			exitcode = 1;
 			return 0;
 		} else {
-			result_ino = bstat.bs_ino;
+			result_ino = bulkstat.bs_ino;
 		}
 	}
 
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);
 	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;
+
+	req = xfrog_bulkstat_alloc_req(1, ino);
+	if (!req)
+		return ENOMEM;
+
+	req->hdr.flags = flags;
+	ret = ioctl(xfd->fd, XFS_IOC_BULKSTAT, req);
+	if (ret) {
+		ret = errno;
+		goto free;
+	}
+
+	if (req->hdr.ocount == 0) {
+		ret = ENOENT;
+		goto free;
+	}
+
+	memcpy(bulkstat, req->bulkstat, sizeof(struct xfs_bulkstat));
+free:
+	free(req);
+	return ret;
+}
+
+/* Bulkstat a single inode using v1 ioctl. */
+static int
+xfrog_bulkstat_single1(
+	struct xfs_fd			*xfd,
+	uint64_t			ino,
+	unsigned int			flags,
+	struct xfs_bulkstat		*bulkstat)
+{
+	struct xfs_bstat		bstat;
+	struct xfs_fsop_bulkreq		bulkreq = { 0 };
+	int				error;
+
+	if (flags)
+		return EINVAL;
+
+	error = xfrog_bulkstat_prep_v1_emulation(xfd);
+	if (error)
+		return error;
+
+	bulkreq.lastip = (__u64 *)&ino;
+	bulkreq.icount = 1;
+	bulkreq.ubuffer = &bstat;
+	error = ioctl(xfd->fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
+	if (error)
+		return errno;
+
+	xfrog_bstat_to_bulkstat(xfd, bulkstat, &bstat);
+	return 0;
+}
+
 /* Bulkstat a single inode.  Returns zero or a positive error code. */
 int
 xfrog_bulkstat_single(
-	struct xfs_fd		*xfd,
-	uint64_t		ino,
-	struct xfs_bstat	*ubuffer)
+	struct xfs_fd			*xfd,
+	uint64_t			ino,
+	unsigned int			flags,
+	struct xfs_bulkstat		*bulkstat)
 {
-	__u64			i = ino;
-	struct xfs_fsop_bulkreq	bulkreq = {
-		.lastip		= &i,
-		.icount		= 1,
-		.ubuffer	= ubuffer,
-		.ocount		= NULL,
-	};
-	int			ret;
+	int				error;
 
-	ret = ioctl(xfd->fd, XFS_IOC_FSBULKSTAT_SINGLE, &bulkreq);
-	if (ret)
-		return errno;
-	return 0;
+	if (xfd->flags & XFROG_FLAG_BULKSTAT_FORCE_V1)
+		goto try_v1;
+
+	error = xfrog_bulkstat_single5(xfd, ino, flags, bulkstat);
+	if (error == 0 || (xfd->flags & XFROG_FLAG_BULKSTAT_FORCE_V5))
+		return error;
+
+	/* If the v5 ioctl wasn't found, we punt to v1. */
+	switch (error) {
+	case EOPNOTSUPP:
+	case ENOTTY:
+		xfd->flags |= XFROG_FLAG_BULKSTAT_FORCE_V1;
+		break;
+	}
+
+try_v1:
+	return xfrog_bulkstat_single1(xfd, ino, flags, bulkstat);
 }
 
 /*
diff --git a/libfrog/bulkstat.h b/libfrog/bulkstat.h
index 6f51c167..3135e752 100644
--- a/libfrog/bulkstat.h
+++ b/libfrog/bulkstat.h
@@ -8,8 +8,8 @@ 
 
 /* Bulkstat wrappers */
 struct xfs_bstat;
-int xfrog_bulkstat_single(struct xfs_fd *xfd, uint64_t ino,
-		struct xfs_bstat *ubuffer);
+int xfrog_bulkstat_single(struct xfs_fd *xfd, uint64_t ino, unsigned int flags,
+		struct xfs_bulkstat *bulkstat);
 int xfrog_bulkstat(struct xfs_fd *xfd, struct xfs_bulkstat_req *req);
 
 struct xfs_bulkstat_req *xfrog_bulkstat_alloc_req(uint32_t nr,
diff --git a/scrub/inodes.c b/scrub/inodes.c
index 851c24bd..2112c9d1 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -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++;
 	}
diff --git a/spaceman/health.c b/spaceman/health.c
index 9bed7fdf..b6e1fcd9 100644
--- a/spaceman/health.c
+++ b/spaceman/health.c
@@ -208,7 +208,7 @@  report_inode_health(
 	unsigned long long	ino,
 	const char		*descr)
 {
-	struct xfs_bstat	bs;
+	struct xfs_bulkstat	bs;
 	char			d[256];
 	int			ret;
 
@@ -217,7 +217,7 @@  report_inode_health(
 		descr = d;
 	}
 
-	ret = xfrog_bulkstat_single(&file->xfd, ino, &bs);
+	ret = xfrog_bulkstat_single(&file->xfd, ino, 0, &bs);
 	if (ret) {
 		errno = ret;
 		perror(descr);