QEMU compatible file locking by arctic-alpaca · Pull Request #180 · cyberus-technology/cloud-hypervisor

arctic-alpaca · 2026-07-06T12:17:58Z

Introduces QEMU compatible file locking.
Please see the commit messages for additional information.

References:

QEMU locking: https://github.com/qemu/qemu/blob/master/block/file-posix.c#L868-L940
QEMU lock checking: https://github.com/qemu/qemu/blob/master/block/file-posix.c#L942-L982
QEMU lock offsets: https://github.com/qemu/qemu/blob/master/block/file-posix.c#L131-L134
QEMU lock permission bits: https://github.com/qemu/qemu/blob/master/include/block/block-common.h#L392-L437

Testing was done on customer infra and the fix is confirmed to fix the reported issues.

The default is set to QEMU compatible file locking to avoid the need for changes in libvirt. We can revert that at a later point and use libvirt to set the appropriate config. In the interest of speed, I favor setting the default for now.
The BLK_PERM_RESIZE permission will be added as a follow-up as to not delay the fix since testing showed the current implementation to be sufficient for the moment (https://github.com/cobaltcore-dev/cobaltcore/issues/627).

Fixes https://github.com/cobaltcore-dev/cobaltcore/issues/595

Libvirt Pipeline: https://gitlab.cyberus-technology.de/cyberus/cloud/libvirt/-/merge_requests/253

phip1611

I know it is still in draft. I reviewed because my vacation starts in ~90min. I think this is looking good and is going into the right direction!

Thanks for the fix and the nice commit history!

Coffeeri

Amazing PR, Julian! Also love the easy-to-follow refactoring commits.

The function suffers from a TOCTOU problem. There is no guarantee that the locks set on the file at the time of the state check are the same as at the time of the locking attempt. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

Rust typically doesn't use `get_` prefixes. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

All functions take a `LockGranularity` already and making them part of `LockGranularity` makes it easier to add associated helpers. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

By extracting the full file and file range locking, it's easier to add file lock handling that's not following this pattern. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

Avoids repeating the retry/error logic when adding new users of the function. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

Reference to pointer coercion doesn't happen for variadic functions like `libc::fcntl`. While this doesn't pose a problem by itself, it removes the check whether a reference can be coerced to the appropriate pointer type or mutability. By explicitly casting to a pointer, we ensure refactors don't accidentally break the safety requirements. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

Coffeeri · 2026-07-09T07:27:13Z

+            LockType::Read => vec![
+                LockGranularity::QemuCompatible
+                    .flock(libc::F_WRLCK, QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE),
+                LockGranularity::QemuCompatible.flock(
+                    libc::F_WRLCK,
+                    QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE,
+                ),
+            ],


Suggested change

LockType::Read => vec![

LockGranularity::QemuCompatible

.flock(libc::F_WRLCK, QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE),

LockGranularity::QemuCompatible.flock(

libc::F_WRLCK,

QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE,

),

],

LockType::Read => vec![

LockGranularity::QemuCompatible

.flock(libc::F_WRLCK, QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE),

LockGranularity::QemuCompatible.flock(

libc::F_WRLCK,

QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE,

),

],

You mean to check the write permission additionally to checking the unshared write permission?

I checked again with my AI friend:

TLDR: We should replace it, not add.

We got in the Read arm the two probes

byte 200 (QEMU_UNSHARE_LOCK_OFFSET + QEMU_CONSISTENT_READ_BYTE)

byte 201 (QEMU_UNSHARE_LOCK_OFFSET + QEMU_WRITE_BYTE)

QEMU does two things in raw_check_lock_bytes:

For every permission it uses, it probes byte 200 + k: "does anyone forbid this permission?" [0].
A reader uses consistent-read, so it probes byte 200. This matches our first probe.

For every permission it does not share, it probes byte 100 + k: "does anyone use this permission?" [1].
A reader does not share write, so it probes byte 101 (QEMU_LOCK_OFFSET + QEMU_WRITE_BYTE). Our second probe targets byte 201 instead.

Byte 201 answers a different question: "does anyone forbid write?". A reader never asks for write permission itself, so someone else forbidding write is no conflict for it. Worse, every other reader sets the flag on byte 201, because it also forbids write. Our F_WRLCK probe collides with that flag, so two read-only VMs could not use the same image. QEMU allows that.

Probing byte 101 instead still detects every writer, because every writer sets the flag on byte 101, whether it shares write or not.

[0] https://github.com/qemu/qemu/blob/c3a63b7c06ab3dae8cbe191e7cae9095022d37d6/block/file-posix.c#L949-L964
[1] https://github.com/qemu/qemu/blob/c3a63b7c06ab3dae8cbe191e7cae9095022d37d6/block/file-posix.c#L965-L980

Right, should be fixed now 👍

QEMU uses locks on specific bytes for modeling file lock permissions. This commit only implements what's needed to model the permissions cloud-hypervisor currently uses. This is needed to be compatible with storage management systems that expect QEMU compatible file locking like NetApp's NFS implementation. QEMU uses two offsets from the start of the file to separate permissions and "unshared" permissions. "unshared" permissions are permissions that cannot be shared between lock holders [0]. Starting from each offset, locks are placed with a length of one byte. Each corresponds to a specific permission [1][2]. The locking is done by first signaling intent by locking the required marker bytes. Next any conflicts with existing locks are detected and in the failure case, the locks are rolled back. Locking is done via `F_RDLCK`, which allows other parties to set the same locks and check for conflicts with `F_WRLCK`. [0]: https://github.com/qemu/qemu/blob/30e8a06b64aa58a3990ba39cb5d09531e7d265e0/block/file-posix.c#L131-L134 [1]: https://github.com/qemu/qemu/blob/30e8a06b64aa58a3990ba39cb5d09531e7d265e0/include/block/block-common.h#L392-L437 [2]: https://github.com/qemu/qemu/blob/30e8a06b64aa58a3990ba39cb5d09531e7d265e0/block/file-posix.c#L868-L940 On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

arctic-alpaca force-pushed the qemu-compatible-locking branch 2 times, most recently from f3b11b1 to 6763d5e Compare July 6, 2026 12:28

phip1611 reviewed Jul 7, 2026

View reviewed changes

Comment thread block/src/fcntl.rs

Comment thread block/src/fcntl.rs Outdated

Comment thread block/src/fcntl.rs

arctic-alpaca force-pushed the qemu-compatible-locking branch 2 times, most recently from 4925935 to dac9921 Compare July 7, 2026 10:37

arctic-alpaca marked this pull request as ready for review July 7, 2026 11:29

arctic-alpaca requested review from Coffeeri and amphi July 7, 2026 11:29

Coffeeri requested changes Jul 8, 2026

View reviewed changes

Comment thread block/src/fcntl.rs

Comment thread virtio-devices/src/block.rs Outdated

Comment thread block/src/fcntl.rs

Comment thread block/src/fcntl.rs Outdated

Comment thread block/src/fcntl.rs Outdated

arctic-alpaca force-pushed the qemu-compatible-locking branch from dac9921 to 3a963d2 Compare July 8, 2026 14:28

arctic-alpaca added 6 commits July 8, 2026 16:37

vmm: remove get_ prefix

8708418

Rust typically doesn't use `get_` prefixes. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

vmm: move error handling into fcntl function

e2541e9

Avoids repeating the retry/error logic when adding new users of the function. On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

arctic-alpaca force-pushed the qemu-compatible-locking branch from 3a963d2 to 0ccd243 Compare July 8, 2026 14:38

Coffeeri reviewed Jul 9, 2026

View reviewed changes

arctic-alpaca added 2 commits July 9, 2026 15:14

vmm: set QemuCompatible to the default LockGranularityChoice

b0d5ab2

On-behalf-of: SAP julian.schindel@sap.com Signed-off-by: Julian Schindel <julian.schindel@cyberus-technology.de>

arctic-alpaca force-pushed the qemu-compatible-locking branch from 0ccd243 to b0d5ab2 Compare July 9, 2026 13:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

QEMU compatible file locking#180

QEMU compatible file locking#180
arctic-alpaca wants to merge 8 commits into
cyberus-technology:gardenlinuxfrom
arctic-alpaca:qemu-compatible-locking

arctic-alpaca commented Jul 6, 2026 •

edited

Loading

Uh oh!

phip1611 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Coffeeri left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Coffeeri Jul 9, 2026

Uh oh!

arctic-alpaca Jul 9, 2026

Uh oh!

Coffeeri Jul 9, 2026

Uh oh!

arctic-alpaca Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

arctic-alpaca commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phip1611 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Coffeeri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Coffeeri Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

arctic-alpaca Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Coffeeri Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

arctic-alpaca Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arctic-alpaca commented Jul 6, 2026 •

edited

Loading