Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZTS: handle FreeBSD version numbers correctly #16340

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

robn
Copy link
Member

@robn robn commented Jul 11, 2024

Motivation and Context

@mcmilk wondered why the "FreeBSD 14+" version tests in the block cloning tests were failing. This is at least part of why!

Description

FreeBSD kernel version numbers (ie the output from uname -r) don't match the Linux format; the patchlevel is expressed differently. This sets up OS-specific checks.

How Has This Been Tested?

By hand on CentOS 7, Fedora 37 and Debian 12.6, and FreeBSD 13.2-RELEASE-p4, 14.0-RELEASE and 15.0-CURRENT.

Note "by hand": I ran ksh, pasted in the kernel_version function, set UNAME, then called kernel_version and eyeballed output. I don't have ZTS on most of these systems so it hasn't been used for real.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

FreeBSD patchlevel versions are optional and, if present, in a different
location in the version string.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <[email protected]>
robn referenced this pull request in robn/zfs Jul 11, 2024
Linux kernel versions have a minor number:

    $ uname -r
    6.1.0-21-amd64

FreeBSD does not (at least, not visibly in the output from uname -r):

    $ uname -r
    14.1-RELEASE

So, make the minor optional. We'll treat it as zero.

This should be sufficient as long as the FreeBSD minor verson
("patchlevel") is not needed for tests.

Sponsored-by: https://despairlabs.com/sponsor/
Signed-off-by: Rob Norris <[email protected]>
Copy link
Contributor

@mcmilk mcmilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It LGTM - but I didn't test it currently.
Can I check it tommorow, before merging?

@mcmilk
Copy link
Contributor

mcmilk commented Jul 12, 2024

It LGTM - but I didn't test it currently. Can I check it tommorow, before merging?

Yes, it works as expected on FreeBSD 14 + 15 👍

@tonyhutter tonyhutter merged commit cbd95a9 into openzfs:master Jul 12, 2024
24 of 25 checks passed
robn added a commit to robn/zfs that referenced this pull request Jul 17, 2024
FreeBSD patchlevel versions are optional and, if present, in a different
location in the version string.

Sponsored-by: https://despairlabs.com/sponsor/

Signed-off-by: Rob Norris <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
calccrypto pushed a commit to hpc/zfs that referenced this pull request Jul 17, 2024
FreeBSD patchlevel versions are optional and, if present, in a different
location in the version string.

Sponsored-by: https://despairlabs.com/sponsor/

Signed-off-by: Rob Norris <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
FreeBSD patchlevel versions are optional and, if present, in a different
location in the version string.

Sponsored-by: https://despairlabs.com/sponsor/

Signed-off-by: Rob Norris <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants