While most objects can perform all their cleanup in the finalizer
method, there can be interactions with other resources / subsystems
/ threads which require that some cleanup be performed on an user
creatable object before unparenting it and entering finalization.
The current 'can_be_deleted' method runs in the deletion path and
is intended to be used to block deletion. While it could be used
to perform cleanup tasks, its name suggests it should be free of
side-effects.
Generalize this by renaming it to 'prepare_delete', explicitly
allowing for cleanup to be provided. Existing users of 'can_be_deleted'
are re-written, which provides them with more detailed/tailored error
messages.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20260706135824.2623960-2-berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
pci, vhost, virtio, iommu: features, fixes, cleanups
intel_iommu:
PASID support for passthrough
some properties renamed
virtio-rtc:
new device
acpi:
watchdog (x86 q35)
COM irqs are now shared
vhost-user:
vhost-user passes GPA not HVA now
vhost SHMEM_MAP/UNMAP support
vhost-vdpa:
svq IN_ORDER support
amd_iommu:
IOMMU XT interrupt support
command buffer fixes
cxl:
PPR support
performant path for non-interleaved cases
vhost-scsi:
build fix for older kernel headers
fixes, cleanups all over the place
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
# -----BEGIN PGP SIGNATURE-----
#
# iQFDBAABCgAtFiEEXQn9CHHI+FuUyooNKB8NuNKNVGkFAmowWT0PHG1zdEByZWRo
# YXQuY29tAAoJECgfDbjSjVRpCHUH+QGGh1U4mM/u5tsPx2w7Bpyut/Fqv4W5YkuX
# XgcbOulZ9DLd6jKOt4na0AsXNvX90fMXvbj+tuDZ3lLKdRzEhmE6HPPQbKvQjzIK
# Ag2vXQqTOagdBLbViRpI2Vnt09Cie6B0kRYz+GhbG8EZxgFcOdydWUwVeLXyCLSW
# hA6IWhBMNxExeWsXiZwFZTv38eJi+s/BEpuIEAdwv4TqBPOq4yjxQScAoCceGDLJ
# jyTmU9dTfwx21K/0Ivp68ANLMnDPr+83yY+8zuLmvT0Tq7H9/blgqkD/TpFd19BM
# 0W3ep/xk1ZJR6Vd+73+pWTbQOEM+qjlHb1WM0lF/ZePOArIq+aQ=
# =AVMI
# -----END PGP SIGNATURE-----
# gpg: Signature made Mon 15 Jun 2026 15:57:49 EDT
# gpg: using RSA key 5D09FD0871C8F85B94CA8A0D281F0DB8D28D5469
# gpg: issuer "mst@redhat.com"
# gpg: Good signature from "Michael S. Tsirkin <mst@kernel.org>" [full]
# gpg: aka "Michael S. Tsirkin <mst@redhat.com>" [full]
# Primary key fingerprint: 0270 606B 6F3C DF3D 0B17 0970 C350 3912 AFBE 8E67
# Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA 8A0D 281F 0DB8 D28D 5469
* tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu: (106 commits)
hw/scsi/vhost-scsi: fix build with older kernel headers
tests/qtest: add 8-byte MMIO access sweep for intel-iommu
intel_iommu: fix guest-triggerable abort on oversized MMIO access
hw/cxl: Add a performant (and correct) path for the non interleaved cases
hw/cxl: Allow cxl_cfmws_find_device() to filter on whether interleaved paths are accepted
hw/cxl/events: Fix handling of component ID in event records generation to not assume it is a string
hw/cxl: Add fixes in Post Package Repair (PPR)
hw/cxl: Fix handling of component ID to not assume it is a string
vhost-user.rst: fix typo
vhost-user-device: Add shared memory BAR
qmp: add shmem feature map
vhost_user.rst: Add GET_SHMEM_CONFIG message
vhost_user: Add frontend get_shmem_config command
vhost_user.rst: Add SHMEM_MAP/_UNMAP to spec
vhost_user.rst: Align VhostUserMsg excerpt members
vhost-user: Add VirtIO Shared Memory map request
tests: acpi: x86/q35: update expected WDAT blob
tests: acpi: x86/q35: add WDAT table test case
tests: acpi: x86/q35: whitelist new WDAT table
x86: q35: generate WDAT ACPI table
...
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
It's not possible to access the image file while there is an incoming
migration in progress, the QEMU process doesn't hold any locks to the
storage at this point so nodes are inactive. Attempting to flush leads
to an assert at bdrv_co_write_req_prepare():
assert(!(bs->open_flags & BDRV_O_INACTIVE))
The issue is reproducible by running iotest 181 on a host under cpu
load. The migration must coincide with the header already containing
the QED_F_NEED_CHECK flag.
The sequence of events is as follows, with the respective call stacks
referenced below:
During block device init, bdrv_qed_attach_aio_context() starts the
'need_check' timer. The timer will not fire during incoming migration
as it uses QEMU_CLOCK_VIRTUAL (to avoid this very issue, as the code
comment indicates). (0)
However, there's still bdrv_qed_drain_begin() which uses the fact that
the timer is live to decide whether to start the
qed_need_check_timer_entry() directly. (1)
The qed_need_check_timer_entry() eventually calls into
qed_write_header() -> bdrv_co_pwrite() leading to the assert. (2)
Skip creating the 'need_check' timer whenever the image is inactive.
The stacks:
(0) == issues timer_mod ==
#6 in qed_start_need_check_timer at ../block/qed.c:340
#7 in bdrv_qed_attach_aio_context at ../block/qed.c:373
#8 in bdrv_qed_do_open at ../block/qed.c:556
#9 in bdrv_qed_open_entry at ../block/qed.c:582
#10 in coroutine_trampoline at ../util/coroutine-ucontext.c:175
#0 in qemu_coroutine_switch<+120> at ../util/coroutine-ucontext.c:321
#1 in qemu_aio_coroutine_enter<+356> at ../util/qemu-coroutine.c:293
#2 in aio_co_enter<+179> at ../util/async.c:710
#3 in aio_co_wake<+53> at ../util/async.c:695
#4 in thread_pool_co_cb<+47> at ../util/thread-pool.c:283
#5 in thread_pool_completion_bh<+241> at ../util/thread-pool.c:202
#6 in aio_bh_call<+109> at ../util/async.c:173
#7 in aio_bh_poll<+299> at ../util/async.c:220
#8 in aio_poll<+690> at ../util/aio-posix.c:745
#9 in bdrv_qed_open<+392> at ../block/qed.c:607
#10 in bdrv_open_driver<+327> at ../block.c:1678
#11 in bdrv_open_common<+1619> at ../block.c:2008
#12 in bdrv_open_inherit<+2556> at ../block.c:4191
#13 in bdrv_open<+118> at ../block.c:4286
#14 in blk_new_open<+199> at ../block/block-backend.c:458
#15 in blockdev_init<+2011> at ../blockdev.c:612
#16 in drive_new<+3008> at ../blockdev.c:1008
#17 in drive_init_func<+51> at ../system/vl.c:662
#18 in qemu_opts_foreach<+227> at ../util/qemu-option.c:1148
#19 in configure_blockdev<+350> at ../system/vl.c:721
#20 in qemu_create_early_backends<+343> at ../system/vl.c:2076
#21 in qemu_init<+12483> at ../system/vl.c:3778
#22 in main<+46> at ../system/main.c:71
(1) == sees timer_pending ==
#6 in bdrv_qed_drain_begin at ../block/qed.c:391
#7 in bdrv_do_drained_begin at ../block/io.c:366
#8 in bdrv_do_drained_begin_quiesce at ../block/io.c:386
#9 in bdrv_child_cb_drained_begin at ../block.c:1207
#10 in bdrv_parent_drained_begin_single at ../block/io.c:133
#11 in bdrv_parent_drained_begin at ../block/io.c:64
#12 in bdrv_do_drained_begin at ../block/io.c:364
#13 in bdrv_drained_begin at ../block/io.c:393
#14 in blk_drain at ../block/block-backend.c:2101
#15 in blk_unref at ../block/block-backend.c:544
#16 in bdrv_open_inherit at ../block.c:4197
#17 in bdrv_open at ../block.c:4286
#18 in blk_new_open at ../block/block-backend.c:458
#19 in blockdev_init at ../blockdev.c:612
#20 in drive_new at ../blockdev.c:1008
#21 in drive_init_func at ../system/vl.c:662
#22 in qemu_opts_foreach at ../util/qemu-option.c:1148
#23 in configure_blockdev at ../system/vl.c:721
#24 in qemu_create_early_backends at ../system/vl.c:2076
#25 in qemu_init at ../system/vl.c:3778
#26 in main at ../system/main.c:71
(2) == crashes ==
#5 in __assert_fail (assertion="!(bs->open_flags & BDRV_O_INACTIVE)", file="../block/io.c", line=1977
#6 in bdrv_co_write_req_prepare at ../block/io.c:1977
#7 in bdrv_aligned_pwritev at ../block/io.c:2099
#8 in bdrv_co_pwritev_part at ../block/io.c:2316
#9 in bdrv_co_pwritev at ../block/io.c:2233
#10 in bdrv_co_pwrite at ../include/block/block_int-io.h:77
#11 in qed_write_header at ../block/qed.c:128
#12 in qed_need_check_timer at ../block/qed.c:305
#13 in qed_need_check_timer_entry at ../block/qed.c:319
Note that this issue is not exactly the same as what's been reported
in Gitlab, but given how easily this reproduces, I imagine it has to
be happening in that setup as well.
Link: https://gitlab.com/qemu-project/qemu/-/work_items/3515
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Message-ID: <20260603193813.2327596-1-farosas@suse.de>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit 8599559580 ("fuse: Set direct_io and parallel_direct_writes")
broke use cases that require mmap() with MAP_SHARED on the export. In
particular, swtpm_setup using its 'file://' protocol requires this.
From the kernel documentation [0]:
> To allow shared mmap, the FUSE_DIRECT_IO_ALLOW_MMAP flag may be
> enabled in the FUSE_INIT reply.
Set the FUSE_DIRECT_IO_ALLOW_MMAP flag to restore compatibility with
users requiring shared mmap. The FUSE_INIT_EXT flag needs to be set
for the flags2 member to have an effect.
[0]: https://www.kernel.org/doc/html/next/filesystems/fuse/fuse-io.html
Cc: qemu-stable@nongnu.org
Fixes: 8599559580 ("fuse: Set direct_io and parallel_direct_writes")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20260506145424.10249-3-f.ebner@proxmox.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The code is switched to use the current 'struct fuse_init_in' in
preparation to use the FUSE_DIRECT_IO_ALLOW_MMAP feature, which is
part of the flags2 member that got added in protocol version 5.36.
To not break compatibility with older kernels, the check for whether
the full header of an operation was read in co_read_from_fuse_fd()
needs to be adapted. In particular, for a FUSE_INIT operation, the
protocol version must be considered, because the length of the header
changed with protocol version 7.36. Always using the length of the
old, shorter struct was inaccurate, since for newer protocol versions
this might mean accepting a truncated read for FUSE_INIT.
Users of the init header that want to use parts of the extended
structure must check with the using_old_fuse_init_in() helper function
if they may do so.
Cc: qemu-stable@nongnu.org
Fixes: a94a1d7699 ("fuse: Manually process requests (without libfuse)")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20260506145424.10249-2-f.ebner@proxmox.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit b8bfb1478d ("qcow2: Fix corruption on discard during write with
COW") added a wait_for_dependencies() at the start of
qcow2_subcluster_zeroize(). That fixes the inconsistency it set out to
fix, but turns the lock-protected pre-check in the caller,
qcow2_co_pwrite_zeroes(), into a stale one: the wait yields s->lock,
so an in-flight allocating write whose QCowL2Meta is already on
s->cluster_allocs (but whose L2 entry is not yet linked) gets to link
its entry during the yield. When the zeroize wakes, the cluster is now
NORMAL, and with BDRV_REQ_MAY_UNMAP the free path in zero_in_l2_slice()
unmaps the just-written cluster, silently dropping the data write's
payload.
This is reachable with detect-zeroes=unmap (the default for VirtIO
disks with discard on in Proxmox VE), under which the block layer
auto-promotes all-zero buffers to BDRV_REQ_ZERO_WRITE |
BDRV_REQ_MAY_UNMAP. A memory-constrained Debian guest running 'apt
full-upgrade' on such a disk reproduces it as random SIGSEGVs:
swapped-out code pages come back as zero.
Wait for in-flight dependencies before the lock-protected check in
qcow2_co_pwrite_zeroes(). If a write linked its L2 entry during the
wait, the type check now fails and the block layer falls back to a
bounce-buffered zero write that only touches the requested subrange,
preserving the racing write's data. Promote wait_for_dependencies() to
qcow2_wait_for_dependencies() so qcow2.c can call it.
Fixes: b8bfb1478d ("qcow2: Fix corruption on discard during write with COW")
Cc: qemu-stable@nongnu.org
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Message-ID: <20260522151318.238064-1-t.lamprecht@proxmox.com>
[kwolf: Reverted unnecessary change to 'nr' assignment]
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit 5634622bcb ("file-posix: allow BLKZEROOUT with -t writeback")
enables the BLKZEROOUT ioctl when using 'writeback' cache, regressing
certain 'qemu-img convert' invocations, because of a pre-existing
issue. Namely, the BLKZEROOUT ioctl might fail with errno EINVAL when
the request is shorter than the block size of the block device.
Fallback to the bounce buffer, similar to when the ioctl is not
supported at all, rather than treating such an error as fatal.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3257
Resolves: https://bugzilla.proxmox.com/show_bug.cgi?id=7197
Cc: qemu-stable@nongnu.org
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20260105143416.737482-1-f.ebner@proxmox.com>
[Added TODO comment describing a larger fix that could be implemented in
the future.
--Stefan]
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
We still build QEMU tools on 32-bit hosts (see commit
cf634dfcd8), however no OS supported by QEMU still runs
on ESA/390 (Linux dropped support in release 4.1 in 2015).
Remove the configure check, directly checking for the 64-bit
z/Architecture.
Also per commit 3704993f54 from 2020:
"we don't support s390, only 64-bit s390x hosts".
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-ID: <20260519171240.97420-4-philmd@linaro.org>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
The QSD vhost-user-blk backend advertises VIRTIO_BLK_F_SIZE_MAX but
sets size_max=0 in config, creating a contradictory state. The feature
bit tells the guest that size_max is valid, but the value is zero.
Windows viostor trusts the feature bit, reads size_max=0, and uses it
in scatter-gather calculations producing zero-length segments that hang
I/O. The disk appears empty (no GPT, no partitions), causing
INACCESSIBLE_BOOT_DEVICE BSOD. Linux tolerates this because
blk_validate_limits() silently corrects max_segment_size=0 to
BLK_MAX_SEGMENT_SIZE (65536).
This affects any VMM using QSD as a vhost-user-blk backend (QEMU,
Cloud Hypervisor, etc.), since both pass through SIZE_MAX from the
backend to the guest via feature intersection.
Remove VIRTIO_BLK_F_SIZE_MAX from the backend feature bits. The
size_max config field remains zero but is now ignored by guests since
the feature is not negotiated.
Signed-off-by: Max Makarov <maxpain@linux.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20260331152856.1802-1-maxpain@linux.com>
qemu_laio_process_completions() wraps its body in defer_call_begin /
defer_call_end. Inside the section, completion callbacks wake coroutines
that queue new aiocbs; laio_do_submit() defers laio_deferred_fn. At the
bottom of qemu_laio_process_completions() the defer_call_end() fires
laio_deferred_fn, which calls ioq_submit(), closing the cycle:
ioq_submit
-> io_submit(2) // some sync completions
-> qemu_laio_process_completions // defer_call_begin
-> aio_co_wake // resumes coroutine
-> laio_do_submit
-> defer_call(laio_deferred_fn, s) // enqueued
-> defer_call_end // nesting drops to 0
-> laio_deferred_fn
-> ioq_submit // +1 stack frame, loop
When io_submit(2) returns asynchronously (O_DIRECT) the cycle
terminates in one extra frame: the fresh aiocb is still in flight, no
completion is drained, no coroutine wakes, no new submission queues.
When submissions complete synchronously (non-O_DIRECT, or per-descriptor
drivers such as vmdk) each level enqueues more work for the next
defer_call_end() to drain, so recursion grows without bound and QEMU
crashes with SIGSEGV on the thread guard page.
The cycle was closed by two performance commits, each correct in
isolation:
076682885d ("block/linux-aio: convert to blk_io_plug_call() API")
-- introduced laio_deferred_fn and wired
laio_do_submit -> defer_call(laio_deferred_fn, s).
84d61e5f36 ("virtio: use defer_call() in virtio_irqfd_notify()")
-- added defer_call_begin/end around qemu_laio_process_completions
so virtio-irqfd notifications batch across a completion pass.
The supported aio=native + cache=none pairing keeps submissions
asynchronous, so the cycle stays bounded; nothing in the code enforces
that contract. Observed in production as a SIGSEGV during a backup job
configured with --cached + aio=native; reproducible on upstream with
qemu-io against vmdk.
Cap ioq_submit() recursion with a counter on LaioQueue, which is only
accessed from the AioContext home thread. On overflow, return without
submitting. The pending work is drained by s->completion_bh, which
qemu_laio_process_completions() has already scheduled on entry -- no
work is lost; one event-loop round-trip of latency is paid only when
the bound is hit, which cannot happen on a supported configuration.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <20260520142503.251959-2-den@openvz.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Instead of a long line with key=value pairs for each block device,
switch to a tabular form with aligned values. This makes it much easier
to find the relevant information in the output.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260512112759.66038-1-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
tests/qemu-iotests/tests/iothreads-create reproduces the hang on
master under `stress-ng --cpu $(nproc) --timeout 0`. The iotest's
vm.run_job() times out and qemu stays permanently stuck in
ppoll(timeout=-1) inside bdrv_graph_wrlock_drained -> blk_remove_bs
during qemu_cleanup(). The timing window is narrow on modern
bare-metal hardware and much wider in a VM guest; downstream trees
that still use plain bdrv_graph_wrlock() in blk_remove_bs() hit it
on the first iteration under the same stress.
bdrv_graph_wrlock() zeroes has_writer around its AIO_WAIT_WHILE loop
so that callbacks dispatched by aio_poll() can still take the read
lock on the fast path. The rdunlock side, however, only kicks a
waiting writer when has_writer is observed set; a reader that drops
its lock inside the polling window silently returns and nothing ever
wakes the writer:
main thread iothread0 coroutine
----------- -------------------
bdrv_graph_wrlock: rdlock held, reader_count=1
bdrv_drain_all_begin_nopoll
has_writer = 0
AIO_WAIT_WHILE_UNLOCKED(
NULL, reader_count >= 1):
num_waiters++
smp_mb
aio_poll(main_ctx, true) --> bdrv_graph_co_rdunlock:
(ppoll, blocked) reader_count-- -> 0
smp_mb
read has_writer = 0
skip aio_wait_kick()
return
reader_count is now 0 and num_waiters is still 1, but no BH, fd or
timer on the main AioContext will fire -- the only entity that could
kick just decided it did not have to. Main stays in ppoll() holding
BQL, so RCU, VCPUs and any iothread path that needs BQL stall behind
it. The hang is final; no timeout, no forward progress, no recovery
as there is no other source of wake up inside qemu_cleanup().
bdrv_drain_all_begin() does not close the race on its own: it
quiesces in-flight I/O, but graph readers also include non-I/O
coroutines (block-job cleanup, virtio-scsi polling) that drain does
not evict. The bdrv_graph_wrlock_drained() wrapper narrows the
window but does not eliminate it; every plain bdrv_graph_wrlock()
site is exposed on the same basis.
Drop the has_writer check in bdrv_graph_co_rdunlock() and call
aio_wait_kick() unconditionally. The helper itself loads num_waiters
atomically and only schedules a dummy BH when a waiter exists, so the
change is a no-op on the no-writer path and closes the missed-wakeup
on the writer path.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Fiona Ebner <f.ebner@proxmox.com>
Message-ID: <20260424103917.248668-2-den@openvz.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Most code in qcow2 that accesses (and potentially modifies) L2 tables
does so while holding s->lock.
There is one exception, which is allocating writes. They hold the lock
initially while allocating clusters, but drop it for writing the guest
payload before taking the lock again for updating the L2 tables. This
allows concurrent requests that touch other parts of the image file to
continue in parallel and is an important performance optimisation.
However, this means that other requests that run while the lock is
dropped for writing guest data must synchronise with the list of
allocating requests in s->cluster_allocs and wait if they would overlap.
For writes, this is done in handle_dependencies(), but discard and write
zeros operations neglect to synchronise with s->cluster_allocs.
This means that discard can free a cluster whose L2 entry will already
be modified in qcow2_alloc_cluster_link_l2() by a previously started
write. In the case of a pre-allocated zero cluster that is in the
process of being overwritten, this means that discard can lead to a
situation where the cluster is still mapped (because the write will
restore the L2 entry just without the zero flag), but its refcount has
been decreased, resulting in a corrupted image.
Add the missing synchronisation to qcow2_cluster_discard() and
qcow2_subcluster_zeroize() to fix the problem.
Cc: qemu-stable@nongnu.org
Reported-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260427170520.101242-4-kwolf@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Tested-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The whole implementation of bdrv_commit() is only correct if no new
writes come in while it's running: It has only a single loop checking
the allocation status for each block and finally calls bdrv_make_empty()
without checking if that throws away any new changes.
We already have to drain while taking the graph write lock. Just extend
the drained section to all of bdrv_commit() to make sure that we don't
get any inconsistencies.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260427170520.101242-2-kwolf@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Tested-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Glusterfs has been marked as deprecated since QEMU v9.2, and as far
as I know, nobody spoke up 'til today that it should be kept.
The listed e-mail address integration@gluster.org in our MAINTAINERS
file seems to be bouncing nowadays, and looking at their website
https://www.gluster.org/ the most recent news are from 2020 / 2021 ...
so it seems like there is really hardly any interest in Glusterfs
anymore. Thus it's time to remove the code now from QEMU.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20260511063013.39805-1-thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
If a device uses blk_inc/dec_in_flight() in order to build macro
operations that involve multiple requests for the block layer and that
need to be completed as a unit before the BlockBackend can be considered
drained, it sets the stage for a deadlock: When a drain is requested,
the inner request at the BlockBackend level will be queued in
blk_wait_while_drained() and wait until the drained section ends, but at
the same time, drain_begin can only return if the whole macro operation
at the device level has completed.
Introduce a new interface to allow implementing the logic correctly:
Instead of queueing individual requests, blk_co_start_request() calls
blk_wait_while_drained() once at the beginning. The individual requests
must then set BDRV_REQ_NO_QUEUE to avoid being queued and running into
the deadlock; being wrapped in blk_co_start/end_request() makes sure
that drain_begin waits for them and they don't sneak in when the
BlockBackend is supposed to already be quiescent.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260421161132.99878-3-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Sometimes reproducing a problem for debugging involves slow I/O, so
let's add something to blkdebug to make I/O slow when we need it. This
can be used either together with an error so that the request fails
after the delay, or with errno=0, which allows the request to succeed
after the delay.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260421161132.99878-2-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The QSIMPLEQ_FOREACH() macro, defined in "qemu/queue.h",
always assigns its iterator variable when entering the
loop. Remove the pointless and possibly misleading
assignment.
Mechanical patch using the following coccinelle spatch:
@@
type T;
identifier e;
iterator FOREACH_MACRO =~ ".*_FOREACH.*";
statement S;
@@
- T *e = ...;
+ T *e;
... when != e
FOREACH_MACRO(e, ...) S
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Message-Id: <20260415215539.92629-4-philmd@linaro.org>
The QLIST_FOREACH() macro, defined in "qemu/queue.h",
always assigns its iterator variable when entering the
loop. Remove the pointless and possibly misleading
assignment.
Mechanical patch using the following coccinelle spatch:
@@
type T;
identifier e;
iterator FOREACH_MACRO =~ ".*_FOREACH.*";
statement S;
@@
- T *e = ...;
+ T *e;
... when != e
FOREACH_MACRO(e, ...) S
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
Message-Id: <20260415215539.92629-2-philmd@linaro.org>
This is a change in semantics. g_ascii_strcasecmp() doesn't honour
locale but strcasecmp() does. But this is OK for at least one reason:
(1) QEMU always runs with the C locale so there's not an actual
behaviour change here
(2) we want the comparison on url prefix and it should be a plain ASCII
one, not to do weird things with "I" in Turkish locales,
so g_ascii_strcasecmp() is better as it's explicit about that
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/r/20260327134401.270186-4-kkostiuk@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Passing an empty list of boundaries to block-latency-histogram-set sets
up a state that leads to a NULL pointer dereference when the next
request should be accounted for. This is not a useful configuration, so
just error out if the user tries to set it.
The crash can easily be reproduced with the following script:
qmp() {
cat <<EOF
{'execute':'qmp_capabilities'}
{'execute':'block-latency-histogram-set',
'arguments': {'id':'ide0','boundaries':[]}}
{'execute':'cont'}
EOF
}
qmp | ./qemu-system-x86_64 -S -qmp stdio \
-drive if=none,format=raw,file=null-co:// \
-device ide-hd,drive=none0,id=ide0
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20260331102608.60882-1-kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The QSD vhost-user-blk export sets opt_io_size=1 and min_io_size=1 in
the virtio config. These values are reported to the guest through the
VPD Block Limits page as OptimalTransferLength=1 block (512 bytes)
and OptimalTransferLengthGranularity=1 block.
Windows respects these hints and splits all I/O into ~512-byte
requests, causing ~100x sequential throughput degradation (150 MB/s
instead of 15+ GB/s). Linux is unaffected as its block layer ignores
these values.
Set both to 0 which means "not reported" per the SCSI Block Limits
VPD spec, allowing Windows to use its own optimal I/O size defaults.
Signed-off-by: Max Makarov <maxpain@linux.com>
Message-ID: <20260330193451.76037-1-maxpain@linux.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Short writes can happen, too, not just short reads. The difference to
aio=native is that the kernel will actually retry the tail of short
requests internally already -- so it is harder to reproduce. But if the
tail of a short request returns an error to the kernel, we will see it
in userspace still. To reproduce this, apply the following patch on top
of the one shown in HEAD^ (again %s/escaped // to apply):
escaped diff --git a/block/export/fuse.c b/block/export/fuse.c
escaped index 67dc50a412..2b98489a32 100644
escaped --- a/block/export/fuse.c
escaped +++ b/block/export/fuse.c
@@ -1059,8 +1059,15 @@ fuse_co_read(FuseExport *exp, void **bufptr, uint64_t offset, uint32_t size)
int64_t blk_len;
void *buf;
int ret;
+ static uint32_t error_size;
- size = MIN(size, 4096);
+ if (error_size == size) {
+ error_size = 0;
+ return -EIO;
+ } else if (size > 4096) {
+ error_size = size - 4096;
+ size = 4096;
+ }
/* Limited by max_read, should not happen */
if (size > FUSE_MAX_READ_BYTES) {
@@ -1111,8 +1118,15 @@ fuse_co_write(FuseExport *exp, struct fuse_write_out *out,
{
int64_t blk_len;
int ret;
+ static uint32_t error_size;
- size = MIN(size, 4096);
+ if (error_size == size) {
+ error_size = 0;
+ return -EIO;
+ } else if (size > 4096) {
+ error_size = size - 4096;
+ size = 4096;
+ }
QEMU_BUILD_BUG_ON(FUSE_MAX_WRITE_BYTES > BDRV_REQUEST_MAX_BYTES);
/* Limited by max_write, should not happen */
I know this is a bit artificial because to produce this, there must be
an I/O error somewhere anyway, but if it does happen, qemu will
understand it to mean ENOSPC for short writes, which is incorrect. So I
believe we need to resubmit the tail to maybe have it succeed now, or at
least get the correct error code.
Reproducer as before:
$ ./qemu-img create -f raw test.raw 8k
Formatting 'test.raw', fmt=raw size=8192
$ ./qemu-io -f raw -c 'write -P 42 0 8k' test.raw
wrote 8192/8192 bytes at offset 0
8 KiB, 1 ops; 00.00 sec (64.804 MiB/sec and 8294.9003 ops/sec)
$ hexdump -C test.raw
00000000 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a |****************|
*
00002000
$ storage-daemon/qemu-storage-daemon \
--blockdev file,node-name=test,filename=test.raw \
--export fuse,id=exp,node-name=test,mountpoint=test.raw,writable=true
$ ./qemu-io --image-opts -c 'read -P 23 0 8k' \
driver=file,filename=test.raw,cache.direct=on,aio=io_uring
read 8192/8192 bytes at offset 0
8 KiB, 1 ops; 00.00 sec (58.481 MiB/sec and 7485.5342 ops/sec)
$ ./qemu-io --image-opts -c 'write -P 23 0 8k' \
driver=file,filename=test.raw,cache.direct=on,aio=io_uring
write failed: No space left on device
$ hexdump -C test.raw
00000000 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 17 |................|
*
00001000 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a 2a |****************|
*
00002000
So short reads already work (because there is code for that), but short
writes incorrectly produce ENOSPC. This patch fixes that by
resubmitting not only the tail of short reads but short writes also.
(And this patch uses the opportunity to make it so qemu_iovec_destroy()
is called only if req->resubmit_qiov.iov is non-NULL. Functionally a
non-op, but this is how the code generally checks whether the
resubmit_qiov has been set up or not.)
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260324084338.37453-4-hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
When password-secret is used, curl_open() resolves it with
qcrypto_secret_lookup_as_utf8() and stores the returned buffer in
s->password.
Unlike s->proxypassword, s->password is not freed either in the open
failure path or in curl_close(), so the resolved secret leaks once it
has been allocated.
Free s->password in both cleanup paths.
Fixes: 1bff960642 ('curl: add support for HTTP authentication parameters')
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
Message-ID: <20260320063016.262954-1-zhaoguohan_salmon@163.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
If s->replace_blocker was already set by an earlier invocation of
mirror_complete(), then there will be an assertion failure when
error_setg() is called for it a second time. The bdrv_op_block_all()
and bdrv_ref() operations should only be done a single time too.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Message-Id: <20260311145717.668492-2-f.ebner@proxmox.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
When a timer is fired a pending I/O request is restarted and
tg->any_timer_armed is reset so other requests can be scheduled.
However we're resetting any_timer_armed first in timer_cb() before
the request is actually restarted, and there's a window between both
moments in which another thread can arm the same timer, hitting an
assertion in throttle_group_restart_queue().
This can be solved by deferring the reset of tg->any_timer_armed to
the moment when the queue is actually restarted, which is protected by
tg->lock, preventing other threads from arming the timer before that.
In addition to that, throttle_group_restart_tgm() is also updated to
hold tg->lock while the timer is being inspected. Here we consider
three different scenarios:
- If the tgm has a timer set, fire it immediately
- If another tgm has a timer set, restart the queue anyway
- If there is no timer set in this group then simulate a timer that
fires immediately, by setting tg->any_timer_armed in order to
prevent other threads from arming a timer in the meantime.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3194
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <825598ef34ad384d936da19d634eda75598508f7.1773316842.git.berto@igalia.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
S3 presigned URLs are signed for a specific HTTP method (typically GET
for our use cases). The curl block driver currently issues a HEAD
request to discover the web server features and the file size, which
fails with 'HTTP 403' (forbidden).
Add a 'force-range' option that skips the HEAD request and instead
issues a minimal GET request (querying 1 byte from the server) to
extract the file size from the 'Content-Range' response header. To
achieve this the 'curl_header_cb' is redesigned to generically parse
HTTP headers.
$ $QEMU -drive driver=https,\
'url=https://s3.example.com/some.img?X-Amz-Security-Token=XXX',
force-range=true
Enabling the 'force-range' option without the web server specified with
@url supporting it might cause the server to respond successfully with
'HTTP 200' and attempt to send the whole file body. With the
'CURLOPT_NOBODY' option set the libcurl will skip reading after the
headers and close the connection. QEMU still gracefully detects the
missing feature. This might waste a small number of TCP packets but is
otherwise transparent to the user.
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Antoine Damhet <adamhet@scaleway.com>
Message-ID: <20260227-fix-curl-v3-v3-3-eb8a4d88feef@scaleway.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
libnfs v6 added a new api structure for read and write requests.
This effectively also adds zero copy read support for cases where
the qiov coming from the block layer has only one vector.
The .brdv_refresh_limits implementation is needed because libnfs v6
silently dropped support for splitting large read/write request into
chunks.
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: Peter Lieven <pl@dlhnet.de>
Message-ID: <20260306142840.72923-1-pl@dlhnet.de>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
FUSE allows creating multiple request queues by "cloning" /dev/fuse FDs
(via open("/dev/fuse") + ioctl(FUSE_DEV_IOC_CLONE)).
We can use this to implement multi-threading.
For configuration, we don't need any more information beyond the simple
array provided by the core block export interface: The FUSE kernel
driver feeds these FDs in a round-robin fashion, so all of them are
equivalent and we want to have exactly one per thread.
These are the benchmark results when using four threads (compared to a
single thread); note that fio still only uses a single job, but
performance can still be improved because of said round-robin usage for
the queues. (Not in the sync case, though, in which case I guess it
just adds overhead.)
file:
read:
seq aio: 261.7k ±1.7k (+168%)
rand aio: 129.2k ±14.3k (+35%)
seq sync: 36.6k ±0.6k (+6%)
rand sync: 10.1k ±0.1k (+2%)
write:
seq aio: 235.7k ±2.8k (+243%)
rand aio: 232.0k ±6.7k (+237%)
seq sync: 31.7k ±0.6k (+4%)
rand sync: 31.8k ±0.5k (+4%)
null:
read:
seq aio: 253.8k ±12.3k (+45%)
rand aio: 248.2k ±12.0k (+45%)
seq sync: 91.6k ±2.4k (+12%)
rand sync: 91.3k ±2.1k (+17%)
write:
seq aio: 208.2k ±9.8k (+6%)
rand aio: 207.0k ±7.4k (+8%)
seq sync: 91.2k ±1.9k (+9%)
rand sync: 90.4k ±2.5k (+14%)
So moderate improvements in most cases, but quite improved AIO
performance with an actual underlying file.
Here's results for numjobs=4:
"Before", i.e. without multithreading in QSD/FUSE (results compared to
numjobs=1):
file:
read:
seq aio: 85.5k ±0.4k (-13%)
rand aio: 92.5k ±0.5k (-3%)
seq sync: 54.5k ±9.1k (+58%)
rand sync: 38.0k ±0.2k (+283%)
write:
seq aio: 67.3k ±0.3k (-2%)
rand aio: 67.6k ±0.3k (-2%)
seq sync: 69.3k ±0.5k (+126%)
rand sync: 69.3k ±0.3k (+126%)
null:
read:
seq aio: 170.6k ±0.8k (-2%)
rand aio: 170.9k ±0.9k (±0%)
seq sync: 187.6k ±1.3k (+129%)
rand sync: 188.9k ±0.9k (+142%)
write:
seq aio: 191.5k ±1.2k (-2%)
rand aio: 193.8k ±1.4k (-1%)
seq sync: 206.1k ±1.3k (+147%)
rand sync: 206.1k ±1.2k (+159%)
As probably expected, little difference in the AIO case, but great
improvements in the sync cases because it kind of gives it an artificial
iodepth of 4.
"After", i.e. with four threads in QSD/FUSE (now results compared to the
above):
file:
read:
seq aio: 198.7k ±2.7k (+132%)
rand aio: 317.3k ±0.6k (+243%)
seq sync: 55.9k ±8.9k (+3%)
rand sync: 39.1k ±0.0k (+3%)
write:
seq aio: 229.0k ±0.8k (+240%)
rand aio: 227.0k ±1.3k (+235%)
seq sync: 102.5k ±0.2k (+48%)
rand sync: 101.7k ±0.2k (+47%)
null:
read:
seq aio: 584.0k ±1.5k (+242%)
rand aio: 581.9k ±1.9k (+240%)
seq sync: 270.6k ±0.9k (+44%)
rand sync: 270.4k ±0.7k (+43%)
write:
seq aio: 598.4k ±2.0k (+212%)
rand aio: 605.2k ±2.0k (+212%)
seq sync: 274.0k ±0.8k (+33%)
rand sync: 275.0k ±0.7k (+33%)
So this helps mainly for the AIO cases, but also in the null sync cases,
because null is always CPU-bound, so more threads help.
One unsolved mystery: When using a multithreaded export, running fio
with 1 job (benchmark at the top of this commit) yields better seqread
performance than doing so with 4 jobs. Actually, with 4 jobs, it's
significantly than randread, which is quite strange.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-24-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The next commit is going to allow multi-threaded access to a FUSE
export. In order to allow safe concurrent SETATTR operations that
can modify the shared st_mode, st_uid, and st_gid, make any access to
those fields atomic operations.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-23-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Make BlockExportType.iothread an alternate between a single-thread
variant 'str' and a multi-threading variant '[str]'.
In contrast to the single-thread setting, the multi-threading setting
will not change the BDS's context (and so is incompatible with the
fixed-iothread setting), but instead just pass a list to the export
driver, with which it can do whatever it wants.
Currently no export driver supports multi-threading, so they all return
an error when receiving such a list.
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-21-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Make fuse_process_request() a coroutine_fn (fuse_co_process_request())
and have read_from_fuse_fd() launch it inside of a newly created
coroutine instead of running it synchronously. This way, we can process
requests in parallel.
These are the benchmark results, compared to (a) the original results
with libfuse, and (b) the results after switching away from libfuse
(i.e. before this patch):
file: (vs. libfuse / vs. no libfuse)
read:
seq aio: 97.8k ±1.5k (-2% / -8%)
rand aio: 95.8k ±3.4k (+90% / +98%)
seq sync: 34.5k ±1.0k (-4% / -3%)
rand sync: 9.9k ±0.1k (-1% / -1%)
write:
seq aio: 68.7k ±1.3k (-5% / -10%)
rand aio: 68.9k ±1.1k (-2% / -10%)
seq sync: 30.6k ±0.9k (±0% / -3%)
rand sync: 30.6k ±0.6k (+1% / -1%)
null:
read:
seq aio: 174.5k ±6.8k (+11% / +8%)
rand aio: 170.9k ±5.7k (+8% / +3%)
seq sync: 82.0k ±3.3k (+2% / +2%)
rand sync: 78.0k ±4.0k (+1% / -1%)
write:
seq aio: 196.0k ±2.8k (+27% / +6%)
rand aio: 191.2k ±7.9k (+24% / +2%)
seq sync: 83.3k ±4.4k (+9% / +1%)
rand sync: 79.5k ±4.4k (+9% / +1%)
So there is not much difference, especially when compared to how it was
with libfuse, except for the randread AIO case with an actual file.
That improves greatly.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-20-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
We are going to introduce parallel processing via coroutines, a maximum
read size of 64 MB may be problematic, allowing users of the export to
force us to allocate quite large amounts of memory with just a few
requests.
At least tone it down to 1 MB, which is still probably far more than
enough. (Larger requests are split automatically by the FUSE kernel
driver anyway.)
(Yes, we inadvertently already had parallel request processing due to
nested polling before. Better to fix this late than never.)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-19-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Manually read requests from the /dev/fuse FD and process them, without
using libfuse. This allows us to safely add parallel request processing
in coroutines later, without having to worry about libfuse internals.
(Technically, we already have exactly that problem with
read_from_fuse_export()/read_from_fuse_fd() nesting.)
We will continue to use libfuse for mounting the filesystem; fusermount3
is a effectively a helper program of libfuse, so it should know best how
to interact with it. (Doing it manually without libfuse, while doable,
is a bit of a pain, and it is not clear to me how stable the "protocol"
actually is.)
Take this opportunity of quite a major rewrite to update the Copyright
line with corrected information that has surfaced in the meantime.
Here are some benchmarks from before this patch (4k, iodepth=16, libaio;
except 'sync', which are iodepth=1 and pvsync2):
file:
read:
seq aio: 99.8k ±1.5k IOPS
rand aio: 50.5k ±1.0k
seq sync: 36.1k ±1.1k
rand sync: 10.0k ±0.1k
write:
seq aio: 72.0k ±9.3k
rand aio: 70.6k ±2.5k
seq sync: 30.6k ±0.8k
rand sync: 30.1k ±1.0k
null:
read:
seq aio: 157.9k ±4.7k
rand aio: 158.7k ±4.8k
seq sync: 80.2k ±2.8k
rand sync: 77.5k ±3.8k
write:
seq aio: 154.3k ±3.6k
rand aio: 154.3k ±4.2k
seq sync: 76.1k ±5.2k
rand sync: 72.9k ±4.0k
And with this patch applied:
file:
read:
seq aio: 106.8k ±1.9k (+7%)
rand aio: 48.3k ±8.8k (-4%)
seq sync: 35.5k ±1.4k (-2%)
rand sync: 10.0k ±0.2k (±0%)
write:
seq aio: 76.3k ±6.6k (+6%)
rand aio: 76.4k ±1.5k (+8%)
seq sync: 31.6k ±0.6k (+3%)
rand sync: 30.9k ±0.8k (+3%)
null:
read:
seq aio: 161.7k ±6.0k (+2%)
rand aio: 165.6k ±7.1k (+4%)
seq sync: 80.5k ±3.0k (±0%)
rand sync: 78.5k ±3.1k (+1%)
write:
seq aio: 185.1k ±3.3k (+20%)
rand aio: 186.7k ±4.8k (+21%)
seq sync: 82.5k ±4.2k (+8%)
rand sync: 78.7k ±3.2k (+8%)
So not much difference, aside from write AIO to a null-co export getting
a bit better.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-18-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
When reading to / writing from non-growable exports, we cap the I/O size
by `offset - blk_len`. This will underflow for accesses that are
completely past the disk end.
Check and handle that case explicitly.
This is also enough to ensure that `offset + size` will not overflow;
blk_len is int64_t, offset is uint32_t, `offset < blk_len`, so from
`INT64_MAX + UINT32_MAX < UINT64_MAX` it follows that `offset + size`
cannot overflow.
Just one catch: We have to allow write accesses to growable exports past
the EOF, so then we cannot rely on `offset < blk_len`, but have to
verify explicitly that `offset + size` does not overflow.
The negative consequences of not having this commit are luckily limited
because blk_pread() and blk_pwrite() will reject post-EOF requests
anyway, so a `size` underflow post-EOF will just result in an I/O error.
So:
- Post-EOF reads will incorrectly result in I/O errors instead of just
0-length reads. We will also attempt to allocate a very large buffer,
which is wrong and not good, but not terrible.
- Post-EOF writes on non-growable exports will result in I/O errors
instead of 0-length writes (which generally indicate ENOSPC).
- Post-EOF writes on growable exports can theoretically overflow on EOF
and truncate the export down to a much too small size, but in
practice, FUSE will never send an offset greater than signed INT_MAX,
preventing a uint64_t overflow. (fuse_write_args_fill() in the kernel
uses loff_t for the offset, which is signed.)
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-15-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This is a flag that we will want when processing FUSE requests
ourselves: When the kernel sends us e.g. a truncated request (i.e. we
receive less data than the request's indicated length), we cannot rely
on subsequent data to be valid. Then, we are going to set this flag,
halting all FUSE request processing.
We plan to only use this flag in cases that would effectively be kernel
bugs.
While not necessary yet, access the flag atomically so that it will be
safe to use once we introduce multi-threading.
(Right now, the flag is unused because libfuse still does our request
processing.)
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-12-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
In fuse_open(), set these flags:
- direct_io: We probably actually don't want to have the host page cache
be used for our exports. QEMU block exports are supposed to represent
the image as-is (and thus potentially changing).
This causes a change in iotest 308's reference output.
- parallel_direct_writes: We can (now) cope with parallel writes, so we
should set this flag. For some reason, it doesn't seem to make an
actual performance difference with libfuse, but it does make a
difference without it, so let's set it.
(See "fuse: Copy write buffer content before polling" for further
discussion.)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-9-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Since I actually took a look into how mounting with libfuse works[1], I
now know that the FUSE mount options are not exactly standard mount
system call options. Specifically:
- We should add "nosuid,nodev,noatime" because that is going to be
translated into the respective MS_ mount flags; and those flags make
sense for us.
- We can set rw/ro to make the mount writable or not. It makes sense to
set this flag to produce a better error message for read-only exports
(EROFS instead of EACCES).
This changes behavior as can be seen in iotest 308: It is no longer
possible to modify metadata of read-only exports.
Similarly, in fuse-allow-other, we must now make the export writable
to use SETATTR.
In addition, in the comment, we can note that the FUSE mount() system
call actually expects some more parameters that we can omit because
fusermount3 (i.e. libfuse) will figure them out by itself:
- fd: /dev/fuse fd
- rootmode: Inode mode of the root node
- user_id/group_id: Mounter's UID/GID
[1] It invokes fusermount3, an SUID libfuse helper program, which parses
and processes some mount options before actually invoking the
mount() system call.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-8-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
If mount_fuse_export() fails to mount the session, destroy it.
Depending on the allow_other configuration, fuse_export_create() may
retry this function on error, which may leak one session instance
otherwise.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-ID: <20260309150856.26800-7-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
