On Fri, Oct 09, 2020 at 01:24:21PM +0800, Gao Xiang wrote: > Check stripe numbers in calc_stripe_factors() by using > xfs_validate_stripe_factors(). > > Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx> > --- > libxfs/libxfs_api_defs.h | 1 + > mkfs/xfs_mkfs.c | 23 +++++++---------------- > 2 files changed, 8 insertions(+), 16 deletions(-) > ... > @@ -2344,11 +2334,12 @@ _("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"), > > /* if no stripe config set, use the device default */ > if (!dsunit) { > - /* Ignore nonsense from device. XXX add more validation */ > - if (ft->dsunit && ft->dswidth == 0) { > + /* Ignore nonsense from device report. */ > + if (!libxfs_validate_stripe_factors(NULL, BBTOB(ft->dsunit), > + BBTOB(ft->dswidth), 0)) { The logic seems fine and from the previous comment it sounds like we're lacking validation in this particular scenario, but do we want to print more error noise from the validation helper in scenarios where failure is not a fatal error? Brian > fprintf(stderr, > -_("%s: Volume reports stripe unit of %d bytes and stripe width of 0, ignoring.\n"), > - progname, BBTOB(ft->dsunit)); > +_("%s: Volume reports invalid stripe unit (%d) and stripe width (%d), ignoring.\n"), > + progname, BBTOB(ft->dsunit), BBTOB(ft->dswidth)); > ft->dsunit = 0; > ft->dswidth = 0; > } else { > -- > 2.18.1 >