block: advisory locks: use byte-range locks to match QEMU behavior#26
Conversation
phip1611
force-pushed
the
poc-disk-locking-granularity
branch
from
97538ee to
6a32e1c
Compare
|
Is it possible to have a test for this one? Checking that read access to the disk image is possible while a CHV VM is using it? |
phip1611
force-pushed
the
poc-disk-locking-granularity
branch
from
6a32e1c to
fb7d521
Compare
Not really. I just tried a few things to test this but even in a NFS environment, whole-file locks are just advisory locks. It must be some management policy of OpenStack cinder to treat whole-file advisory locks as mandatory lock. |
phip1611
force-pushed
the
poc-disk-locking-granularity
branch
from
fb7d521 to
e5420d2
Compare
virtio-devices/src/block.rs
Outdated
| .disk_image | ||
| .size() | ||
| .map(|size| LockGranularity::ByteRange(0, size)) | ||
| .unwrap_or(LockGranularity::WholeFile); |
There was a problem hiding this comment.
Just so I understand everything correctly:
This will basically always result in a byte sized lock?
Even if we do not make it configurable for the user right now, would it make sense to pass the lock granularity more explicitly?
There was a problem hiding this comment.
This will basically always result in a byte sized lock?
exactly except for if .size() returns Err which I think should almost never happen. From the commit message:
Whole-file locks remain a valid use case and could be supported later.
This patch only provides the necessary groundwork; making it configurable
is out of scope for now.
|
Looks good! Hope that fixes our current problems! |
The granularity has significant implications in typical cloud deployments with network storage. The Linux kernel will sync advisory locks to network file systems, but these backends may have different policies and handle locks differently. For example, Netapp speaks a NFS API but will treat advisory OFD locks for the whole file as mandatory locks, whereas byte-range locks for the whole file will remain advisory [0]. As it is a valid use case to prevent multiple CHV instances from accessing the same disk but disk management software (e.g., Cinder in OpenStack) should be able to snapshot disks while VMs are running, we need special control over the lock granularity. Therefore, it is a valid use case to lock the whole byte range of a disk image without technically locking the whole file - to get the best of both worlds. This also brings CHVs behavior in line with QEMU [1]. Whole-file locks remain a valid use case and could be supported later. This patch only provides the necessary groundwork; making it configurable is out of scope for now. [0] https://kb.netapp.com/on-prem/ontap/da/NAS/NAS-KBs/How_is_Mandatory_Locking_supported_for_NFSv4_on_ONTAP_9 [1] <qemu>/util/osdep.c::qemu_lock_fcntl() Signed-off-by: Philipp Schuster <philipp.schuster@cyberus-technology.de> On-behalf-of: SAP philipp.schuster@sap.com
phip1611
force-pushed
the
poc-disk-locking-granularity
branch
from
e5420d2 to
ddc56ed
Compare
|
@phip1611 thanks for the quick fix. looks good. |
tpressure
merged commit 7163bfe
into
cyberus-technology:gardenlinux
3 participants
See commit message for context.
Ticket: https://github.com/cobaltcore-dev/cobaltcore/issues/289