To prepare for Rust toolchain uprev, ran `tools/clippy --fix` with
Rust 1.88 for each platform.
To build with the current Rust version 1.81, some changes with unstable
features were manually reverted.
BUG=b:455879436
TEST=CQ
Change-Id: I4a91460d4fc2de5d7afbc2da04b7f7765219bb2e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/7089630
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Only allocate errors if they are hit as opposed to always allocating the
error.
BUG=b:440153729
TEST=presubmit
Change-Id: I3aa902e4925cd73c02e9b08765fdaf72340b6aa7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6886671
Reviewed-by: Frederick Mayle <fmayle@google.com>
Auto-Submit: Elie Kheirallah <khei@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
We don't track "needs fsync" vs "needs fdatasync" because, as mentioned
in the original fdatasync commit (https://crrev.com/c/4523029), we don't
care about the file metadata in our use cases.
BUG=b:281609112
Change-Id: Icff7254d92e5d202557cba8bcce44dc360fb5e07
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6306252
Reviewed-by: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
There was some variation before, with `anyhow = "1"` as the most common
target.
```
$ rg 'anyhow =' --no-filename --no-line-number | sort | uniq -c
2 anyhow = "*"
31 anyhow = "1"
2 anyhow = "1.0"
7 anyhow = "1.0.32"
1 anyhow = "1.0.57"
1 anyhow = "1.0.75"
```
With this change, everything has been aligned to the most common target.
```
$ rg 'anyhow =' --no-filename --no-line-number | sort | uniq -c
44 anyhow = "1"
```
(Specifically the `anyhow = "*"` lines were giving me trouble with
b/394932208 and cl/723721713, I can change this to standardize on
something else if desired)
Bug: b/394932208
Change-Id: Iea4fa3d020fe5646aa9f444de50645195a1b9778
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6238753
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Replace open-coded reimplementations of these rounding-related math
functions where applicable (mostly identified as candidates for
div_ceil() by Rust 1.84 clippy, but in some cases, next_multiple_of()
was a better match).
Change-Id: Ib851689836cb8d44cde5a5766a3cec8fd5c90f40
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6195237
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
The secondary GPT header should be at the end of the image, but its
offset was chosen before the disk size was aligned to 64k. This caused
Linux to print warnings at boot, like
GPT:Primary header thinks Alt. header is not at the end of the disk.
GPT:105736 != 105855
GPT:Alternate GPT header not at the end of the disk.
GPT:105736 != 105855
GPT: Use GNU Parted to correct GPT errors.
It is likely there was no practical issue besides the log noise.
As part of the fix, we no longer align the disk size to 64k. It is
unclear why we aligned to 64k to begin with and maintaining that
alignment made the appropriate fix more complicated.
Change-Id: I2f41a2033f14d34e2fd3dd3dd3afe2da80266196
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6036619
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Many reads issued by kernel is smaller than 128KB (our default frame
size). When doing sequential reads with 4K size, the current
implementation can lead to up to 32x decompression overhead due to
decompressing the same frame consecutively read again and again.
This single frame cache solves this issue.
On my very fast workstation, 4k read speed 453->829MB/s, 128k read
speed 618->838MB/s. For reference, 1 thread zstd decompress on host
of the same file is 1292MB/s. I expect much larger improvement on
CPU-bounded devices (e.g. many chromebooks).
TESTED=observed correct result when run `ls -lR` on mounted zstd disk
TESTED=`sha256sum` of files are correct
Change-Id: I9a3be6c34d3256bf5e222e3375e70e9b9dda761a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6036328
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Zihan Chen <zihanchen@google.com>
A raw disk image can be compressed as a seekable zstd and attached
transaprently to a VM as a block device.
TESTED=can ro mount and read seekable compressed debian rootfs
BUG=b:377945783
Change-Id: Iba1950dbfc0ba99b0581e842964848d5a447b824
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6024317
Commit-Queue: Zihan Chen <zihanchen@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Zihan Chen <zihanchen@google.com>
The previous implementation of QcowRawFile::read_pointer_table() called
read_exact() on the underlying File for each individual entry in the
table; for a typical 64KB cluster size, this would mean read() would be
called 8192 times per cluster. Similarly, read_refcount_block() would
issue 32768 individual read() calls per cluster.
Using zerocopy, we can read the whole Vec<u64> or Vec<u16> reinterpreted
as a byte slice in a single read() call, then perform endian translation
and masking in a separate loop, which should be significantly more
efficient.
Change-Id: Ia66f4b9363be63935388adaa185a31b515f47951
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5977144
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
This makes the behavior closer to Linux (which uses flock).
Change-Id: Iff587b58647eec7378972e5442d4da95f3e2b7a8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5841071
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
The recent changes to make crosvm lock nested disk files is running
afoul of Android Virtualization Framework's SELinux policies. The file
locking doesn't add much value in that environement because all the
shared files are read-only, so add an opt-out for locking to buy us time
to re-engineer the policies.
BUG=b:330911976
Change-Id: I0b35732978e946a2331507d6061729d53955a8d3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5849284
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
In practice this probably only matters for the flock calls because, when
a disk is read-only, the block device should never issue write calls to
it to begin with.
BUG=b:330911976
Change-Id: I9fc4e8e09e9c5a51f53ec342fae1c69c984999c0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5838023
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
The disk crate is now responsible for opening the file and handling
options like OVERLAPPED and O_DIRECT.
O_DIRECT is now applied to all nested disk files. For now, OVERLAPPED
contiues to only apply to the root disk file.
BUG=b:330911976
BUG=b:190435784
Change-Id: Ib46a965c0589bf1c1e8e4ae5f0c81747530eff98
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5842394
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
flock is called on the root disk file to make sure nothing is
concurrently modifying a disk (which would appear to the guest as disk
corruption). This CL extends that functionality to all of the nested
files of composite and qcow2 disks.
BUG=b:330911976
Change-Id: I4489222943c3530255ffc52aba5a11aa4bc82f9a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5840920
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Cody Schuffelen <schuffelen@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
new_from_header will open the backing file as part of the normal code
path, we don't need to manually insert it into the resulting QcowFile
object.
Change-Id: I06dfa0954b9ac4b8b18fc43b3047d684088ceef8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5840919
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
No behavior difference intended. This is prep work to treat nested
DiskFiles consistently, e.g. using flock for them.
BUG=b:330911976
Change-Id: Id887ca21a9d5d186ba4dc77280c1ce4bfec8b319
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5840918
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Note that this CL doesn't mandate specifying the partition GUID. If
partition GUID is omitted, it will be automatically created as before.
The commandline interface has changed but in a backwards compatible
manner. The argument synatax for 'composite-disk' command is
LABEL:PATH:[writable]:[uuid]
BUG=352244386
TEST=tools/dev_container tools/presubmit
Change-Id: Ibf5ba0c2a3aa16e8c9393943c2fee0c71c38ed25
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5690989
Commit-Queue: Jiyong Park <jiyong@google.com>
Auto-Submit: Jiyong Park <jiyong@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
This was used in a single informational log statement when opening a
disk image, which was added for debugging an issue and is no longer
needed. It was also only ever implemented for Linux and not other
platforms.
BUG=None
TEST=tools/dev_container tools/presubmit
Change-Id: Id77244a883b6bb4b90ee1a909fa305ab4193c17e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5656451
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
The cros_async Overlapped I/O implementation on Windows does not support
roundtripping from AsyncDisk to DiskFile and then back to AsyncDisk
again with a new executor instance because, once a file is added to an
IOCP instance, it is permanently associated with that instance.
Attempting to add it to new IOCP instance results in an error.
It is possible to workaround this by changing the cros_async API so that
we can keeping using the same IOCP instance when we restart the worker
thread, but we decided that is too complex.
Instead, we'll just keep the worker thread running indefinitely. Since
`AsyncDisk::into_inner` isn't portable, we delete it so that someone in
the future doesn't accidentally design their code around it.
The main new complexity in the code is support for swapping out the
active `Interrupt` in the worker thread. Technically crosvm seems to
always resend the same `Interrupt` object, but it seems best to not make
any assumptions.
BUG=b:301269927
Change-Id: Iee0492d18fe640f45ae08ed946536b9a9dd6f525
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5640657
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
This exercises more of the CacheMap API, and it also silences a clippy
warning about the value inside the test's NumCache structure being
unused.
BUG=b:344974550
TEST=cargo test -p disk --features=qcow
TEST=tools/clippy
Change-Id: I4cbee164fdd7c0bc662c804c50482d118ec05df3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5626478
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Use .create_new() rather than .create() for cases where we always want
to ensure that the file does not already exist, and add .truncate(false)
for a case where we do want to open an existing file and don't want to
overwrite it.
BUG=b:344974550
TEST=tools/clippy
Change-Id: Ie82a6db306532c600c140efab3d310b6c7cf25a7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5604660
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Ensure that every Cargo.toml dependency on a third-party crates.io crate
specifies at least a major version, or a minor version for 0.x crates,
to ensure that if a new major version is published, it cannot cause API
breaks.
The versions are selected to match the ones already in Cargo.lock, so
this should have no functional change, but it will help prevent new "*"
versions from being introduced via copy-and-paste.
For rationale, see the Cargo FAQ:
<https://doc.rust-lang.org/cargo/faq.html#can-libraries-use--as-a-version-for-their-dependencies>
`minijail`, `audio_streams`, and `cras` are left as "*" for now, since
they have unusual situations (imported from a submodule and/or replaced
at build time with ebuild magic).
BUG=None
TEST=tools/dev_container tools/presubmit
TEST=verify Cargo.lock is unchanged
Change-Id: Ifa18199f812f01d2d10bfb4146b3353c1a76527c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5555656
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
The tempfile crate is only used in tests, so it does not need to be in
the main [dependencies] section of any of our first-party crates.
Also fix a couple of [dev_dependencies] instances to the officially
documented [dev-dependencies] name for consistency.
BUG=None
TEST=cargo tree
Change-Id: I1dcb6be10c302baec4e8ebe6f72480f76e4e3760
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5521511
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
The disk traits no longer require exclusive refs. It might be reasonable
to forgo the blocking thread as well, but I'm leaving it as is for now.
BUG=b:338274203
Change-Id: I286c8983a7e850b3aa96c96d9388c77d1c9b7f04
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5507758
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Was only needed for QcowFile, which can now use its internal mutex.
BUG=b:338274203
Change-Id: I7db283cb946508daf8fd7bd8cf9397aa7f97a478
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5507757
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Was only needed for QcowFile, which can now use its internal mutex.
BUG=b:338274203
Change-Id: Iff684f5a25b0f488b006e05eebe913f92f404a2b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5507756
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Was only needed for QcowFile, which can now use its internal mutex.
BUG=b:338274203
Change-Id: I810b537e971bf007b6b34f0cbc605f2b3943a763
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5507754
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Was only needed for QcowFile, which can now use its internal mutex.
BUG=b:338274203
Change-Id: I995cf646a66909b9b146b9a7c0ff7d5a8348acd7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5507755
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
A lot of disk traits require exclusive references to file-like objects
for operations that are generally threadsafe, e.g. you ought to be able
to have an `Arc<File>` and perform pread/pwrite-like operations from
multiple threads concurrently.
Qcow files are the only case stopping us from changing these APIs. This
patch adds a big lock inside of the implementation. Follow up patches
will make use of that lock to switch various APIs from `&mut self` to
`&self`.
There is already a big lock around qcow files in AsyncDiskFileWrapper
and that lock will go away after all the APIs are switched over, so it
is unlikely there will be a performance penalty in the end.
BUG=b:338274203
Change-Id: Ifbac628395ab64eb9a327ceba31e03fd6ba2c192
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5507753
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
The div_ceil() is available since Rust 1.73. Replace our custom version
with the standard library function.
BUG=None
TEST=cargo test -p disk
Change-Id: I6b44d1f5189b1653d8617a51af41cd8a7cc3d162
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5438907
Reviewed-by: Takaya Saeki <takayas@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Use zerocopy functions directly instead. This is a step toward replacing
our data_model crate with standard crates.io functionality where
possible.
BUG=b:312312646
TEST=tools/dev_container tools/presubmit
Change-Id: I9717edce6fe2b4ca53ad9043db61de2e9bc55b78
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5046345
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
https://crrev.com/c/5019243 added a `once_cell::unsync::OnceCell` to
`IoSource`, making it `!Sync`, which, by definition, makes `&IoSource`
`!Send` and so (nearly) any future that does IO will be `!Send`.
This commit removes the `OnceCell` from `IoSource`, making it `Sync`
again. Instead, we eagerly check whether a backing file is a block
device when the file is converted to an `AsyncDisk` so that the result
is a plain `bool` and move the branch into `SingleDiskFile::punch_hole`.
The other `AsyncDisk` implementors don't need this logic.
"android_sparse" is read-only (i.e. you can't punch a hole).
"composite_disk" delegates hole punching to the backing disk.
"qcow" might benefit from it, but it doesn't use async yet, so
there is no regression.
BUG=b:271297810
Change-Id: Ifd1d2361adabfd4db1b48e71d3a3a31e57a0cfb1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5086673
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Nigthly is enabled as part of https://crrev.com/c/4950268
This change contains the formatting changes resulting from the switch.
BUG=b:302055317
TEST=dev_container presubmit format --no-delta
Change-Id: Idaf2b8bae2e09c624b19d9cd3dd5fc8e4d099b3c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5067088
Reviewed-by: Zihan Chen <zihanchen@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
IoBuf is defined in a platform-specific way (it is either iovec on unix
or WSABUF for windows), so it fits into the mission statement of the
base crate, which is meant to be *the* platform abstraction layer in
crosvm.
IoBufMut and VolatileMemory/VolatileSlice are defined in terms of IoBuf,
so they are also moved into base for consistency. Every crate that uses
these types already depended on base, so no extra dependencies are
added, and a few can be removed.
BUG=b:312312646
TEST=tools/dev_container tools/presubmit
Change-Id: I4dddc55d46906dfc55b88e8e6a967d7e1c1922dd
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5046605
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
`PunchHoleMut` trait is defined in the disk crate, but the base crate is
more suitable since `PunchHole` is defined by the base. This change
moves `PunchHoleMut` to base.
BUG=None
TEST=./tools/presubmit
Change-Id: Iee3eb48c88e58e4f54118bedde62a78fa9ff05dc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5019622
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Currently, cros_async's async IO source provides async `fallocate`
function, and `punch_hole` and `write_zeroes_at` are delegated to
`fallocate`. However, this design has three rooms for improvements.
1. Actually, no one calls async `fallocate`.
2. We have duplicated `fallocate` and `AllocateMode` implementations in
`base::sys`.
3. Windows has to pretend to have `fallocate` to provide `punch_hole`
and `write_zeroes_at`
So, this change removes `fallocate` and `AllocateMode`, which no one
actually calls, from `IoSource` and deletegate it to `base::sys`. With
this, we don't have to maintain `fallocate` implementation in
cros_async.
This will also enable us to call a non-`fallocate` systemcall in
`punch_hole` implementation which will be necessary to correctly support
block files.
BUG=b:302437024
TEST=./tools/presubmit
Change-Id: I8e7fcd94380da980cc52e6f751b134fa325d7b39
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5019241
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
