Support qcow backing file #5573
Merged
Conversation
This file contains 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
yukiiiteru
commented
Jul 6, 2023
|
|
||
| [features] | ||
| default = [] | ||
| authors = ["The Cloud Hypervisor Authors", "The Chromium OS Authors"] |
There was a problem hiding this comment.
I don't know if there is any problem with updating this here 🤔
yukiiiteru
force-pushed
the
feature/qcow-backing-file
branch
3 times, most recently
from
589a706
to
9e3b671
Compare
|
I made too many low-level mistakes on file names in integration tests 😢 |
yukiiiteru
force-pushed
the
feature/qcow-backing-file
branch
2 times, most recently
from
69b36ae
to
ee259be
Compare
|
It seems that the integration tests behave differently in different arches 😢 Tests on x86_64 always download these images, maybe these images are deleted somewhere, but tests on aarch64 don't download images because they exist on the machine. Is there any way to delete them on the test machine, e.g., webshell? Or how about just deleting them at the beginning? The following is the log during the test: aarch64x86_64 |
Indeed, the disk images are handled in different ways. x86_64 test runs in VM. A new VM instance is created for an integration. When the test starts, there isn't any image in the VM. So the images are always downloaded freshly. aarch64 test runs in a baremetal server. We don't remove the images every time because they can be reused. I removed the old |
Got it, thanks! |
yukiiiteru
force-pushed
the
feature/qcow-backing-file
branch
5 times, most recently
from
931ca4c
to
8f2d5fb
Compare
|
My code conflicts with the "main" branch, I rebased them. And there are some new warnings with the latest beta rust toolchain, the latest 3 commits fixed them. |
yukiiiteru
force-pushed
the
feature/qcow-backing-file
branch
2 times, most recently
from
7183268
to
196ac18
Compare
|
There are too much clippy warnings in beta toolchain 😢 I'm sorry for using too many CI resources. |
likebreath
reviewed
Jul 12, 2023
Sure, a new PR #5590 has been created. This PR will not be updated for now, I will rebase it after that one is merged. |
|
@wfly1998 Please rebase to drop your clippy changes |
rbradford
reviewed
Jul 17, 2023
This commit introduces a generic type `FixedVhd` for its sync and async disk I/O types, and the disk I/O types will use it directly instead of maintaining a file themselves. Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
This commit merges crates `qcow`, `vhdx` and `block_util` into the crate `block`, which can allow `qcow` to use functions from `block_util` without introducing a circular crate dependency. This commit is based on crosvm implementation: https://chromium.googlesource.com/crosvm/crosvm/+/f2eecc4152eca8d395566cffa2c102ec090a152d Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
This commit introduces the trait `BlockBackend` with generic ops including read, write and seek, which can be used for common I/O interfaces for the block types without using `DiskFile` and `AsyncIo`. Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
Fixes: 3f02cca Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
This commit allows opening qcow with a backing file, which supports any type implementing `BlockBackend`. This commit is based on crosvm implementation: https://chromium.googlesource.com/crosvm/crosvm/+/9ca6039b030a5c83062cfec9a5ff52f42814fa13 Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
Reads to qcow files with backing files will fall through to the backing file if there is no allocated cluster. As of this change, a write will still trash the cluster and hide any data already present. This commit is based on crosvm implementation: https://chromium.googlesource.com/crosvm/crosvm/+/d8144a56e26ca09e2c7ff97ed63c57e7e7965674 Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
This preserves any data that the backing file had on a cluster when doing a write to a subset of that cluster. These writes cause a performance penalty on creating new clusters if a backing file is present. This commit is based on crosvm implementation: https://chromium.googlesource.com/crosvm/crosvm/+/5ad3bc345904b252efd6dd2ef4853f5ee06ae3c5 Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
This test case creates a new qcow2 file using the image of ubuntu as its backing file, and boot a virtual machine with this image file. Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
yukiiiteru
force-pushed
the
feature/qcow-backing-file
branch
from
196ac18
to
dfb1e09
Compare
|
@rbradford I have done the rebase, but one of the integration tests is failing and I don't know how to fix it |
It was a flake. |
rbradford
approved these changes
Jul 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently, the qcow format does not support backing file. Noticed that the qcow implementation comes from CrosVM and the CrosVM supports the qcow support files now, so I backported it in this PR.
This PR does the following:
qcow,vhdxandblock_utilintoblockcrateFixedVhdtype forBlockBackendBlockBackendfor generic ops (e.g., read, write and seek), and the backing file can be any type which implements this traitAnd I also fixed a bug when testing the
QcowHeaderwith backing file name 😂 some values that don't belong to v2 are written into the v2 header but it, and these values are never read. So the previous unit tests always pass.