vmm: config: Expose disk lock granularity option#7763
vmm: config: Expose disk lock granularity option#7763rbradford merged 2 commits intocloud-hypervisor:mainfrom
Conversation
phip1611
left a comment
phip1611
left a comment
There was a problem hiding this comment.
Thanks for working on this! Left some remarks
rbradford
left a comment
rbradford
left a comment
There was a problem hiding this comment.
What's OFD? And I don't see any documentation updates?
The locking mechanism I introduced in #6974 is based on Open File-Description locks. UNIX and POSIX file locks are fundamentally broken; OFD is the modern API of Linux |
|
@rbradford For documentation, would an update to docs/device_model.md in the virtio-block section be sufficient, or would you also like a release-notes entry? |
My mistake - I though there was a document for the |
rbradford
left a comment
rbradford
left a comment
There was a problem hiding this comment.
Maybe add a docs/disk_locking.md to provide details. I don't think the OpenAPI yaml file is the correct place for such documentation.
And is there no way we can detect this and just do the right thing rather than have to have it be an option?
|
Will add Regarding auto-detection, the default (byte-range) already does the right thing in most cases. The issue is that some NFS server implementations (e.g. NetApp) treat whole-file OFD locks as mandatory, but there's no reliable way to detect this from the client side I believe. |
vieux
force-pushed
the
disk-lock-granularity-config
branch
from
6f75bd2 to
75ef46f
Compare
vieux
force-pushed
the
disk-lock-granularity-config
branch
from
75ef46f to
d164f8d
Compare
|
Almost there! Thanks for working on that |
vieux
force-pushed
the
disk-lock-granularity-config
branch
from
d164f8d to
a2669ad
Compare
|
@phip1611 Sounds good, used your exact suggestions :) |
|
(I don't think I can re-trigger CI on my side, seems like a temporary failure) |
vieux
force-pushed
the
disk-lock-granularity-config
branch
2 times, most recently
from
159a947 to
00f62dd
Compare
|
Please fix CI! You can also locally run |
|
@phip1611 as I mentioned in the thread I'm waiting in @rbradford to see if they're happy with the new approach, I kept it as a separate commit for ease of review. If it's 👍 I'll squash to be mergeable. Does that work for you? |
Add a per-disk lock_granularity parameter that lets users choose between byte-range OFD locks and whole-file OFD locks: --disk path=/foo.img,lock_granularity=byte-range --disk path=/bar.img,lock_granularity=full Byte-range is the default and matches QEMU behavior, working best with storage backends where whole-file OFD locks are treated as mandatory. The full option restores the original whole-file locking for environments that depend on it. The LockGranularityChoice enum and its FromStr impl live in the block crate alongside the existing LockGranularity type. The Block device converts the user-facing choice to the internal LockGranularity at lock time, keeping device_manager.rs simple. Closes: cloud-hypervisor#7553 Signed-off-by: Victor Vieux <vieux@repl.it>
Add docs/disk_locking.md explaining advisory OFD locking, the lock_granularity parameter, byte-range vs whole-file semantics, and fallback behavior. Signed-off-by: Victor Vieux <vieux@repl.it>
vieux
force-pushed
the
disk-lock-granularity-config
branch
from
00f62dd to
d1ad2e2
Compare
3 participants
Add a per-disk lock_granularity parameter that lets users choose between byte-range OFD locks and whole-file OFD locks:
--disk path=/foo.img,lock_granularity=byte-range
--disk path=/bar.img,lock_granularity=full
Byte-range is the default and matches QEMU behavior, working best with storage backends like NetApp where whole-file OFD locks are treated as mandatory. The full option restores the original whole-file locking for environments that depend on it.
This plumbs the LockGranularity enum introduced in PR #7494 through the config layer (CLI, OpenAPI, DiskConfig) down to virtio-devices.
Closes: #7553