Disallow code emitting warnings in run_tests instead of
.cargo/config.toml.
There are three places where we can specify "-Dwarnings":
(1). rustflags in .cargo/config.toml
(2). flags in health-check
(3). RUSTFLAGS environment variable used in run_tests
(1) was the place where we had -Dwarnings, but this affects downstream
projects that use cargo. So, it can cause an unexpected build error when
unsupported combinations of feature flags are used in the downstream
project. So, we want to force "-Dwarnings" only in the upstream CI.
(2) is a place we already have the flag. However, the health-check script
only runs on x86_64 platform in our CI.
So, we choose (3) in this CL so we can force "-Dwarnings" only for fixed
sets of features used in the upstream CI. Also, this can support all of
platforms in the upstream CI.
BUG=b:181763000
TEST=CQ
Change-Id: Iceededcc51e19d6781758e7fd7d5a2d7941d9ebc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4014329
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
1. remove `--current-branch` as it shouldn't be used for crOS
checkout as crOS can have ebuilds sync to commit not part of main
2. allow unshallow to fail as sometimes repo sync results in a deep
checkout
TEST=led get-builder luci.crosvm.ci:chromeos_hatch | led edit-cr-cl https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015697 | led edit-recipe-bundle | led launch
BUG=b:240692674
Change-Id: I10335c07dc5f907b7b8989b6af53a24c803844d6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015697
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Auto-Submit: Zihan Chen <zihanchen@google.com>
This config does not include the privileges of the usual devcontainer.
BUG=None
TEST=Open in github codespaces
Passes both:
cargo test --workspace --lib --bins --features=all-x86_64
tools/run_tests --unit-tests
Change-Id: Ia68b0b4f7baa2be109079909eb023ba4eda5e037
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015008
Reviewed-by: Zihan Chen <zihanchen@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Before this CL, there were several places with suppressed clippy
warnings about needless_return statements following the same pattern of
spawning a worker thread in a match statement. After this CL, those
places have had the work refactored out of the match statement such that
there is no needless_return triggered by the code, but it does the same
thing.
BUG=chromium:908640, b:157245930
TEST=cargo clippy with needless_return suppression removed
Change-Id: Idfa4a16597d973ef840a031990c01640c4a8252d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4014833
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
This extra file/mod was left over from before the base/sys_util refactor
and is unnecessary now. Everything has been moved into the
stream_channel mod.
BUG=b:256739711
TEST=presubmit
Change-Id: Ie77c54e3f27ff2173c26060a62258941f710206a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015561
Reviewed-by: Richard Zhang <rizhang@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
They were missing before and would prevent some tests from being run.
BUG=None
TEST=cargo test --workspace --bins --lib
Change-Id: Iec9db80a15d8ddc2f4c83e6b272eae8a99f138d0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015007
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
The fuzz binary targets do not compile upstream.
This allows us to run all unit tests via cargo.
BUG=None
TEST=cargo test --workspace --lib --bins
Change-Id: I3923c79a8d622824956599b9b8552c7d1a610b70
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015006
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
These tests require access to kvm and cannot run in every context.
The tests do not pass on aarch64, so the DO_NOT_RUN_AARCH64
flag remains.
BUG=b:244623107
TEST=presubmit
Change-Id: I221e1055dfe2da7c2f0763f1f26acc211ced4400
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3990013
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Dennis Kempin <denniskempin@google.com>
This reduces the runtime of devices unit tests from ~5s to 1.7s.
BUG=None
TEST=None
Change-Id: I9801bba025db71ea3c69d26f8afcfce07db7d894
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004680
Reviewed-by: Vikram Auradkar <auradkar@google.com>
The test would take ~5s to run, cutting down the number of blocks
to write/read in the test will improve execution speed to .5s.
BUG=None
TEST=tools/run_tests -v disk\*
Change-Id: Ia877335bc662f4e7c5c91eff56c072bb1eb30e8d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004300
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Auto-Submit: Dennis Kempin <denniskempin@google.com>
The tests are explicitly verifying the timeout behavior, which
by default is 5s. We can speed up the tests by using a smaller
timeout when running a test.
Reduces the time to run the test from 6s to 1s
BUG=None
TEST=tools/run_tests cros_async:cros_async -v
Change-Id: I6262b653878171c6746a1ca90139b00a099dd91e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004299
Auto-Submit: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
This CL contains a few fixes to needless_return which need approval from
someone familiar with the code. In two cases, the return was removed
from a function that returns a value, so it is considered safe because
if code is added later into the function, it will be clear that this is
removing an implicit return from the function. In the last case
(iommu.rs), the function has several error! macros that do not trigger
an early return, so it seems that it is okay to remove the last early
return.
BUG=b:157245930, chromium:908640
TEST=cargo clippy (no logical code changes)
Change-Id: I00685c9033499a92713eb591eadb5f90c29e761d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4014835
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: David Stevens <stevensd@chromium.org>
Currently, we ignore clippy::needless_return, with many violations in
the code base. This CL fixes trivial, safe-to-fix violations of
needless_return. These were found by removing the needless_return
suppression in .cargo/config.toml and then fixing needless returns.
These are not all needless returns in the code base; it is only the
trivial fixes. This CL is a part of a chain of CLs to fix the
needless_returns in the code base.
BUG=chromium:908640, b:157245930
TEST=cargo clippy + cargo build
Change-Id: I6f8f3a0bed25d774fba7ae4b6e4c021af439ec22
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4014832
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Don't cause compile error when the `gdb` feature is specified on
unsupported platform (e.g. armhf). Instead, do nothing
when it's specified.
Also, enable "gdb" feature in all-armhf feature so it's tested in CI.
Note that this change is a follow-up of
https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4011747/comment/973cd5d8_02e5a2f7/
BUG=none
TEST=run_tests --platform=armhf with "gdb" feature enabled
Change-Id: I06fc2e428c3595eb01c7172759945aa4a3159e8a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015999
Auto-Submit: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
This is a reland of commit 77688e305d
Tests are ignored on some architectures due to the single thread test
issues b/258371694
Original change's description:
> base: unix: add fork_process
>
> The vmm-swap feature will fork a process to run monitoring userfaultfd.
>
> crosvm uses minijail to fork device processes `ProxyDevice::new()`.
>
> Minijail panics on fork if there are any other threads running. The test
> must be executed in a single thread.
>
> design document: go/tanooki-phase1-dd
>
> BUG=b:215093219
> TEST=cargo test -p base -- --test-threads=1
>
> Change-Id: I408dbfa4d606cbe7b2218096b414512710d60100
> Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3935683
> Reviewed-by: David Stevens <stevensd@chromium.org>
Bug: b:215093219
Change-Id: I94b912b04947cada3e3332861f18988873dfcf81
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015626
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: David Stevens <stevensd@chromium.org>
Auto-Submit: Shin Kawamura <kawasin@google.com>
The vmm-swap feature will fork a process to run monitoring userfaultfd.
crosvm uses minijail to fork device processes `ProxyDevice::new()`.
Minijail panics on fork if there are any other threads running. The test
must be executed in a single thread.
design document: go/tanooki-phase1-dd
BUG=b:215093219
TEST=cargo test -p base -- --test-threads=1
Change-Id: I408dbfa4d606cbe7b2218096b414512710d60100
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3935683
Reviewed-by: David Stevens <stevensd@chromium.org>
ProcessesGuard stops all the crosvm processes except the caller process
using SIGSTOP signal because we must guarantee that no one changes the
guest memory contents while vmm-swap out.
This is a short term solution and will be replaced with device
suspension feature later.
See the "Write back atomicity" section in go/tanooki-phase1-dd
BUG=b:215093219
TEST=cargo build --features=swap
Change-Id: Ie62a0cf537a045128c0d298e7a73d222fea96ef0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3932497
Reviewed-by: David Stevens <stevensd@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
SwapFile saves the active pages in the memory region to a file.
design document: go/tanooki-phase1-dd
BUG=b:215093219
TEST=cargo test -p swap
Change-Id: I4ce16eef36ace832d26622e44444f7549299c1e7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3930082
Reviewed-by: David Stevens <stevensd@chromium.org>
userfaultfd enables applications to handle page faults on designated
memory area.
vmm-swap feature uses userfaultfd to catch page fault event and swap
in the guest memory from the swap file.
design document: go/tanooki-phase1-dd
BUG=b:215093219
TEST=cargo build --features=swap
Change-Id: I13ae09cd97e4215b00e5d834d4f97eb6b507b892
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3895235
Reviewed-by: David Stevens <stevensd@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
We never use this method, so remove it. Besides how to copy an image is
dependent of the format and handled by read(), so this should be the
prefered way of accessing frames.
This also allows us to remove the dummy mapped handles.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
Change-Id: I4909a33f2ba3dbcc61e27493ed0ad3b8a75cd882
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005491
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Error descriptions should have no leading capital and do not end with a
dot.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
Change-Id: I807e4874c862bc8e32dc4a394b1e5e46df60c8d0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005489
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
These types are only used in two places, and their actual definition is
more explicit about their purpose.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.
Change-Id: I90eec9751bc4aaccab33fe70d96a94a5875f6f6c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3995769
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
The DynDecodedHandle traits defined for each codec are basically
identical, and can be simplified into a single one now that we also have
a common DecodedHandle struct.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.
Change-Id: I666d941dfea5b5e40ca46dab661e87aec60d65e4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998376
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
The DecodedHandle structs defined for each codec are basically identical
and can be simplified into a single one by adding an associated type.
Doing so required the H.264 types to be constrained with non-trivial
where clauses due to the fact the H264 PictureData is itself generic,
but there is a plan to simplify this too.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.
Change-Id: Idcb0717740e6e700ae83d758f99ba0035981dcca
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998375
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
SurfaceContainer was implemented for each codec type, but it can be
simplified into a single generic implementation if move the RefCell out
of InnerHandle.
Since SurfaceContainer's purpose is just to convert one object into a
Surface, rename it into the more idiomatic IntoSurface that just
requires TryInto<Option<Surface>>.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.
Change-Id: I00a56132ceaeab5d305e144976a9ea597355bbf8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998374
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
The Picture structs defined for each codec are now basically identical,
and can be simplified into a single one by adding one generic argument.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs:wq
TEST=`cros-codecs --include-ignored` passes on hatch.
Change-Id: I48a79c615f7c53de5d39bcf37da0b64d05c65a09
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998373
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
The Picture structs defined for each codec are essentially identical,
with only the H.264 one having its codec-specific data inlined into the
struct instead of being part of another one. Move the H.264 data in
order to make the 3 structures similar. This will allow us to merge them
into one common struct in the next CL.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.
Change-Id: I3282fde1dbbc2ad303f2691e00c46aa8b6c0de41
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998372
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
backend_handle_unchecked* were only used in tests, and the DynPicture
interface provides equivalent functions anyway, so use these ones and
drop the former.
Ideally these methods that can potentially panic should be replaced by
something else, but this will have to wait further cleanups.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.
Change-Id: I339f6893599c5ffc24e738f5ad11f35cbfcaab25
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998371
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Prior to this CL, clippy warnings needless_doctest_main and
blocks_in_if_conditions were suppressed. It turns out they have no
violations, as removing them and running cargo clippy produces no
warning. This CL removes those suppressions.
BUG=b:157245930, chromium:908640
Test=cargo clippy
Change-Id: I82f82bf2fdb7740f45e7187a9b2710fb7dc2ca04
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4015559
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Clarissa Garvey <clarissagarvey@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
The value of VIRTIO_GPU_RESP_OK_RESOURCE_PLANE_INFO (which is not
upstream) conflicts with upstream VIRTIO_GPU_RESP_OK_EDID. We assign
both VIRTIO_GPU_RESP_OK_EDID and VIRTIO_GPU_RESP_OK_RESOURCE_UUID to the
same value.
BUG=b:2050923
TEST=none (just comments)
Change-Id: I8fe2c3818e451cc456ae6c15b9b827c22cfbd6e9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3993318
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Auto-Submit: Chad Versace <chadversary@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
There is no CLI change in this CL. All existing tests are either
moved to gpu_config.rs or merged with other tests.
BUG=b:254284360
TEST=presubmit
Change-Id: Id9be8247d540337230d20db6f313a983743664b1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3971033
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Pujun Lun <lunpujun@google.com>
The gdb feature is supported only on x86_64 and aarch64.
This change removes some compiler warnings.
BUG=b:181763000
TEST=CQ
Change-Id: I22e99574e31147d5ec681bea02c5e0e43c9ca592
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4011922
Auto-Submit: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Show the debug log to tell the configured default async executor
backend.
BUG=b:251289312
TEST=confirmed the debug log is shown by --async-executor option
Change-Id: I82f040d2ad19739cd486945bae2b5d99492a62a1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4006066
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Currently, --async-executor option to switch the backend of
cros_async::Executor is a global option of crosvm main command. This is
because --async-executor is an option to switch the async runtime
backend engine of cros_async crate for a whole crosvm.
However, in practice, only run, device, and devices subcommands have the
motivation to switch the backends for the performance. Other subcommands
may or may not use async functions, but it's unlikely they want to
switch the backends. So, it makes sense to make --async-executor an
option of those three subcommands, not of the global option. Also, it
allows us to switch --async-executor in arvm_dev.conf, which overrides
the options for `crosvm run` command, and to control the async executor
by the coming configuration file feature.
Thus, this commit moves the --async-executor option from the global
command to the run, device, and devices subcommands. This is a breaking
change, but it is unlikely to have many users using this relatively new
option. Takayas also confirmed there's no usage on codesearch.
BUG=b:251289312
TEST=confirmed `crosvm --log-level debug run --async-executor ...`
switches the executor with additional debug log in the coming CL.
TEST=confirmed `crosvm --log-level debug device --async-executor ...`
switches the executor with additional debug log in the coming CL.
TEST=confirmed `crosvm --log-level debug devices --async-executor ...`
switches the executor with additional debug log in the coming CL.
TEST=./tools/dev_container ./tools/presubmit
Change-Id: Ia25f2d0b296b8d73f31d71c362e5f90678166d96
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005473
Auto-Submit: Takaya Saeki <takayas@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Using a fixed CID may cause conflict with another guest easily.
Change to use a random CID for a test VM.
BUG=b:258072662
TEST=`./tools/presubmit`
Change-Id: I1f3bab8bd64664b1b2feb480a6d0e285ea4099bb
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4012278
Commit-Queue: Ryuichiro Chiba <chibar@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
The patch makes usb feature no-op on windows as USB pass-through is
not supported on windows.
BUG=b:241251677
BUG=b:213149155
TEST=tools/presubmit
Change-Id: Id82d4c732a86e782695d2af698cc08e8e3fd2d35
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4006819
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Vikram Auradkar <auradkar@google.com>
This was previously only referenced by the trunks test code, which was
removed in commit 62770b484a ("Remove trunks proto from crosvm
build").
BUG=None
TEST=tools/presubmit
Change-Id: I8883e805037f0da3feb999d685d722cb182bf585
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4010558
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
