| Message ID | 20221205124144.23770-1-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ext4/044: Fix failure when mount options are incompatible with ext3 | expand |
Hi Jan, On Mon, 5 Dec 2022 13:41:44 +0100, Jan Kara wrote: > There are some mount options that are incompatible with ext3 filesystem > type. If they are used, this test fails because it tries to remount the > filesystem as ext3. The test makes sense even without remounting as ext3 > so just make the test silently skip the remount. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > tests/ext4/044 | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/tests/ext4/044 b/tests/ext4/044 > index 50de5a40bdc7..96fa70cc0d1e 100755 > --- a/tests/ext4/044 > +++ b/tests/ext4/044 > @@ -53,9 +53,10 @@ _within_tolerance "sec_ctime" $sec_ctime $sec 1 -v > > _scratch_unmount >> $seqres.full 2>&1 > > -# Test mount to ext3 then mount back to ext4 and check timestamp again > -_mount -t ext3 `_scratch_mount_options $*` || _fail "ext3 mount failed" > -_scratch_unmount >> $seqres.full 2>&1 > +# Test mount to ext3 then mount back to ext4 and check timestamp again. We > +# ignore if ext3 failed to mount. It can happen because some mount options are > +# incompatible with ext3. Still the test makes sense. > +_mount -t ext3 `_scratch_mount_options $*` >> $seqres.full 2>&1 && _scratch_unmount >> $seqres.full 2>&1 > _scratch_mount I suppose this makes sense compared to filtering or dropping all mount options. $* should always be empty here, right? Looks fine otherwise... Reviewed-by: David Disseldorp <ddiss@suse.de>
Hi David! On Tue 06-12-22 00:35:42, David Disseldorp wrote: > On Mon, 5 Dec 2022 13:41:44 +0100, Jan Kara wrote: > > There are some mount options that are incompatible with ext3 filesystem > > type. If they are used, this test fails because it tries to remount the > > filesystem as ext3. The test makes sense even without remounting as ext3 > > so just make the test silently skip the remount. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > --- > > tests/ext4/044 | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/tests/ext4/044 b/tests/ext4/044 > > index 50de5a40bdc7..96fa70cc0d1e 100755 > > --- a/tests/ext4/044 > > +++ b/tests/ext4/044 > > @@ -53,9 +53,10 @@ _within_tolerance "sec_ctime" $sec_ctime $sec 1 -v > > > > _scratch_unmount >> $seqres.full 2>&1 > > > > -# Test mount to ext3 then mount back to ext4 and check timestamp again > > -_mount -t ext3 `_scratch_mount_options $*` || _fail "ext3 mount failed" > > -_scratch_unmount >> $seqres.full 2>&1 > > +# Test mount to ext3 then mount back to ext4 and check timestamp again. We > > +# ignore if ext3 failed to mount. It can happen because some mount options are > > +# incompatible with ext3. Still the test makes sense. > > +_mount -t ext3 `_scratch_mount_options $*` >> $seqres.full 2>&1 && _scratch_unmount >> $seqres.full 2>&1 > > _scratch_mount > > I suppose this makes sense compared to filtering or dropping all mount > options. Yeah, filtering would need constant updating and dropping all mount options could have some unexpected sideeffects. > $* should always be empty here, right? Yeah, likely yes. I've just copied that without thinking too much about it :). > Looks fine otherwise... > Reviewed-by: David Disseldorp <ddiss@suse.de> Thanks for review! Honza
diff --git a/tests/ext4/044 b/tests/ext4/044 index 50de5a40bdc7..96fa70cc0d1e 100755 --- a/tests/ext4/044 +++ b/tests/ext4/044 @@ -53,9 +53,10 @@ _within_tolerance "sec_ctime" $sec_ctime $sec 1 -v _scratch_unmount >> $seqres.full 2>&1 -# Test mount to ext3 then mount back to ext4 and check timestamp again -_mount -t ext3 `_scratch_mount_options $*` || _fail "ext3 mount failed" -_scratch_unmount >> $seqres.full 2>&1 +# Test mount to ext3 then mount back to ext4 and check timestamp again. We +# ignore if ext3 failed to mount. It can happen because some mount options are +# incompatible with ext3. Still the test makes sense. +_mount -t ext3 `_scratch_mount_options $*` >> $seqres.full 2>&1 && _scratch_unmount >> $seqres.full 2>&1 _scratch_mount nsec_atime2=`$here/src/t_get_file_time $SCRATCH_MNT/tmp_file atime nsec`
There are some mount options that are incompatible with ext3 filesystem type. If they are used, this test fails because it tries to remount the filesystem as ext3. The test makes sense even without remounting as ext3 so just make the test silently skip the remount. Signed-off-by: Jan Kara <jack@suse.cz> --- tests/ext4/044 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)