On Tue, Jul 05, 2022 at 01:55:36PM +1000, Dave Chinner wrote:
> On Tue, Jul 05, 2022 at 08:49:58AM +0530, Srikanth C S wrote:
> > For a 2GB FS we have
> > $ mkfs.xfs -f -d agcount=129 test.img
> > mkfs.xfs: xfs_mkfs.c:3021: align_ag_geometry: Assertion `!cli_opt_set(&dopts, D_AGCOUNT)' failed.
> > Aborted
>
> Ok, that's because the size of the last AG is too small when trying
> to align the AG size to stripe geometry. It fails an assert that
> says "we should not get here if the agcount was specified on the
> CLI".
>
> >
> > With this change we have
> > $ mkfs.xfs -f -d agcount=129 test.img
> > Invalid value 129 for -d agcount option. Value is too large.
What version of mkfs is this?
$ truncate -s 2g /tmp/a
$ mkfs.xfs -V
mkfs.xfs version 5.18.0
$ mkfs.xfs -f -d agcount=129 /tmp/a
agsize (4065 blocks) too small, need at least 4096 blocks
> OK, but....
>
> >
> > Signed-off-by: Srikanth C S <srikanth.c.s@xxxxxxxxxx>
> > ---
> > mkfs/xfs_mkfs.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 057b3b09..32dcbfff 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -2897,6 +2897,13 @@ _("agsize (%s) not a multiple of fs blk size (%d)\n"),
> > cfg->agcount = cli->agcount;
> > cfg->agsize = cfg->dblocks / cfg->agcount +
> > (cfg->dblocks % cfg->agcount != 0);
> > + if (cfg->agsize < XFS_AG_MIN_BYTES >> cfg->blocklog)
> > + {
> > + fprintf(stderr,
> > +_("Invalid value %lld for -d agcount option. Value is too large.\n"),
> > + (long long)cli->agcount);
> > + usage();
> > + }
>
> .... that's not where we validate the calculated ag size. That
> happens via align_ag_geometry() -> validate_ag_geometry(). i.e. we
> can't reject an AG specification until after we've applied all the
> necessary modifiers to it first (such as stripe alignment
> requirements).
>
> IOWs, we do actually check for valid AG sizes, it's just that this
> specific case hit an ASSERT() check before we got to validating the
> AG size. I'm pretty sure just removing the ASSERT - which assumes
> that "-d agcount=xxx" is not so large that it produces undersized
> AGs - will fix the problem and result in the correct error message
> being returned.
(Agreed.)
--D
> Cheers,
>
> Dave.
>
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
