On Thu, Sep 26, 2019 at 04:01:43PM -0500, Eric Sandeen wrote:
> On 9/25/19 4:32 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Convert xfrog_bulkstat() and xfrog_bulkstat_single() to take arguments
> > using v5 bulkstat semantics and return bulkstat information in v5
> > structures. If the v5 ioctl is not available, the xfrog wrapper should
> > use the v1 ioctl to emulate v5 behaviors. Add flags to the xfs_fd
> > structure to constrain emulation for debugging purposes.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> gawd this is a lot of frobnication of ioctl results but I ... guess there's
> no way around it.
>
> Presumably we want all callers to use v5 for the bigger fields, right,
> so it's not like we can just leave some old v1 callers as-is if they don't
> need new fields ....
Well... in theory we could leave them, but eventually we'll either want
new functionality or we'll want to deprecate the old ones.
> > ---
> > fsr/xfs_fsr.c | 64 ++++++--
> > io/open.c | 27 ++-
> > io/swapext.c | 9 +
> > libfrog/bulkstat.c | 431 +++++++++++++++++++++++++++++++++++++++++++++++++---
> > libfrog/bulkstat.h | 14 +-
> > libfrog/fsgeom.h | 9 +
> > quota/quot.c | 29 ++-
> > scrub/inodes.c | 39 +++--
> > scrub/inodes.h | 2
> > scrub/phase3.c | 6 -
> > scrub/phase5.c | 8 -
> > scrub/phase6.c | 2
> > scrub/unicrash.c | 6 -
> > scrub/unicrash.h | 4
> > spaceman/health.c | 33 ++--
> > 15 files changed, 572 insertions(+), 111 deletions(-)
> >
> >
> > diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> > index a53eb924..af5d6169 100644
> > --- a/fsr/xfs_fsr.c
> > +++ b/fsr/xfs_fsr.c
<snip>
> > @@ -623,7 +643,14 @@ fsrfs(char *mntdir, xfs_ino_t startino, int targetrange)
> > (p->bs_extents < 2))
> > continue;
> >
> > - fd = jdm_open(fshandlep, p, O_RDWR|O_DIRECT);
> > + ret = xfrog_bulkstat_v5_to_v1(&fsxfd, &bs1, p);
>
> ew. In the long run, I guess I'd rather convert these to take v5 when needed
> but alas. I guess that has xfsdump implications too. :(
Worse -- these are public libhandle functions, so we'll have to upgrade
its interfaces very carefully.
> > + if (ret) {
> > + fsrprintf(_("bstat conversion error: %s\n"),
> > + strerror(ret));
> > + continue;
> > + }
>
> ... but then we wouldn't potential fail on bstats w/ big numbers :(
>
> Oh well, another day.
>
> > +
> > + fd = jdm_open(fshandlep, &bs1, O_RDWR | O_DIRECT);
> > if (fd < 0) {
> > /* This probably means the file was
> > * removed while in progress of handling
<snip>
> > diff --git a/scrub/inodes.c b/scrub/inodes.c
> > index 580a845e..2112c9d1 100644
> > --- a/scrub/inodes.c
> > +++ b/scrub/inodes.c
<snip>
> > @@ -135,10 +135,12 @@ xfs_iterate_inodes_range(
> > errbuf, DESCR_BUFSZ));
> > }
> >
> > - xfs_iterate_inodes_range_check(ctx, &inogrp, bstat);
> > + xfs_iterate_inodes_range_check(ctx, &inogrp, breq->bulkstat);
> >
> > /* Iterate all the inodes. */
> > - for (i = 0, bs = bstat; i < inogrp.xi_alloccount; i++, bs++) {
> > + for (i = 0, bs = breq->bulkstat;
> > + i < inogrp.xi_alloccount;
> > + i++, bs++) {
> > if (bs->bs_ino > last_ino)
> > goto out;
>
> leaks the breq here, no?
>
> >
> > @@ -184,6 +186,7 @@ _("Changed too many times during scan; giving up."));
> > str_liberror(ctx, error, descr);
> > moveon = false;
> > }
> > + free(breq);
> > out:
>
> maybe free should be here?
Ugh, I think I mismerged that. Fixed. :(
--D
> > diff --git a/scrub/inodes.h b/scrub/inodes.h
> > index 631848c3..3341c6d9 100644
> > --- a/scrub/inodes.h
> > +++ b/scrub/inodes.h
> > @@ -7,7 +7,7 @@
> > #define XFS_SCRUB_INODES_H_
> >
> > typedef int (*xfs_inode_iter_fn)(struct scrub_ctx *ctx,
> > - struct xfs_handle *handle, struct xfs_bstat *bs, void *arg);
> > + struct xfs_handle *handle, struct xfs_bulkstat *bs, void *arg);
> >
> > #define XFS_ITERATE_INODES_ABORT (-1)
> > bool xfs_scan_all_inodes(struct scrub_ctx *ctx, xfs_inode_iter_fn fn,
> > diff --git a/scrub/phase3.c b/scrub/phase3.c
> > index 81c64cd1..a32d1ced 100644
> > --- a/scrub/phase3.c
> > +++ b/scrub/phase3.c
> > @@ -30,7 +30,7 @@ xfs_scrub_fd(
> > struct scrub_ctx *ctx,
> > bool (*fn)(struct scrub_ctx *ctx, uint64_t ino,
> > uint32_t gen, struct xfs_action_list *a),
> > - struct xfs_bstat *bs,
> > + struct xfs_bulkstat *bs,
> > struct xfs_action_list *alist)
> > {
> > return fn(ctx, bs->bs_ino, bs->bs_gen, alist);
> > @@ -45,7 +45,7 @@ struct scrub_inode_ctx {
> > static void
> > xfs_scrub_inode_vfs_error(
> > struct scrub_ctx *ctx,
> > - struct xfs_bstat *bstat)
> > + struct xfs_bulkstat *bstat)
> > {
> > char descr[DESCR_BUFSZ];
> > xfs_agnumber_t agno;
> > @@ -65,7 +65,7 @@ static int
> > xfs_scrub_inode(
> > struct scrub_ctx *ctx,
> > struct xfs_handle *handle,
> > - struct xfs_bstat *bstat,
> > + struct xfs_bulkstat *bstat,
> > void *arg)
> > {
> > struct xfs_action_list alist;
> > diff --git a/scrub/phase5.c b/scrub/phase5.c
> > index 3ff34251..99cd51b2 100644
> > --- a/scrub/phase5.c
> > +++ b/scrub/phase5.c
> > @@ -80,7 +80,7 @@ xfs_scrub_scan_dirents(
> > struct scrub_ctx *ctx,
> > const char *descr,
> > int *fd,
> > - struct xfs_bstat *bstat)
> > + struct xfs_bulkstat *bstat)
> > {
> > struct unicrash *uc = NULL;
> > DIR *dir;
> > @@ -140,7 +140,7 @@ xfs_scrub_scan_fhandle_namespace_xattrs(
> > struct scrub_ctx *ctx,
> > const char *descr,
> > struct xfs_handle *handle,
> > - struct xfs_bstat *bstat,
> > + struct xfs_bulkstat *bstat,
> > const struct attrns_decode *attr_ns)
> > {
> > struct attrlist_cursor cur;
> > @@ -200,7 +200,7 @@ xfs_scrub_scan_fhandle_xattrs(
> > struct scrub_ctx *ctx,
> > const char *descr,
> > struct xfs_handle *handle,
> > - struct xfs_bstat *bstat)
> > + struct xfs_bulkstat *bstat)
> > {
> > const struct attrns_decode *ns;
> > bool moveon = true;
> > @@ -228,7 +228,7 @@ static int
> > xfs_scrub_connections(
> > struct scrub_ctx *ctx,
> > struct xfs_handle *handle,
> > - struct xfs_bstat *bstat,
> > + struct xfs_bulkstat *bstat,
> > void *arg)
> > {
> > bool *pmoveon = arg;
> > diff --git a/scrub/phase6.c b/scrub/phase6.c
> > index 506e75d2..b41f90e0 100644
> > --- a/scrub/phase6.c
> > +++ b/scrub/phase6.c
> > @@ -172,7 +172,7 @@ static int
> > xfs_report_verify_inode(
> > struct scrub_ctx *ctx,
> > struct xfs_handle *handle,
> > - struct xfs_bstat *bstat,
> > + struct xfs_bulkstat *bstat,
> > void *arg)
> > {
> > char descr[DESCR_BUFSZ];
> > diff --git a/scrub/unicrash.c b/scrub/unicrash.c
> > index 17e8f34f..b02c5658 100644
> > --- a/scrub/unicrash.c
> > +++ b/scrub/unicrash.c
> > @@ -432,7 +432,7 @@ unicrash_init(
> > */
> > static bool
> > is_only_root_writable(
> > - struct xfs_bstat *bstat)
> > + struct xfs_bulkstat *bstat)
> > {
> > if (bstat->bs_uid != 0 || bstat->bs_gid != 0)
> > return false;
> > @@ -444,7 +444,7 @@ bool
> > unicrash_dir_init(
> > struct unicrash **ucp,
> > struct scrub_ctx *ctx,
> > - struct xfs_bstat *bstat)
> > + struct xfs_bulkstat *bstat)
> > {
> > /*
> > * Assume 64 bytes per dentry, clamp buckets between 16 and 64k.
> > @@ -459,7 +459,7 @@ bool
> > unicrash_xattr_init(
> > struct unicrash **ucp,
> > struct scrub_ctx *ctx,
> > - struct xfs_bstat *bstat)
> > + struct xfs_bulkstat *bstat)
> > {
> > /* Assume 16 attributes per extent for lack of a better idea. */
> > return unicrash_init(ucp, ctx, false, 16 * (1 + bstat->bs_aextents),
> > diff --git a/scrub/unicrash.h b/scrub/unicrash.h
> > index fb8f5f72..feb9cc86 100644
> > --- a/scrub/unicrash.h
> > +++ b/scrub/unicrash.h
> > @@ -14,9 +14,9 @@ struct unicrash;
> > struct dirent;
> >
> > bool unicrash_dir_init(struct unicrash **ucp, struct scrub_ctx *ctx,
> > - struct xfs_bstat *bstat);
> > + struct xfs_bulkstat *bstat);
> > bool unicrash_xattr_init(struct unicrash **ucp, struct scrub_ctx *ctx,
> > - struct xfs_bstat *bstat);
> > + struct xfs_bulkstat *bstat);
> > bool unicrash_fs_label_init(struct unicrash **ucp, struct scrub_ctx *ctx);
> > void unicrash_free(struct unicrash *uc);
> > bool unicrash_check_dir_name(struct unicrash *uc, const char *descr,
> > diff --git a/spaceman/health.c b/spaceman/health.c
> > index a8bd3f3e..b195a229 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);
> > @@ -266,11 +266,10 @@ static int
> > report_bulkstat_health(
> > xfs_agnumber_t agno)
> > {
> > - struct xfs_bstat bstat[BULKSTAT_NR];
> > + struct xfs_bulkstat_req *breq;
> > char descr[256];
> > uint64_t startino = 0;
> > uint64_t lastino = -1ULL;
> > - uint32_t ocount;
> > uint32_t i;
> > int error;
> >
> > @@ -279,26 +278,34 @@ report_bulkstat_health(
> > lastino = cvt_agino_to_ino(&file->xfd, agno + 1, 0) - 1;
> > }
> >
> > + breq = xfrog_bulkstat_alloc_req(BULKSTAT_NR, startino);
> > + if (!breq) {
> > + perror("bulk alloc req");
> > + exitcode = 1;
> > + return 1;
> > + }
> > +
> > do {
> > - error = xfrog_bulkstat(&file->xfd, &startino, BULKSTAT_NR,
> > - bstat, &ocount);
> > + error = xfrog_bulkstat(&file->xfd, breq);
> > if (error)
> > break;
> > - for (i = 0; i < ocount; i++) {
> > - if (bstat[i].bs_ino > lastino)
> > + for (i = 0; i < breq->hdr.ocount; i++) {
> > + if (breq->bulkstat[i].bs_ino > lastino)
> > goto out;
> > - snprintf(descr, sizeof(descr) - 1, _("inode %llu"),
> > - bstat[i].bs_ino);
> > - report_sick(descr, inode_flags, bstat[i].bs_sick,
> > - bstat[i].bs_checked);
> > + snprintf(descr, sizeof(descr) - 1, _("inode %"PRIu64),
> > + breq->bulkstat[i].bs_ino);
> > + report_sick(descr, inode_flags,
> > + breq->bulkstat[i].bs_sick,
> > + breq->bulkstat[i].bs_checked);
> > }
> > - } while (ocount > 0);
> > + } while (breq->hdr.ocount > 0);
> >
> > if (error) {
> > errno = error;
> > perror("bulkstat");
> > }
> > out:
> > + free(breq);
> > return error;
> > }
> >
> >
