| Message ID | 20200916204056.29247-1-preichl@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] mkfs.xfs: fix ASSERT on too-small device with stripe geometry | expand |
On Wed, Sep 16, 2020 at 10:40:56PM +0200, Pavel Reichl wrote: > When a too-small device is created with stripe geometry, we hit an > assert in align_ag_geometry(): > > mkfs.xfs: xfs_mkfs.c:2834: align_ag_geometry: Assertion `cfg->agcount != 0' failed. > > This is because align_ag_geometry() finds that the size of the last > (only) AG is too small, and attempts to trim it off. Obviously 0 > AGs is invalid, and we hit the ASSERT. > > Reported-by: Zdenek Kabelac <zkabelac@redhat.com> > Suggested-by: Dave Chinner <dchinner@redhat.com> > Signed-off-by: Pavel Reichl <preichl@redhat.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Sep 16, 2020 at 10:40:56PM +0200, Pavel Reichl wrote: > When a too-small device is created with stripe geometry, we hit an > assert in align_ag_geometry(): > > mkfs.xfs: xfs_mkfs.c:2834: align_ag_geometry: Assertion `cfg->agcount != 0' failed. > > This is because align_ag_geometry() finds that the size of the last > (only) AG is too small, and attempts to trim it off. Obviously 0 > AGs is invalid, and we hit the ASSERT. > Patch looks good, next time please keep the Changelog :) Cheers. Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> > Reported-by: Zdenek Kabelac <zkabelac@redhat.com> > Suggested-by: Dave Chinner <dchinner@redhat.com> > Signed-off-by: Pavel Reichl <preichl@redhat.com> > --- > include/xfs_multidisk.h | 14 +++++++------- > mkfs/xfs_mkfs.c | 6 +++--- > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h > index 1b9936ec..abfb50ce 100644 > --- a/include/xfs_multidisk.h > +++ b/include/xfs_multidisk.h > @@ -14,7 +14,6 @@ > #define XFS_DFL_BLOCKSIZE_LOG 12 /* 4096 byte blocks */ > #define XFS_DINODE_DFL_LOG 8 /* 256 byte inodes */ > #define XFS_DINODE_DFL_CRC_LOG 9 /* 512 byte inodes for CRCs */ > -#define XFS_MIN_DATA_BLOCKS 100 > #define XFS_MIN_INODE_PERBLOCK 2 /* min inodes per block */ > #define XFS_DFL_IMAXIMUM_PCT 25 /* max % of space for inodes */ > #define XFS_MIN_REC_DIRSIZE 12 /* 4096 byte dirblocks (V2) */ > @@ -25,13 +24,14 @@ > * accept w/o warnings > */ > > -#define XFS_AG_BYTES(bblog) ((long long)BBSIZE << (bblog)) > -#define XFS_AG_MIN_BYTES ((XFS_AG_BYTES(15))) /* 16 MB */ > -#define XFS_AG_MAX_BYTES ((XFS_AG_BYTES(31))) /* 1 TB */ > -#define XFS_AG_MIN_BLOCKS(blog) (XFS_AG_MIN_BYTES >> (blog)) > -#define XFS_AG_MAX_BLOCKS(blog) ((XFS_AG_MAX_BYTES - 1) >> (blog)) > +#define XFS_AG_BYTES(bblog) ((long long)BBSIZE << (bblog)) > +#define XFS_MIN_DATA_BLOCKS(cfg) (XFS_AG_MIN_BLOCKS((cfg)->blocklog)) > +#define XFS_AG_MIN_BYTES ((XFS_AG_BYTES(15))) /* 16 MB */ > +#define XFS_AG_MAX_BYTES ((XFS_AG_BYTES(31))) /* 1 TB */ > +#define XFS_AG_MIN_BLOCKS(blog) (XFS_AG_MIN_BYTES >> (blog)) > +#define XFS_AG_MAX_BLOCKS(blog) ((XFS_AG_MAX_BYTES - 1) >> (blog)) > > -#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1)) > +#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1)) > > /* > * These values define what we consider a "multi-disk" filesystem. That is, a > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index a687f385..204dfff1 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -2530,10 +2530,10 @@ _("size %s specified for data subvolume is too large, maximum is %lld blocks\n") > cfg->dblocks = DTOBT(xi->dsize, cfg->blocklog); > } > > - if (cfg->dblocks < XFS_MIN_DATA_BLOCKS) { > + if (cfg->dblocks < XFS_MIN_DATA_BLOCKS(cfg)) { > fprintf(stderr, > -_("size %lld of data subvolume is too small, minimum %d blocks\n"), > - (long long)cfg->dblocks, XFS_MIN_DATA_BLOCKS); > +_("size %lld of data subvolume is too small, minimum %lld blocks\n"), > + (long long)cfg->dblocks, XFS_MIN_DATA_BLOCKS(cfg)); > usage(); > } > > -- > 2.26.2 >
diff --git a/include/xfs_multidisk.h b/include/xfs_multidisk.h index 1b9936ec..abfb50ce 100644 --- a/include/xfs_multidisk.h +++ b/include/xfs_multidisk.h @@ -14,7 +14,6 @@ #define XFS_DFL_BLOCKSIZE_LOG 12 /* 4096 byte blocks */ #define XFS_DINODE_DFL_LOG 8 /* 256 byte inodes */ #define XFS_DINODE_DFL_CRC_LOG 9 /* 512 byte inodes for CRCs */ -#define XFS_MIN_DATA_BLOCKS 100 #define XFS_MIN_INODE_PERBLOCK 2 /* min inodes per block */ #define XFS_DFL_IMAXIMUM_PCT 25 /* max % of space for inodes */ #define XFS_MIN_REC_DIRSIZE 12 /* 4096 byte dirblocks (V2) */ @@ -25,13 +24,14 @@ * accept w/o warnings */ -#define XFS_AG_BYTES(bblog) ((long long)BBSIZE << (bblog)) -#define XFS_AG_MIN_BYTES ((XFS_AG_BYTES(15))) /* 16 MB */ -#define XFS_AG_MAX_BYTES ((XFS_AG_BYTES(31))) /* 1 TB */ -#define XFS_AG_MIN_BLOCKS(blog) (XFS_AG_MIN_BYTES >> (blog)) -#define XFS_AG_MAX_BLOCKS(blog) ((XFS_AG_MAX_BYTES - 1) >> (blog)) +#define XFS_AG_BYTES(bblog) ((long long)BBSIZE << (bblog)) +#define XFS_MIN_DATA_BLOCKS(cfg) (XFS_AG_MIN_BLOCKS((cfg)->blocklog)) +#define XFS_AG_MIN_BYTES ((XFS_AG_BYTES(15))) /* 16 MB */ +#define XFS_AG_MAX_BYTES ((XFS_AG_BYTES(31))) /* 1 TB */ +#define XFS_AG_MIN_BLOCKS(blog) (XFS_AG_MIN_BYTES >> (blog)) +#define XFS_AG_MAX_BLOCKS(blog) ((XFS_AG_MAX_BYTES - 1) >> (blog)) -#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1)) +#define XFS_MAX_AGNUMBER ((xfs_agnumber_t)(NULLAGNUMBER - 1)) /* * These values define what we consider a "multi-disk" filesystem. That is, a diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index a687f385..204dfff1 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -2530,10 +2530,10 @@ _("size %s specified for data subvolume is too large, maximum is %lld blocks\n") cfg->dblocks = DTOBT(xi->dsize, cfg->blocklog); } - if (cfg->dblocks < XFS_MIN_DATA_BLOCKS) { + if (cfg->dblocks < XFS_MIN_DATA_BLOCKS(cfg)) { fprintf(stderr, -_("size %lld of data subvolume is too small, minimum %d blocks\n"), - (long long)cfg->dblocks, XFS_MIN_DATA_BLOCKS); +_("size %lld of data subvolume is too small, minimum %lld blocks\n"), + (long long)cfg->dblocks, XFS_MIN_DATA_BLOCKS(cfg)); usage(); }
When a too-small device is created with stripe geometry, we hit an assert in align_ag_geometry(): mkfs.xfs: xfs_mkfs.c:2834: align_ag_geometry: Assertion `cfg->agcount != 0' failed. This is because align_ag_geometry() finds that the size of the last (only) AG is too small, and attempts to trim it off. Obviously 0 AGs is invalid, and we hit the ASSERT. Reported-by: Zdenek Kabelac <zkabelac@redhat.com> Suggested-by: Dave Chinner <dchinner@redhat.com> Signed-off-by: Pavel Reichl <preichl@redhat.com> --- include/xfs_multidisk.h | 14 +++++++------- mkfs/xfs_mkfs.c | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-)