| Message ID | 20240205175552.GJ6188@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | common/metadump: fix _cleanup_verify_metadump system destructor fleet | expand |
On Mon, Feb 05, 2024 at 09:55:52AM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Zorro reported that the cleanup function in common/xfs_metadump_tests > zapped one of his test machines because of a well known shell variable > expansion + globbing footgun. This can trigger when running fstests on > older configurations where a test adds _cleanup_verify_metadump to the > local _cleanup function but exits before calling _setup_verify_metadump > to set XFS_METADUMP_IMG to a non-empty value. > > Redesign the cleanup function to check for non-empty values of > XFS_METADUMP_{FILE,IMG} before proceeding with the rm. Change the > globbed parameter of "rm -f $XFS_METADUMP_IMG*" to a for loop so that if > the glob does not match any files, the loop variable will be set to a > path that does not resolve anywhere. > > Longer term maybe we ought to set -u or something. Or figure out how to > make the root filesystem readonly when we're running a test. > > Reported-by: Zorro Lang <zlang@redhat.com> > Link: https://lore.kernel.org/fstests/20240205060016.7fgiyafbnrvf5chj@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/ > Fixes: 85c1e0f518 ("common: refactor metadump v1 and v2 tests") > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- Hi Darrick, This change looks good to me, but I've reset the for-next branch, remove the v2024.02.04 totally, to avoid it kills others' systems. Now there's not v2024.02.04 release anymore, so you can resend a V2 of: [PATCHSET] fstests: random fixes for v2024.01.14 or resend the: [PATCH 05/10] common: refactor metadump v1 and v2 tests if you don't have more changes on others. > common/xfs_metadump_tests | 19 +++++++++++++++---- I saw you name this patch as common/metadump, if already have a patchset which changes the common/xfs_metadump_tests to common/metadump, feel free to send to me directly as V2 :) Thanks, Zorro > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/common/xfs_metadump_tests b/common/xfs_metadump_tests > index 5a5579b6b9..c8652f16ea 100644 > --- a/common/xfs_metadump_tests > +++ b/common/xfs_metadump_tests > @@ -15,12 +15,23 @@ _setup_verify_metadump() > > _cleanup_verify_metadump() > { > + local img > + > _scratch_unmount &>> $seqres.full > > - losetup -n -a -O BACK-FILE,NAME | grep "^$XFS_METADUMP_IMG" | while read backing ldev; do > - losetup -d "$ldev" > - done > - rm -f "$XFS_METADUMP_FILE" "$XFS_METADUMP_IMG"* > + test -n "$XFS_METADUMP_FILE" && rm -f "$XFS_METADUMP_FILE" > + > + if [ -n "$XFS_METADUMP_IMG" ]; then > + losetup -n -a -O BACK-FILE,NAME | grep "^$XFS_METADUMP_IMG" | while read backing ldev; do > + losetup -d "$ldev" > + done > + > + # Don't call rm directly with a globbed argument here to avoid > + # issues issues with variable expansions. > + for img in "$XFS_METADUMP_IMG"*; do > + test -e "$img" && rm -f "$img" > + done > + fi > } > > # Can xfs_metadump snapshot the fs metadata to a v1 metadump file? >
diff --git a/common/xfs_metadump_tests b/common/xfs_metadump_tests index 5a5579b6b9..c8652f16ea 100644 --- a/common/xfs_metadump_tests +++ b/common/xfs_metadump_tests @@ -15,12 +15,23 @@ _setup_verify_metadump() _cleanup_verify_metadump() { + local img + _scratch_unmount &>> $seqres.full - losetup -n -a -O BACK-FILE,NAME | grep "^$XFS_METADUMP_IMG" | while read backing ldev; do - losetup -d "$ldev" - done - rm -f "$XFS_METADUMP_FILE" "$XFS_METADUMP_IMG"* + test -n "$XFS_METADUMP_FILE" && rm -f "$XFS_METADUMP_FILE" + + if [ -n "$XFS_METADUMP_IMG" ]; then + losetup -n -a -O BACK-FILE,NAME | grep "^$XFS_METADUMP_IMG" | while read backing ldev; do + losetup -d "$ldev" + done + + # Don't call rm directly with a globbed argument here to avoid + # issues issues with variable expansions. + for img in "$XFS_METADUMP_IMG"*; do + test -e "$img" && rm -f "$img" + done + fi } # Can xfs_metadump snapshot the fs metadata to a v1 metadump file?