Re: [PATCH] fstests: filter.btrfs: handle detailed missing device report better

[Filipe Manana <fdmanana@kernel.org>]

On Wed, Nov 09, 2022 at 02:22:36PM +0800, Qu Wenruo wrote:
> [FAILURES]
> The following btrfs test cases failed with newer btrfs-progs:
> 
> - btrfs/197
> - btrfs/198
> - btrfs/254
> 
> They all fail due to output mismatch like the following:
> 
>      Label: none  uuid: <UUID>
>      	Total devices <NUM> FS bytes used <SIZE>
>      	devid <DEVID> size <SIZE> used <SIZE> path SCRATCH_DEV
>     -	*** Some devices missing
>     +	devid <DEVID> size 0 used 0 path  MISSING
> 
> [CAUSE]
> Since btrfs-progs commit 957a79c9b016 ("btrfs-progs: fi show: print
> missing device for a mounted file system"), we output the detailed info
> of a missing device if "btrfs filesystem show" is executed using
> "-m <mnt>" option.
> 
> Such detailed output no longer follows the old format, thus causing the
> output mismatch.
> 
> [FIX]
> Update _filter_btrfs_filesystem_show() to handle detailed missing
> device by:
> 
> - Buffer the output first
> 
> - Output the first two lines
>   Which is always label/uuid and the total device accounting.
> 
> - Replace the detailed missing device line with old output
> 
> - Sort (in reverse order) and uniq the device list
> 
> By this we can handle both old and new output correctly.
> Although this means we lacks the ability to detect mutltiple missing
> devices, thankfully the involved test cases are not checking this yet.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  common/filter.btrfs | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/common/filter.btrfs b/common/filter.btrfs
> index d4169cc6..c570d366 100644
> --- a/common/filter.btrfs
> +++ b/common/filter.btrfs
> @@ -35,7 +35,17 @@ _filter_btrfs_filesystem_show()
>  	_filter_size | _filter_btrfs_version | _filter_devid | \
>  	_filter_zero_size | \
>  	sed -e "s/\(Total devices\) $NUMDEVS/\1 $NUM_SUBST/g" | \
> -	uniq
> +	uniq > $tmp.btrfs_filesystem_show
> +
> +	# The first two lines are Label/UUID and total devices
> +	head -n 2 $tmp.btrfs_filesystem_show
> +
> +	# The remaining is the device list, first filter out the detailed
> +	# missing device to the generic one.
> +	# Then sort and uniq the result
> +	tail -n +3 $tmp.btrfs_filesystem_show | \
> +	sed -e "s/devid <DEVID> size 0 used 0 path  MISSING/*** Some devices missing/" | \
> +	sort -r | uniq

Ah, I had updated to btrfs-progs 6.0 last week, and was wondering how
was it possible no one had fixed this before - either no one is testing
with 5.19+ or no one cares.

It looks good to me, and it works for me.

I had a local patch here as well, but you beat me to it.
My version is based on awk only, no sorting needed:

diff --git a/common/filter.btrfs b/common/filter.btrfs
index d4169cc6..a27a7276 100644
--- a/common/filter.btrfs
+++ b/common/filter.btrfs
@@ -30,11 +30,22 @@ _filter_btrfs_filesystem_show()
                UUID=$2
        fi
 
-       # the uniq collapses all device lines into 1
+       # The awk script converts between the output format of btrfs-progs 5.19+
+       # to the output format of older versions when there are missing devices.
+       # That format changed in the btrfs-progs commit:
+       #
+       #   957a79c9b016 ("btrfs-progs: fi show: print missing device for a mounted file system")
+       #
+       # The uniq at the end collapses all device lines into 1.
        _filter_uuid $UUID | _filter_scratch | _filter_scratch_pool | \
        _filter_size | _filter_btrfs_version | _filter_devid | \
        _filter_zero_size | \
        sed -e "s/\(Total devices\) $NUMDEVS/\1 $NUM_SUBST/g" | \
+       $AWK_PROG -e 'BEGIN { missing = 0 }' \
+                 -e '/\sMISSING$/ { missing = 1 }' \
+                 -e '/^\s*$/ { next }' \
+                 -e '!/\sMISSING$/ { print }' \
+                 -e 'END { if (missing) print "\t*** Some devices missing\n" }' | \
        uniq
 }

Either way, it looks good to me, thanks!

Reviewed-by: Filipe Manana <fdmanana@suse.com>


>  }
>  
>  # This eliminates all numbers, and shows only unique lines,
> -- 
> 2.38.0
>