| Message ID | 26571df609d06e0a484a800d424be3e22c0f9961.1664936628.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: selftests fixes | expand |
On Wed, Oct 05, 2022 at 10:25:12AM +0800, Qu Wenruo wrote: > [BUG] > After upgrading to kernel v6.0-rc, btrfs-progs selftest mkfs/001 no > longer checks single device RAID0 and other new features introduced in > v5.13: > > # make TEST=001\* test-mkfs > [TEST] mkfs-tests.sh > [TEST/mkfs] 001-basic-profiles > $ grep -IR "RAID0\/1" tests/mkfs-tests-results.txt > ^^^ No output > > [CAUSE] > The existing check_min_kernel_version() is doing an incorrect check. > > The old check looks like this: > > [ "$unamemajor" -lt "$argmajor" ] || return 1 > [ "$unameminor" -lt "$argminor" ] || return 1 > return 0 > > For 6.0-rc kernels, we have the following values for mkfs/001 > > $unamemajor = 6 > $unameminor = 0 > $argmajor = 5 > $argminor = 12 > > The first check doesn't exit immediately, as 6 > 5. > Then we check the minor, which is already incorrect. > > If our major is larger than target major, we should exit immediate with > 0. > > [FIX] > Fix the check and add extra comment. > > Personally speaking I'm not a fan or short compare and return, thus all > the checks will explicit "if []; then fi" checks. Agreed the explicit if should be used in most cases, what is probably ok a 'command || _fail' pattern for simple commands. I try to unify the shell coding style in new patches but some bits may slip through.
diff --git a/tests/common b/tests/common index 3ee42a80dcda..d985ef72a4f1 100644 --- a/tests/common +++ b/tests/common @@ -659,6 +659,9 @@ check_kernel_support() # compare running kernel version to the given parameter, return success # if running is newer than requested (let caller decide if to fail or skip) # $1: minimum version of running kernel in major.minor format (eg. 4.19) +# +# Return 0 if we meet the minimal version requirement. +# Return 1 if not. check_min_kernel_version() { local unamemajor @@ -672,10 +675,22 @@ check_min_kernel_version() uname=${uname%%-*} IFS=. read unamemajor unameminor tmp <<< "$uname" IFS=. read argmajor argminor tmp <<< "$1" - # "compare versions: ${unamemajor}.${unameminor} ? ${argmajor}.${argminor}" - [ "$unamemajor" -lt "$argmajor" ] || return 1 - [ "$unameminor" -lt "$argminor" ] || return 1 - return 0 + # If our major > target major, we definitely meet the requirement. + # If our major < target major, we definitely don't meet the requirement. + if [ "$unamemajor" -gt "$argmajor" ]; then + return 0 + fi + if [ "$unamemajor" -lt "$argmajor" ]; then + return 1 + fi + + # Only when our major is the same as the target, we need to compare + # the minor number. + # In this case, if our minor >= target minor, we meet the requirement. + if [ "$unameminor" -ge "$argminor" ]; then + return 0; + fi + return 1 } # Get subvolume id for specified path
[BUG] After upgrading to kernel v6.0-rc, btrfs-progs selftest mkfs/001 no longer checks single device RAID0 and other new features introduced in v5.13: # make TEST=001\* test-mkfs [TEST] mkfs-tests.sh [TEST/mkfs] 001-basic-profiles $ grep -IR "RAID0\/1" tests/mkfs-tests-results.txt ^^^ No output [CAUSE] The existing check_min_kernel_version() is doing an incorrect check. The old check looks like this: [ "$unamemajor" -lt "$argmajor" ] || return 1 [ "$unameminor" -lt "$argminor" ] || return 1 return 0 For 6.0-rc kernels, we have the following values for mkfs/001 $unamemajor = 6 $unameminor = 0 $argmajor = 5 $argminor = 12 The first check doesn't exit immediately, as 6 > 5. Then we check the minor, which is already incorrect. If our major is larger than target major, we should exit immediate with 0. [FIX] Fix the check and add extra comment. Personally speaking I'm not a fan or short compare and return, thus all the checks will explicit "if []; then fi" checks. Now mkfs/001 works as expected: # make TEST=001\* test-mkfs [TEST] mkfs-tests.sh [TEST/mkfs] 001-basic-profiles $ grep -IR "RAID0\/1" tests/mkfs-tests-results.txt Data,RAID0/1: 204.75MiB Metadata,RAID0/1: 204.75MiB System,RAID0/1: 8.00MiB Signed-off-by: Qu Wenruo <wqu@suse.com> --- tests/common | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)