Message ID | 156774092210.2643497.7118033849671297049.stgit@magnolia (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xfsprogs: port utilities to bulkstat v5 | expand |
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.
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 --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);