virtio-devices, block: generalize disk locking - #191
Open
Coffeeri wants to merge 3 commits into
Open
Conversation
Calculate advisory lock granularity for an explicitly supplied disk backend and path instead of always using Block::disk_image. This lets mirror destinations reuse the configured locking policy. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de>
Extract advisory lock acquisition into a helper that accepts a disk backend, path, requested mode, and current mode. Keep try_lock_image as the source-disk wrapper. This enables the generalized use of `try_lock_disk_image` for other disk images within the same Block device, which occurs whenever we mirror a disk image. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de>
Downgrading a QEMU-compatible lock from write to read leaves the write marker byte locked. try_acquire_lock_qemu() only releases unneeded bytes when rolling back after a failed acquisition, so a successful downgrade keeps the write marker in place. This blocks mirroring of read-only disks. The destination acquires a write lock while data is copied and must downgrade to the source's read-only lock when the mirror completes. The stale write marker prevents another reader from locking the image. Release the bytes that the new lock type does not need once the conflict checks succeed, and add a test that downgrades a write lock to a read lock and verifies that another reader can still acquire the image. On-behalf-of: SAP leander.kohler@sap.com Signed-off-by: Leander Kohler <leander.kohler@cyberus-technology.de>
Coffeeri
force-pushed
the
generalize-lock
branch
from
August 12, 2026 13:52
276e1d8 to
383d0dd
Compare
|
I'm not sure breaking open the API is a maintenance-friendly way to achieve the goal. On the other hand, we need to rethink parts of the disk locking anyway to address https://github.com/cobaltcore-dev/cobaltcore/issues/628. To unblock the blockdev-mirror, I think it's fair to continue with the PR on the premise that we will have to refactor disk locking anyway and can take the blockdev-mirror use case into account. |
Comment on lines
+1016
to
+1017
| /// Acquires an advisory lock for an arbitrary disk backend. | ||
| fn try_lock_disk_image( |
There was a problem hiding this comment.
The comment should retain the notion of trying to acquire the lock, since locking can fail.
Comment on lines
+982
to
+987
| /// Returns the configured advisory lock granularity for `disk_image`. | ||
| fn lock_granularity( | ||
| &self, | ||
| disk_image: &dyn AsyncFullDiskFile, | ||
| disk_path: &Path, | ||
| ) -> LockGranularity { |
There was a problem hiding this comment.
The comment should include, that the lock granularity is influenced by the choice made in block.
| return Err(error); | ||
| } | ||
|
|
||
| self.release_unneeded_locks_qemu(file, lock_type)?; |
There was a problem hiding this comment.
Could you add a comment why we're doing this, similar to the commit message?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These changes support blockdev-mirror.
Blockdev-Mirroring keeps both the source and destination disk open. The existing locking helpers always operated on
Block::disk_image, so they could not be reused for the destination.This PR generalizes lock granularity and acquisition to accept an explicit disk backend and path, while keeping the existing source-disk wrapper.
It also fixes stale QEMU-compatible locks after a successful downgrade. Read-only sources use a write lock for the destination while mirroring, then downgrade it to a read lock after completion. Previously, the write marker remained locked after the successful downgrade, preventing another reader from acquiring the image. The unused lock bytes are now released.