On Mon, May 16, 2022 at 12:09:51PM +0530, Ojaswin Mujoo wrote: > A recent ext4 patch discussed [1] that some devices (eg LVMs) can > have a discard granularity as big as 42MB which makes it larger > than the group size of ext4 FS with 1k BS. This causes the FITRIM > IOCTL to fail. > > This case was not correctly handled by this test since > "_require_batched_discard" incorrectly interpreted the FITRIM > failure as SCRATCH_DEV not supporting the IOCTL. This caused the test > to report "not run" instead of "failed" in case of large discard granularity. > > Fix "_require_batched_discard" to use a more accurate method > to determine if discard is supported. > > [1] commit 173b6e383d2 > ext4: avoid trim error on fs with small groups > > Signed-off-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx> > Reviewed-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx> > --- > > Changes since v1 [1] > > * Changed $RET to a local variable > * Fixed the grep command > > [1] > https://lore.kernel.org/all/20220401055713.634842-1-ojaswin@xxxxxxxxxxxxx/ > > common/rc | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index e2d3d72a..f366e409 100644 > --- a/common/rc > +++ b/common/rc > @@ -3858,7 +3858,13 @@ _require_batched_discard() > exit 1 > fi > _require_fstrim > - $FSTRIM_PROG $1 > /dev/null 2>&1 || _notrun "FITRIM not supported on $1" > + > + grep -q "not supported" <($FSTRIM_PROG $1 2>&1) > + local ret=$? > + if [ "$ret" = "0" ] Oh I forgot to ask why we need to add a variable (ret), to record the return value at here. Why can't use "$?" directly? e.g. grep -q "not supported" <($FSTRIM_PROG $1 2>&1) if [ $? -eq 0 ] Others look good to me. Thanks, Zorro > + then > + _notrun "FITRIM not supported on $1" > + fi > } > > _require_dumpe2fs() > -- > 2.27.0 >
