> 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
>
For the same version I get Assertion failed
$ truncate -s 2g /tmp/a
$ mkfs.xfs -V
mkfs.xfs version 5.18.0
$ mkfs.xfs -f -d agcount=129 /tmp/a
mkfs.xfs: xfs_mkfs.c:3033: align_ag_geometry: Assertion `!cli_opt_set(&dopts, D_AGCOUNT)' failed.
Aborted (core dumped)
> > 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
