zfs-2.2.7 patchset #16720
Merged
zfs-2.2.7 patchset #16720
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Use the macros in few places it was missed. - Reduce scope of DB_DNODE_ENTER/EXIT() and inline some DB_DNODE() uses to make it more obvious what exactly is protected there and make unprotected accesses by mistake more difficult. - Make use of zrl_owner(). Reviewed-by: Rob Wing <[email protected] Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Allan Jude <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16374
ZIL log record structs (lr_XX_t) are frequently allocated with extra
space after the struct to carry variable-sized "payload" items.
Linux 6.10+ compiled with CONFIG_FORTIFY_SOURCE has been doing runtime
bounds checking on memcpy() calls. Because these types had no indicator
that they might use more space than their simple definition,
__fortify_memcpy_chk will frequently complain about overruns eg:
memcpy: detected field-spanning write (size 7) of single field
"lr + 1" at zfs_log.c:425 (size 0)
memcpy: detected field-spanning write (size 9) of single field
"(char *)(lr + 1)" at zfs_log.c:593 (size 0)
memcpy: detected field-spanning write (size 4) of single field
"(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0)
memcpy: detected field-spanning write (size 7) of single field
"lr + 1" at zfs_log.c:425 (size 0)
memcpy: detected field-spanning write (size 9) of single field
"(char *)(lr + 1)" at zfs_log.c:593 (size 0)
memcpy: detected field-spanning write (size 4) of single field
"(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0)
memcpy: detected field-spanning write (size 7) of single field
"lr + 1" at zfs_log.c:425 (size 0)
memcpy: detected field-spanning write (size 9) of single field
"(char *)(lr + 1)" at zfs_log.c:593 (size 0)
memcpy: detected field-spanning write (size 4) of single field
"(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0)
To fix this, this commit adds flex array fields to all lr_XX_t structs
that require them, and then uses those fields to access that
end-of-struct area rather than more complicated casts and pointer
addition.
Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16501
Closes openzfs#16539
This sets RHEL8's base kernel[1] as the floor, and includes the oldest kernel.org LTS (4.19). 1. https://access.redhat.com/articles/3078#RHEL8 Sponsored-by: https://despairlabs.com/sponsor/ Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16479
Update the META file to reflect compatibility with the 6.11 kernel. Reviewed-by: Rob Norris <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16586
Last commit should fix the underlying problem, so these should be passing reliably again. Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Rob Norris <[email protected]> Closes openzfs#16364
Optionally turn off disk's enclosure slot if an I/O is hung triggering the deadman. It's possible for outstanding I/O to a misbehaving SCSI disk to neither promptly complete or return an error. This can occur due to retry and recovery actions taken by the SCSI layer, driver, or disk. When it occurs the pool will be unresponsive even though there may be sufficient redundancy configured to proceeded without this single disk. When a hung I/O is detected by the kmods it will be posted as a deadman event. By default an I/O is considered to be hung after 5 minutes. This value can be changed with the zfs_deadman_ziotime_ms module parameter. If ZED_POWER_OFF_ENCLOSURE_SLOT_ON_DEADMAN is set the disk's enclosure slot will be powered off causing the outstanding I/O to fail. The ZED will then handle this like a normal disk failure. By default ZED_POWER_OFF_ENCLOSURE_SLOT_ON_DEADMAN is not set. As part of this change `zfs_deadman_events_per_second` is added to control the ratelimitting of deadman events independantly of delay events. In practice, a single deadman event is sufficient and more aren't particularly useful. Alphabetize the zfs_deadman_* entries in zfs.4. Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16226
Some libc's like uClibc lag the proper definition of SEEK_DATA and SEEK_HOLE. Since we have only two files in ZTS which use these definitons, let's define them by hand: ``` #ifndef SEEK_DATA #define SEEK_DATA 3 #endif #ifndef SEEK_HOLE #define SEEK_HOLE 4 #endif ``` There should be no failures, because: - FreeBSD has support for SEEK_DATA/SEEK_HOLE since FreeBSD 8 - Linux has it since Linux 3.1 - the libc will submit the parameters unchanged to the kernel Signed-off-by: Tino Reichardt <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Allan Jude <[email protected]> Signed-off-by: Mateusz Piotrowski <[email protected]> Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Closes openzfs#16248
Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Allan Jude <[email protected]> Signed-off-by: Mateusz Piotrowski <[email protected]> Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Closes openzfs#16248
Reviewed-by: George Melikov <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Ameer Hamza <[email protected]> Closes openzfs#16432
The report generator expects the log to be clean and tidy UTF-8. That can be a problem if you use some of the verbose/debug test runner options, which sends all sorts of weird output from arbitrary programs to the log. This just makes Python a little more relaxed about such things. It shouldn't matter in practice, as those lines didn't match the test result regex anyway, and are discarded immediately. Sponsored-by: https://despairlabs.com/sponsor/ Signed-off-by: Rob Norris <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Tony Hutter <[email protected]>
The test needs some adjusting within the timings. Reviewed by: Brian Behlendorf <[email protected]> Signed-off-by: Tony Hutter <[email protected]> Co-authored-by: Tino Reichardt <[email protected]> Closes openzfs#16537
On load the test needs sometimes a bit more time then just one second. Doubling the time will help on the QEMU based testings. Reviewed by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16537
This commit adds functional tests for these systems:
- AlmaLinux 8, AlmaLinux 9, ArchLinux
- CentOS Stream 9, Fedora 39, Fedora 40
- Debian 11, Debian 12
- FreeBSD 13, FreeBSD 14, FreeBSD 15
- Ubuntu 20.04, Ubuntu 22.04, Ubuntu 24.04
- enabled by default:
- AlmaLinux 8, AlmaLinux 9
- Debian 11, Debian 12
- Fedora 39, Fedora 40
- FreeBSD 13, FreeBSD 14
Workflow for each operating system:
- install qemu on the github runner
- download current cloud image of operating system
- start and init that image via cloud-init
- install dependencies and poweroff system
- start system and build openzfs and then poweroff again
- clone build system and start 2 instances of it
- run functional testings and complete in around 3h
- when tests are done, do some logfile preparing
- show detailed results for each system
- in the end, generate the job summary
Real-world benefits from this PR:
1. The github runner scripts are in the zfs repo itself. That means
you can just open a PR against zfs, like "Add Fedora 41 tester", and
see the results directly in the PR. ZFS admins no longer need
manually to login to the buildbot server to update the buildbot config
with new version of Fedora/Almalinux.
2. Github runners allow you to run the entire test suite against your
private branch before submitting a formal PR to openzfs. Just open a
PR against your private zfs repo, and the exact same
Fedora/Alma/FreeBSD runners will fire up and run ZTS. This can be
useful if you want to iterate on a ZTS change before submitting a
formal PR.
3. buildbot is incredibly cumbersome. Our buildbot config files alone
are ~1500 lines (not including any build/setup scripts)!
It's a huge pain to setup.
4. We're running the super ancient buildbot 0.8.12. It's so ancient
it requires python2. We actually have to build python2 from source
for almalinux9 just to get it to run. Ugrading to a more modern
buildbot is a huge undertaking, and the UI on the newer versions is
worse.
5. Buildbot uses EC2 instances. EC2 is a pain because:
* It costs money
* They throttle IOPS and CPU usage, leading to mysterious,
* hard-to-diagnose, failures and timeouts in ZTS.
* EC2 is high maintenance. We have to setup security groups, SSH
* keys, networking, users, etc, in AWS and it's a pain. We also
* have to periodically go in an kill zombie EC2 instances that
* buildbot is unable to kill off.
6. Buildbot doesn't always handle failures well. One of the things we
saw in the past was the FreeBSD builders would often die, and each
builder death would take up a "slot" in buildbot. So we would
periodically have to restart buildbot via a cron job to get the slots
back.
7. This PR divides up the ZTS test list into two parts, launches two
VMs, and on each VM runs half the test suite. The test results are
then merged and shown in the sumary page. So we're basically
parallelizing ZTS on the same github runner. This leads to lower
overall ZTS runtimes (2.5-3 hours vs 4+ hours on buildbot), and one
unified set of results per runner, which is nice.
8. Since the tests are running on a VM, we have much more control over
what happens. We can capture the serial console output even if the
test completely brings down the VM. In the future, we could also
restart the test on the VM where it left off, so that if a single test
panics the VM, we can just restart it and run the remaining ZTS tests
(this functionaly is not yet implemented though, just an idea).
9. Using the runners, users can manually kill or restart a test run
via the github IU. That really isn't possible with buildbot unless
you're an admin.
10. Anecdotally, the tests seem to be more stable and constant under
the QEMU runners.
Reviewed by: Brian Behlendorf <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Signed-off-by: Tony Hutter <[email protected]>
Closes openzfs#16537
On larger files this should improve the speed. Sample values of my system: [mcmilk@xz]$ time dd if=/dev/zero bs=128k count=1k | sha256sum 254bcc3fc4f27172636df4bf32de9f107f620d559b20d760197e452b97453917 - real 0m1,050s user 0m0,985s sys 0m0,153s [mcmilk@xz]$ time dd if=/dev/zero bs=128k count=1k | openssl sha256 -r 254bcc3fc4f27172636df4bf32de9f107f620d559b20d760197e452b97453917 *stdin real 0m0,254s user 0m0,206s sys 0m0,160s I think cli_root/zdb/zdb_backup.ksh runs also an FreeBSD and I needed to include the sysutils/coreutils package for the FreeBSD tests within the QEMU patchset. This could be reverted, when this pull request gets upstream Reviewed-by: George Melikov <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16543
Fix that error: "cat /tmp/failed.txt: No such file or directory" Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16549
This commit changes the workflow of the github actions. - Ubuntu 20.04, 22.04, 24.04 will be tested via QEMU now - remove unused scripts of this commit: b7bc334 - re-add the zloop standalone testings via zloop.yml Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16549
In zpool_create.shlib, check_feature_set iterates over all features mentioned in provided compatibility file to check if only those features are enabled on the pool. This commit fixes skipping over comment lines correctly. Otherwise, the test case fails as comment lines are also treated as feature names by check_feature_set function. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Umer Saleem <[email protected]> Closes openzfs#15909
The qemu-9-summary-page.sh script reads the file env.txt in the first lines. When the module didn't build, this file was not copied into the tarfile - causing the scipt to abort. Fix: copy needed files into the tarfile in case of module build failures. The fix ignores also empty tarfiles in future. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Tino Reichardt <[email protected]> Closes openzfs#16555
There is no longer be a need for the ci_reason exception with the update CI GitHub Actions infrastruture. Retire it. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16553
All supported Linux kernels, 4.18 and newer, provide O_TMPFILE. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16553
The following tests have been observed to occasionally fail when running under the CI. Updated our exceptions list to track them. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16553
Switch from v2 to v3 CodeQL Actions. The v2 actions will no longer
be supported as of Dec '24 so we need to move to v3. According to
the release notes they should be functionally equivalent.
Note that the only difference between v2 and v3 of the CodeQL
Action is the node version they support, ... For example 3.22.11
was the first v3 release and is functionally identical to 2.22.11.
https://github.com/github/codeql-action/blob/main/CHANGELOG.md
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16560
Update the CONTRIBUTING.md documentation to refer to the GitHub Actions workflows which have replaced the buildbot infrastructure. Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16561
For checkstyle, zloop, zfs-qemu, and codeql workflows cancel in-progress jobs when the PR is updated. Relevant GitHub Actions documentation: The following concurrency group cancels in-progress jobs or run on pull_request events only; if github.head_ref is undefined, the concurrency group will fallback to the run ID, which is guaranteed to be both unique and defined for the run. https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#example-using-a-fallback-value Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16562
The commit uses heuristics to determine whether a PR is behavioral: It runs "quick" CI (i.e., only use sanity.run on fewer OSes) if (explicitly requested by user): - the *last* commit message contains a line 'ZFS-CI-Type: quick', or if (by heuristics): - the files changed are not in the list of specified directory, and - all commit messages does not contain 'ZFS-CI-Type: full'. It runs "full" CI otherwise. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Shengqi Chen <[email protected]> Closes openzfs#16564
Update the test case to freeze the pool then export it to better simulate a hard failure. This is preferable to copying the vdev while the pool's imported since with a copy we're not guaranteed the on-disk state will be consistent. That can in turn result in a pool import failure and a spurious test failure. Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16570
On failure attempt to include the most relevant portions of the ztest logs in the CI output. This full logs are still available for download but often a backtrace and the last output is enough. Install libunwind to improve the odds of a useful backtrace. Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16573
Lower the minimum number of expected deadman events from 4 to 3. All that is strictly required is a single event to consider the test a pass. However, since I've never seen a count of less than 3 reported by the CI that should be sufficient. Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: George Melikov <[email protected]> Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16575
Update the test case to freeze the pool then export it to better simulate a hard failure. This is preferable to copying the vdev while the pool's imported since with a copy we're not guaranteed the on-disk state will be consistent. That can in turn result in a pool import failure and a spurious test failure. Reviewed-by: Tino Reichardt <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16578
|
@robn thanks, I included it in my latest push. |
|
Do you want to add 6.12 compatibility with PR #16793 ? |
|
@tonyhutter I think it would be great to have 38c0324 included, as a lot of 2.2 users experience the underlying issue. |
amotin
approved these changes
Nov 26, 2024
Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Umer Saleem <[email protected]> Reviewed-by: Ameer Hamza <[email protected]> Reviewed-by: Tony Hutter <[email protected]>
On 64bit FreeBSD this reduces one from 296 to 280 bytes. On small block workloads dbufs may consume gigabytes of ARC, and this saves 5% of it. Reviewed-by: Tino Reichardt <[email protected]> Reviewed-by: Brian Atkinson <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16684
If we write less than 113 bytes with enabled compression we get embeded block, which then fails check for number of cloned blocks in bclone_test. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Pawel Jakub Dawidek <[email protected]> Reviewed-by: Brian Atkinson <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16740
We are doing exactly the same checks around all brt_pending_add(). Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Pawel Jakub Dawidek <[email protected]> Reviewed-by: Brian Atkinson <[email protected]> Signed-off-by: Alexander Motin <[email protected]> Sponsored by: iXsystems, Inc. Closes openzfs#16740
Update the META file to reflect compatibility with the 6.12 kernel. Reviewed-by: Umer Saleem <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes openzfs#16793
by protecting against sb->s_shrink eviction on umount with newer kernels deactivate_locked_super calls shrinker_free and only then sops->kill_sb cb, resulting in UAF on umount when trying to reach for the shrinker functions in zpl_prune_sb of in-umount dataset Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Adam Moss <[email protected]> Signed-off-by: Pavel Snajdr <[email protected]> Closes openzfs#16770
tonyhutter
force-pushed
the
zfs-2.2.7-hutter
branch
from
0973ff7 to
b1f6139
Compare
|
For full 6.11+ kernel compatibility I guess we'd also need #16805 - once it's merged.... |
|
And also please #16817 (after it's reviewed and merged in some form) |
Adjust the m4 function to mimic sentinel we use in spl-proc.c This fixes the detection on kernels compiled with CONFIG_RANDSTRUCT=y Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Rob Norris <[email protected]> Reviewed-by: Pavel Snajdr <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Ivan Volosyuk <[email protected]> Closes: openzfs#16620 Closes: openzfs#16805
When replacing a disk, a child process is forked to run a script called zfs_prepare_disk (which can be useful for disk firmware update or health check). The parent than calls waitpid and checks the child error/status code. However, the _reap_children thread (created from zed_exec_process to manage zedlets) also waits for all children with the same PGID and can stole the signal, causing the replace operation to be aborted. As waitpid returns -1, the parent incorrectly assume that the child process had an error or was killed. This, in turn, leaves the newly added disk in REMOVED or UNAVAIL status rather than completing the replace process. This patch changes the PGID of the child process execuing the prepare script, shielding it from the _reap_children thread. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Tony Hutter <[email protected]> Signed-off-by: Gionatan Danti <[email protected]> Closes openzfs#16801
This fixes a serious performance problem with NFS handling of large directories, as the new get_name code is much more efficient than the default zfs_readdir. This is actually part of 20232ec in 2.3. But I've taken only the minimum code to implement get_name, and not the rest of the long name changes. Signed-off-by: Charles Hedrick <[email protected]> Co-authored-by: Charles L. Hedrick <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Reviewed by: Brian Behlendorf <[email protected]> Reviewed by: Brian Behlendorf <[email protected]> Reviewed-by: Tony Hutter <[email protected]>
META file and changelog updated. Signed-off-by: Tony Hutter <[email protected]>
tonyhutter
force-pushed
the
zfs-2.2.7-hutter
branch
from
b1f6139 to
e269af1
Compare
|
Any chance this patchset will also bring 6.12 compatibility? As 6.12 will be the next LTS release, Archlinux will probably switch to it in January, making updates somewhat problematic. |
|
@allddd I run 2.3.0-rc3 on 6.12.1 right now - https://github.com/n0xena/archzfs/releases |
|
@tonyhutter great work, thank you 🙇 (release nit: min FreeBSD is 13.0-RELEASE). |
|
Thank you! On the release page, "Supported platforms" contradicts "Changes" on the matter of minimal supported FreeBSD version. |
|
@robn thanks, I updated the release notes page. |
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Description
Proposed zfs-2.2.7 patchset. Support 6.11 kernel and github QEMU runners.
How Has This Been Tested?
Runners will test
Types of changes
Checklist:
Signed-off-by.