arc: harness buf_hdr's anon state invariant checks
There are several places where hdr is checked for arc_anon state
without the hash_lock, for example at arc_release() or at
arc_buf_destroy(), and it's supposed that the hdr must not be
in the hash table in this state and not having IO_IN_PROGRESS.
However, those invariants are not asserted in release builds.
But even if they would, there is one place in the current code
where they would pass without noticing the problem.
This place is at arc_write_done() where we first insert hdr
into the hash table, then clear IO_IN_PROGRESS flag, then drop
the reference and only then move its state from anon to mru:
7048 exists = buf_hash_insert(hdr, &hash_lock); /* still anon */
...
7081 arc_hdr_clear_flags(hdr, ARC_FLAG_IO_IN_PROGRESS);
7082 VERIFY3S(remove_reference(hdr, hdr), >, 0); /* ref: 2 -> 1 */
7084 if (exists == NULL && hdr->b_l1hdr.b_state == arc_anon)
[34 lines not shown]
zstd: declare __asan_*_memory_region() for user space builds
ZSTD_ASAN_POISON() and ZSTD_ASAN_UNPOISON() call
__asan_poison_memory_region() and __asan_unpoison_memory_region()
directly, but nothing declares them in the user space build. The kernel
block just above declares both -- and stubs them out, since KASAN does
not provide them -- while the user space block was left with only the
macros.
That builds where an implicit function declaration is a warning, since
the real symbols come from libasan at link time, but fails on compilers
that make it an error, as GCC 14 and newer do by default:
zfs_zstd.c:278:34: error: implicit declaration of function
'__asan_poison_memory_region' [-Wimplicit-function-declaration]
Declare both in the user space block, mirroring the kernel block.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Moch <mail at alexmoch.com>
Closes #19017
unit: add nvpair test suite
Exercises add and lookup for all types, iteration, removal, replacement,
merge & copy, pack & unpack with both encodings, formatting and
infallibile wrappers.
Sponsored-by: TrueNAS
Reviewed-by: Chris Longros <chris.longros at gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #19006
unit: add null/notnull helpers
Sponsored-by: TrueNAS
Reviewed-by: Chris Longros <chris.longros at gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #19006
unit: make unit_eq() etc type-aware
Previously, unit_eq() and friends were naively routed to
munit_assert_uint64(), regardless of the actual types involved. This
meant that on failure, the error message would report eg a small
negative number as a huge unsigned one, and also plain doesn't work for
types that don't have an implicit conversion to uint64_t, like double.
Here we add a type-aware replacement that generates comparison and
failure reporting functions for the wanted types, then uses _Generic to
dispatch to the correct one.
Sponsored-by: TrueNAS
Reviewed-by: Chris Longros <chris.longros at gmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #19006
Inherit the project ID for every object type
zfs_mknode() only assigned a project ID to regular files and
directories, so a symlink, device node, FIFO or socket created inside a
directory with ZFS_PROJINHERIT set was left at ZFS_DEFAULT_PROJID. The
cross-project checks in zfs_rename() and zfs_link() compare the object's
project ID against the directory's, so such an object is treated as
foreign to the very directory holding it, and cannot be renamed or
linked there at all -- "ln -sfn", which creates the new symlink under a
temporary name and renames it into place, fails with EXDEV.
ext4 and XFS store a project ID on every inode type and so do not have
this problem. Do the same, and quota-check new symlinks and rename
whiteouts against the inherited ID rather than the default one, so their
space is accounted to the project that owns them.
This changes accounting for newly created objects: symlinks, device
nodes and FIFOs now consume the project's quota where they previously
consumed none, so on a dataset already at its project quota, creating
[12 lines not shown]
Allow renames within a single directory under project inheritance
A rename that keeps the object in the directory it already lives in
cannot move it between projects, so refusing it with EXDEV is never
right. Objects created before the previous commit carry no project ID of
their own, which makes this reachable on existing pools: a symlink in a
project directory cannot be renamed even to another name beside itself,
and "ln -sfn" over an existing path fails.
zfs_link() keeps its unconditional check. It has no source directory to
compare against -- a hard link names an object that may live anywhere --
so there is no equivalent "the object is already here" case to exempt.
Linking a pre-existing symlink, device node or FIFO into the project
directory that already holds it therefore still fails with EXDEV, until
the object is given a project ID of its own.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Matt Turner <mattst88 at gmail.com>
Closes #18932
ZTS: check project ID inheritance and renames within a project directory
Covers both of the preceding changes. For the inheritance: a new symlink
and a new FIFO each add an object to the containing directory's project,
which only happens once non-regular files inherit a project ID, and a
symlink can then be renamed into a different directory carrying the same
project ID, which rename(2) permits only when the two project IDs match.
For the rename exemption: a regular file and a symlink are created
before the directory is tagged, so they carry no project ID of their
own, as every symlink on an existing pool does. Renaming each of them
within that directory afterwards fails with EXDEV unless the
cross-project check exempts renames whose source and target directories
are the same. The same renames are then repeated for objects created
after the tagging, along with replacing a symlink with "ln -sfn", and
again in an inheriting subdirectory.
A rename that does cross into a different project is still refused.
[7 lines not shown]
DDT: Use proper size when calling kmem_free on ddt_prune_entry_t
Linux and FreeBSD do not need a correct size passed to kmem_free, but
other platforms can.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
Signed-off-by: Richard Yao <richard at ryao.dev>
Closes #19029
CI: force GRUB onto serial console for headless VM
The QEMU CI VMs have no display device. When the
deps step powers the VM off and the build step
restarts it, grub2-mkconfig emits
'terminal_output gfxterm', which leaves GRUB stuck
before the kernel starts on the headless VM, so SSH
never comes up (seen on CentOS Stream 10). Force
GRUB_TERMINAL_INPUT/OUTPUT to the serial console,
which is always present in these VMs.
Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: tiehexue <tiehexue at hotmail.com>
Closes #19002
zstream: self-tuning queues
This PR removes the `qp_batch_budget` and `qp_queue_length` parameters
from the `zq_params_t` struct that's used to create `zstream_queues.`
Queue lengths and batch budgets still exist, but they're set internally
by the queue implementation. In addition, `ZQ_MAX_BATCH` is raised to
1024 items.
### Motivation
Multithreaded queues need some specific value for queue lengths and
batch sizes, but the impact of these values is largely opaque to
developers without detailed benchmarking.
Queue lengths in particular have been difficult to optimize because they
were formerly the only way to limit the amount of in-flight stream
data. Streams vary in payload density, so queue lengths were set
according to relatively pessimistic assumptions. But as of #18982, the
memory consumed by in-flight payloads is limited independently of queue
[102 lines not shown]
docs: fix typo independantly -> independently
Reviewed-by: George Melikov <mail at gmelikov.ru>
Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
Signed-off-by: Vaibhav Srivastava <vaibhavsri1712 at gmail.com>
Closes #19015
ZTS: Do not run rm -rf / when $TESTPOOL is empty
Grok accidentally did this to itself in a cloud development environment.
Let us learn from its goof.
This also handles other cases where $TESTPOOL is set to something like
`./`, which would still lead to ` rm -rf /` in our scripts. In these
cases, rm implementations that implement --no-preserve-root such as GNU
coreutils 9.1 and later, will not save us.
Reported-by: Grok 4.5 Build Beta
Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
Reviewed-by: Chris Longros <chris.longros at gmail.com>
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
Signed-off-by: Richard Yao <richard at ryao.dev>
Closes #18933
ZTS: exec_001_pos: copy the exec test binary under its own name
The test copies $STF_PATH/ls to $TESTDIR/myls and executes it directly,
to verify a filesystem with exec=on actually allows running a binary
from it. On Alpine, ls is an applet of a multi-call binary (the
"coreutils" package) that dispatches on argv[0]'s basename; invoked as
"myls" it doesn't recognize any applet by that name and exits with
"coreutils: unknown program 'myls'" before the exec=on behavior this
test is meant to verify ever comes into play.
Copy it to $TESTDIR/ls instead of $TESTDIR/myls. Dispatch is by basename
only, so this is a plain rename with no other behavior change, and
continues to exercise exactly what the test intends: copy a real binary
onto the pool and confirm it runs and mmap(2)s with PROT_EXEC
successfully.
exec_002_neg.ksh has the identical $TESTDIR/myls pattern but sets
exec=off before ever trying to run it; the kernel refuses the execve(2)
itself (EACCES/126) before the binary's own argv[0] dispatch would run
[7 lines not shown]
ZTS: fix user_namespace_001 on Alpine Linux (BusyBox)
On Alpine, touch and chmod are symlinks to /bin/busybox, a multi-call
binary that determines the program to run from argv[0]. readlink -f
resolves through the symlink to /bin/coreutils, losing the program name,
which causes BusyBox to fail with "unknown program".
Drop readlink -f and use command -v directly, which returns the symlink
path itself. On other platforms the commands are real binaries, so this
is a no-op.
Fixes the following tests on Alpine 3.24:
- user_namespace/user_namespace_001
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Moch <mail at alexmoch.com>
Closes #18999
ZTS: procfs_list_stale_read: accept Alpine's EIO error message
The procfs_list_stale_read test expects `cat` to print "Input/output
error", which is the wording used by GNU coreutils. Alpine's `cat`
reports the same EIO condition as "I/O error", causing the test to fail
even though the expected I/O error occurred.
Accept both error message variants so the test is independent of the
`cat` implementation's wording.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Moch <mail at alexmoch.com>
Closes #18998
vdev_disk_open: fix ENOENT retry & cleanup error handling
In 88261a83ae I reorganized the blockdev open error handling to
accommodate the permission check. What I didn't notice was that I'd made
the ENOENT case break after the timeout, rather than looping and
retrying.
This commit fixes it up the right way, and this time adds a lot more
commentary to make it easier to see what's happening for each case.
Sponsored-by: TrueNAS
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Reviewed-by: Ameer Hamza <ahamza at ixsystems.com>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18993
arc: harness buf_hdr's anon state invariant checks
There are several places where hdr is checked for arc_anon state
without the hash_lock, for example at arc_release() or at
arc_buf_destroy(), and it's supposed that the hdr must not be
in the hash table in this state and not having IO_IN_PROGRESS.
However, those invariants are not asserted in release builds.
But even if they would, there is one place in the current code
where they would pass without noticing the problem.
This place is at arc_write_done() where we first insert hdr
into the hash table, then clear IO_IN_PROGRESS flag, then drop
the reference and only then move its state from anon to mru:
7048 exists = buf_hash_insert(hdr, &hash_lock); /* still anon */
...
7081 arc_hdr_clear_flags(hdr, ARC_FLAG_IO_IN_PROGRESS);
7082 VERIFY3S(remove_reference(hdr, hdr), >, 0); /* ref: 2 -> 1 */
7084 if (exists == NULL && hdr->b_l1hdr.b_state == arc_anon)
[32 lines not shown]
zdb: add detailed diagnostics for MOS leaks and spacemap refs
- Print per-object leak details for leaked DSL clone maps and
leaked SPA space map objects.
- When spacemap histogram refcounts mismatch, print component
breakdown (dtl, metaslab, obsolete, checkpoint, log).
- List unreferenced histogram space map objects with their
`smp_alloc` and `smp_length` fields for debugging.
Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Martin Minkus <martin.minkus at gmail.com>
Closes #18906
ZTS: fix zinject argument order in mmp_write_uberblocks
glibc's getopt() permutes argv by default (a GNU extension), reordering
flags to the front regardless of where they appear. musl's getopt() is
strict POSIX and stops parsing options at the first non-option argument.
This test's zinject invocation places -L after the positional pool name
($TESTPOOL), which only glibc tolerates; under musl, zinject sees two
leftover positional tokens ($TESTPOOL and the argument to the now-
unparsed -L) and fails with "device (-d) injection requires a single
pool name" instead of injecting the fault.
Move "-L uber" before the positional pool name. This is accepted
identically under glibc's permuting getopt(), so the change is a no-op
on every other platform.
Fixes the following tests on Alpine 3.24:
- mmp/mmp_write_uberblocks
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
[2 lines not shown]
Pool split leaks DTL spacemap objects
zpool split does not release DTL space map objects for
removed vdevs. Use the same approach as in spa_vdev_detach().
Mark vdev detached, and dirty DTL.
vdev_count_leaves() is implemented to set reader lock
to reliably count vdevs with private function. However,
while we are already holding spa config locks, we can not
count vdevs.
zpool split does clone original pool MOS for new pool byte-by-byte,
including DTL data for vdevs. As new pool does not have vdevs
left into old pool, this would result leaking those DTL's.
ZTS updates:
remove vdev_zaps_007_pos from zts-report.py.in
zpool_split_devices: use zdb -MC to test split pools
[3 lines not shown]
zstream: track and limit memory use
`zstream` reads stream record payloads into memory while processing a
stream. Most subcommands use several parallel queues, which currently
have queue lengths on the order of 1000 slots. Depending on the
pipeline, there may be several thousand records in memory at any given
moment.
This is fine for typical filesystem send streams because most records
aren't payload-bearing and default record sizes are modest. However,
it's possible for streams to be far denser, as illustrated by this zvol
send stream breakdown posted by @ryan-moeller in #18565:
```
SUMMARY:
Total DRR_BEGIN records = 1 (0 bytes)
Total DRR_END records = 1 (0 bytes)
Total DRR_OBJECT records = 2 (0 bytes)
Total DRR_FREEOBJECTS records = 1 (0 bytes)
[36 lines not shown]
ZTS: give mmp_reset_interval headroom above the suspend window
A pool suspends when no MMP write succeeds for fail_intervals *
multihost_interval ms. The test drives the interval down to
MMP_INTERVAL_MIN while fail_intervals is as low as
MMP_FAIL_INTERVALS_MIN, which arms a window of 200 ms. A test machine
which stalls for longer than that suspends the pool, and the suspended
pool then keeps its devices, so the test which runs next cannot create
its own pool either.
Add MMP_INTERVAL_TEST_MIN, half the default interval, and use it as the
floor wherever this test varies the interval. The smallest window it
can arm becomes 1000 ms.
The floor is needed in the second loop as well as the first. An import
restarts the mmp thread, which initialises the window flat at
fail_intervals * interval with no smoothing, so the export and import
actions could otherwise still arm a window as small as 200 ms.
[3 lines not shown]
ZTS: resume a suspended pool in mmp_reset_interval cleanup
A pool suspended by MMP cannot be destroyed, so a failure which
suspends it leaves the pool and its devices behind, and the test which
runs next cannot create its own pool either. Since c3f2f1aa2 such a
pool can be resumed, so clear it before the destroy.
Add mmp_clear_suspended() to mmp.kshlib and call it from
mmp_reset_interval cleanup. The helper lives in the library so the
rest of the suite can take the same guard.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Michael Heller <75820586+mkhllr at users.noreply.github.com>
Closes #18977
CI: Add tzdata, libcap-utils, and ksh93's stable branch to Alpine
Missing tzdata silently breaks history_007_pos's TZ-dependent timestamp
check (stays in UTC instead of shifting). Missing libcap-utils breaks
zoned_uid_common.kshlib's capsh-based tests ("unshare: failed to
execute"). And building ksh93 from its unpinned "dev" branch, rather
than the stable "1.0" branch, causes musl crashes in ksh93's own
environment-building code, hit by the zpool_iostat/zpool_status "-c"
custom-script tests.
Combined into one commit since all three touch the same function in
qemu-3-deps-vm.sh.
Fixes the following tests on Alpine 3.24:
- history/history_007_pos
- zoned_uid/zoned_uid_023_pos
- zoned_uid/zoned_uid_025_pos
- zoned_uid/zoned_uid_026_pos
- zoned_uid/zoned_uid_030_pos
[10 lines not shown]
dmu_objset_open_impl: unregister prop callbacks on error
If dsl_prop_register() fails partway, the error path frees the objset
without removing the callbacks already registered, leaving records
that point at freed memory. Call dsl_prop_unregister_all() first.
Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Martin Minkus <martin.minkus at gmail.com>
Closes #18903
ZTS: fix CDDL boilerplate in zpool_expand_007_pos.ksh
Commit 1b7143557 added this file with the old OpenSolaris-style CDDL
header (CDDL HEADER START/END wrappers, wrong URL). Replace with the
current short-form CDDL boilerplate, as done in f836c9ee7 for the
block_cloning FIDEDUPERANGE tests.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Moch <mail at alexmoch.com>
Closes #18985
ZTS: replace uncompress with gunzip in history tests
Alpine Linux (musl/BusyBox) does not provide uncompress; gunzip handles
.Z (LZW compress) files and accepts the same -f flag, making it a
drop-in replacement on all supported platforms.
Fixes the following tests on Alpine 3.24:
- history/history_001_pos
- history/history_007_pos
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Moch <mail at alexmoch.com>
Closes #18981
Keep a grown vdev from adopting an older pool's labels
Labels 2 and 3 sit at offsets relative to the end of a device, so
growing a vdev moves them onto space the pool has never written.
vdev_copy_uberblocks() already says as much of the uberblock rings
found there, "either empty or contain garbage", yet the config and the
uberblocks are still read from those labels as though they were ours.
Whatever pool held the device before is usually still on it, its labels
are perfectly well formed, and vdev_label_read_config() ranks labels by
nothing but txg, so a leftover label with a higher txg wins and
vdev_validate() fails the device for belonging to a foreign pool. The
reporter grew LVM volumes laid over used disks and got a raidz child
back FAULTED with "corrupted data"; where no redundancy is left to
cover the member, the pool suspends. Issue #16144.
Read the config for validation from the two labels at the head of the
device, whose offsets do not depend on its size, and fall back to the
trailing pair only when the head cannot be read, so a damaged head
still has the second copy behind it. vdev_label_sync() writes the
[45 lines not shown]
zstream_selftest_queue: fix alignas(type) build failure under C99
alignas() with a type-name argument is C11 syntax; this file is
compiled with -std=gnu99 where _Alignas only accepts constant
expressions. Replace alignas(uint64_t) with alignas(__alignof__(
uint64_t)), which is a GCC/Clang builtin available in all modes and
produces the same alignment value.
Musl's stdalign.h defines alignas unconditionally (unlike the glibc /
compiler built-in header which guards it behind a C11 version check),
so this manifests when building against musl (tested on Alpine 3.24
x86_64 and Gentoo AArch64).
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Reviewed-by: Garth Snyder <garth at garthsnyder.com>
Signed-off-by: Alexander Moch <mail at alexmoch.com>
Closes #18971
