| Message ID | 20240630-common-fixes-v2-2-16d26fb1dee0@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | common fixes | expand |
On Sun, Jun 30, 2024 at 11:52:41PM +0200, Daniel Gomez via B4 Relay wrote: > From: Daniel Gomez <da.gomez@samsung.com> > > Add support for test device recreation (RECREATE_TEST_DEV=true) for > tmpfs. > > Signed-off-by: Daniel Gomez <da.gomez@samsung.com> > --- > common/rc | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/common/rc b/common/rc > index 163041fea..51827119c 100644 > --- a/common/rc > +++ b/common/rc > @@ -604,6 +604,9 @@ _test_mkfs() > pvfs2) > # do nothing for pvfs2 > ;; > + tmpfs) Indentation problem here. > + # do nothing for tmpfs If we're recreating the test filesystem, shouldn't that unmount and remount for tmpfs? Or at least rm -rf everything underneath it? That's generally the effect of _test_mkfs for disk filesystems. (That said, I'm much less familiar with non-disk filesystems...) --D > + ;; > udf) > $MKFS_UDF_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null > ;; > > -- > 2.43.0 > > >
On Tue, Jul 2, 2024 at 12:36 AM Darrick J. Wong <djwong@kernel.org> wrote: > > On Sun, Jun 30, 2024 at 11:52:41PM +0200, Daniel Gomez via B4 Relay wrote: > > From: Daniel Gomez <da.gomez@samsung.com> > > > > Add support for test device recreation (RECREATE_TEST_DEV=true) for > > tmpfs. > > > > Signed-off-by: Daniel Gomez <da.gomez@samsung.com> > > --- > > common/rc | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/common/rc b/common/rc > > index 163041fea..51827119c 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -604,6 +604,9 @@ _test_mkfs() > > pvfs2) > > # do nothing for pvfs2 > > ;; > > + tmpfs) > > Indentation problem here. What's actually the format for indentation in bash scripts across xfstests-dev project? I see a mix of tabs and spaces everywhere. For this particular _test_mkfs(), I see: <spaces>overlay) <tabs># do nothing for overlay <tabs>;; <spaces>pvfs2) <tabs># do nothing for pvfs2 <tabs>;; <spaces>udf) <spaces><spaces>$MKFS_UDF_PROG ... <tabs>;; <spaces>btrfs) <spaces><spaces>$MKFS_BTRFS_PROG... > > > + # do nothing for tmpfs > > If we're recreating the test filesystem, shouldn't that unmount and > remount for tmpfs? Or at least rm -rf everything underneath it? That's > generally the effect of _test_mkfs for disk filesystems. I thought this was already supported. When I enable recreation, I get the following error: ./check -s tmpfs_huge_always -R xunit generic/003 SECTION -- tmpfs_huge_always RECREATING -- tmpfs on /media/test our local _test_mkfs routine ... mkfs: failed to execute mkfs.tmpfs: No such file or directory check: failed to mkfs $TEST_DEV using specified options Interrupted! Interrupted! Passed all 0 tests Xunit report: /result.xml SECTION -- tmpfs_huge_always ========================= Passed all 0 tests So, adding the tmpfs) case to _test_mkfs() just lets recreation go through, and mount and umount when the test is finished. I'm not sure if I'm missing something or maybe I should rename this patch as a fix? > > (That said, I'm much less familiar with non-disk filesystems...) When umounting a tmpfs mount dir, data is lost. So in the recreation case, _test_unmount() would take care of that. > > --D > > > + ;; > > udf) > > $MKFS_UDF_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null > > ;; > > > > -- > > 2.43.0 > > > > > >
On Fri, Jul 05, 2024 at 11:46:53PM +0200, Daniel Gomez (Samsung) wrote: > On Tue, Jul 2, 2024 at 12:36 AM Darrick J. Wong <djwong@kernel.org> wrote: > > > > On Sun, Jun 30, 2024 at 11:52:41PM +0200, Daniel Gomez via B4 Relay wrote: > > > From: Daniel Gomez <da.gomez@samsung.com> > > > > > > Add support for test device recreation (RECREATE_TEST_DEV=true) for > > > tmpfs. > > > > > > Signed-off-by: Daniel Gomez <da.gomez@samsung.com> > > > --- > > > common/rc | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/common/rc b/common/rc > > > index 163041fea..51827119c 100644 > > > --- a/common/rc > > > +++ b/common/rc > > > @@ -604,6 +604,9 @@ _test_mkfs() > > > pvfs2) > > > # do nothing for pvfs2 > > > ;; > > > + tmpfs) > > > > Indentation problem here. > > What's actually the format for indentation in bash scripts across > xfstests-dev project? I see a mix of tabs and spaces everywhere. For > this particular _test_mkfs(), I see: > <spaces>overlay) > <tabs># do nothing for overlay > <tabs>;; > <spaces>pvfs2) > <tabs># do nothing for pvfs2 > <tabs>;; > <spaces>udf) > <spaces><spaces>$MKFS_UDF_PROG ... > <tabs>;; > <spaces>btrfs) > <spaces><spaces>$MKFS_BTRFS_PROG... I don't think there's a set convention anywhere, I usually just copy the thing above it. I was reacting to the diff; if you actually just copied the pvfs2 clause and then s/pvfs2/tmpfs/ then I withdraw the comment. > > > + # do nothing for tmpfs > > > > If we're recreating the test filesystem, shouldn't that unmount and > > remount for tmpfs? Or at least rm -rf everything underneath it? That's > > generally the effect of _test_mkfs for disk filesystems. > > I thought this was already supported. When I enable recreation, I get > the following error: > > ./check -s tmpfs_huge_always -R xunit generic/003 > SECTION -- tmpfs_huge_always > RECREATING -- tmpfs on /media/test > our local _test_mkfs routine ... > mkfs: failed to execute mkfs.tmpfs: No such file or directory > check: failed to mkfs $TEST_DEV using specified options > Interrupted! > Interrupted! > Passed all 0 tests > Xunit report: /result.xml > SECTION -- tmpfs_huge_always > ========================= > Passed all 0 tests > > So, adding the tmpfs) case to _test_mkfs() just lets recreation go > through, and mount and umount when the test is finished. I'm not sure > if I'm missing something or maybe I should rename this patch as a fix? Ah! Yes, you're right, I forgot that there is no mkfs.tmpfs, everything is done by mount -t tmpfs. Can you change the comment to explain why _test_mkfs does nothing for tmpfs? tmpfs) # mount initializes the fs, no need to format anything ;; for dunces like me? :) > > > > (That said, I'm much less familiar with non-disk filesystems...) > > When umounting a tmpfs mount dir, data is lost. So in the recreation > case, _test_unmount() would take care of that. <nod> --D > > > > --D > > > > > + ;; > > > udf) > > > $MKFS_UDF_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null > > > ;; > > > > > > -- > > > 2.43.0 > > > > > > > > > >
diff --git a/common/rc b/common/rc index 163041fea..51827119c 100644 --- a/common/rc +++ b/common/rc @@ -604,6 +604,9 @@ _test_mkfs() pvfs2) # do nothing for pvfs2 ;; + tmpfs) + # do nothing for tmpfs + ;; udf) $MKFS_UDF_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null ;;