There is a lot of code and test duplication w.r.t. the non-async impl
but we expect to delete the non-async impl in the near future, so the
temporary duplication is preferable vs increased code complexity.
Bug: 219595052
Change-Id: I402166331606d41178c1557c66be591d95c98c45
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4158596
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
These should be written as ChromeOS and ChromiumOS (without the space)
to match the updated branding. The copyright headers were already
migrated to the new style (https://crrev.com/c/3894243), but there were
some more instances left over.
BUG=None
TEST=tools/cargo-doc
Change-Id: I8c76aea2eb33b2e370ab71ee9b5cc0a4cfd00585
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4129934
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This reverts commit ca90b4d1bd.
Reason for revert: This causes issues for Cattlefish
Original change's description:
> devices: virtio: create the Executor for the block before jail
>
> To utilize the io_uring-based Executor with the sandbox, each device
> must create any `Executor`s before jailing.
>
> This commit has the virtio-blk device make its `Executor` in `new`
> method, which is called in the main process before jailing. This change
> allows the virtio-block device to run in the sandbox with io_uring.
>
> BUG=b:251039942
> BUG=b:262341514
> TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=uring` runs
> with and without the sandbox
> TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=epoll` runs
> with and without the sandbox
>
> Change-Id: Iadb002d436a9d45f63220528273f9d2e283fbd88
> Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4098933
> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Bug: b:251039942
Bug: b:262341514
Change-Id: I23097d929a0e9f680e3aaa02f4d83d8f7e1ddb26
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4114052
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Takaya Saeki <takayas@chromium.org>
To utilize the io_uring-based Executor with the sandbox, each device
must create any `Executor`s before jailing.
This commit has the virtio-blk device make its `Executor` in `new`
method, which is called in the main process before jailing. This change
allows the virtio-block device to run in the sandbox with io_uring.
BUG=b:251039942
BUG=b:262341514
TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=uring` runs
with and without the sandbox
TEST=`crosvm run --rwdisk path=blkdisk.img,async_executor=epoll` runs
with and without the sandbox
Change-Id: Iadb002d436a9d45f63220528273f9d2e283fbd88
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4098933
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
io_uring allows you to submit a lot of file operations. Those operations
have corresponding syscalls, but bypass the seccomp filter since they
are submitted by single syscall. To address this security hole, the
IORING_REGISTER_RESTRICTIONS can be used to apply an allow-list of
operations to a given io_uring queue.
This commit introduces `IORING_REGISTER_RESTRICTIONS` support for
`URingContext`, and has URingExecutor apply a basic allow-list to the
UringContexts that it creates. This basic allow-list is limited to
operations allowed by common_device.policy plus fsync and fallocate.
However, note that this commit is only one step for the effective jail
of io_uring. Since `IORING_REGISTER_RESTRICTIONS` is applicable only
per-queue, a compromised device process can still easily bypass this
restriction just by creating another io_uring queue. The effective jail
is completed with coming changes that allow executor creation only
before jailing.
This commit also updates the policy file with `io_uring_register`.
BUG=b:251039942
TEST=`cargo test -p io_uring` passed
TEST=`crosvm run --disable-sandbox --async-executor uring ...` booted
TEST=`crosvm run --async-executor uring ...` with the jail booted on x86
TEST=`cargo build` succeded on aarch64.
TEST=`cargo check` succeded on armv7.
Change-Id: I6e024f240eaf9fb6ce16f1a3d46b7f923a5f5169
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4071824
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Add async_new_async_{playback,capture}_stream to StreamSource trait.
These methods are used to create new async playback or capture stream
asynchronously. Default implementation is to call their sync version.
cros_async: Add wait_fd_readable to AudioStreamsExecutor trait, this
will be used by libcras to implement the new async_new_async_*_stream
method.
BUG=b:258759773
TEST=emerge audio_streams cros_async
TEST=FEATURES=test emerge-amd64-generic audio_streams
Change-Id: I8dfc743eb8fa6552a1a6b2953b79666e2909e417
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4066242
Commit-Queue: Pattara Teerapong <pteerapong@chromium.org>
Reviewed-by: Chih-Yang Hsia <paulhsia@chromium.org>
block device
Currently, crosvm has `--async-executor` option to switch the async
runtime engine for the whole crosvm. On the other hand, it is reported
that io_uring executor causes regression for some devices such as audio
(b:207621853). Thus, for incrementally enabling the io_uring executor,
we want per-device async-executor option to enable it for devices which
we observed the improvement.
This CL adds the per-device async-executor option for the block device.
BUG=b:251105847
TEST=confirmed that the async is switched by `crosvm run ... --block
...,async-executor=uring`
TEST=confirmed that the async is switched by `crosvm devices --block
...,async-executor=uring`
Change-Id: I0ff12741c2113b9aa5999357a0ce52e8dbd11933
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4054806
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
These are not caught during presubmit or when running cargo clippy but
nonetheless exist and should be cleaned up.
BUG=b:157245930, crbug:908640
TEST=cargo build
Change-Id: I8b4d79bf224b800fbdfeaa7729c5f283d42908e9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4039522
Auto-Submit: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This CL is a documentation-only change to add # Safety in pre-existing
safety documenation so clippy picks it up and does not flag it as
undocumented.
BUG=b:157245930, chromium:908640
TEST=remove suppression -> cargo clippy -> warnings removed
Change-Id: Iaf79839e2c8e4d8e7f74d18f244f4d02f1576bb2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4021747
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
...round 2 because building locally missed some :)
Most are safe, similar to crrev.com/c/4014832
Two require an exception, similar to crrev.com/c/4018392
BUG=b:157245930, crbug:908640
TEST=CQ with needless_return suppression disabled
Change-Id: I0f54ee1e4688afe9c9f596bd32a98070aac20612
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4019273
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: 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>
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>
The most notable changes are:
* Playback Async support
* Audio Client flags to hide app from SndVol when audio session has
expired
* Updates to audio_streams and cros_async crates for playback async
support
BUG=b:256655413
TEST=Verified to work downstream
Change-Id: Ifbe9a15791feaa41d6e1d5eaf2c5824b2c7c25b8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3994882
Commit-Queue: Richard Zhang <rizhang@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
Previously, several tests used the same hard-coded filename for test
files ("/tmp/write_from_vec"), which could cause flaky tests when
running in parallel. Replace these with a tempfile invocation so the
tests can run simultaneously.
BUG=None
TEST=cargo test -p cros_async
Change-Id: I2b32d998583f9a44359345628d803b2397af33b1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3993927
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This allows resetting an Event without waiting on it. Windows already
had this functionality in its EventExt, and we can provide an equivalent
implementation for eventfd on Linux, so promote this function to the
cross-platform Event type.
BUG=b:231344063
TEST=tools/presubmit --all
Change-Id: I6dba3cb2e0b2d702e8a3f0dacf5c25c1dd044a13
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3966480
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Trying to reconcile the difference between the linux and windows
implementations.
Code relying on the eventfd count must now use the linux specific
`EventExt` interface.
BUG=b:231344063
TEST=presubmits
Change-Id: I14eb50f7a02d766a00f27aca388823309633e193
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3864030
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Upstreaming a fix from nkgold@ for the following issue:
ReadNotifiers shouldn't ever be reset as their state is managed by the
type that exposed the notifier (e.g. StreamChannel). Resetting it can
lead to lose notifications. idanr@ discovered that EventAsync::clone_raw
was creating a regular EventAsync (e.g. with reset), and it was being
used with read notifiers. This CL replaces clone_raw with a new factory
that produces `without_reset` EventAsyncs.
BUG=b:253255209
TEST=presubmit, downstream
Change-Id: I0b9de2f12cc3597ba05a8ac108f4cd0a8e726b3f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3960857
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Idan Raiter <idanr@google.com>
cros_async and by extension serde_keyvalue are needed on ChromeOS, so
add the `# provided by ebuild` comments where appropriate to make it
possible to uprev cros_async (which requires pulling in serde_keyvalue).
BUG=b:250883877
TEST=kokoro passes
Change-Id: I423cac0c4e5e30afac058b914358caca7d27edbf
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3953082
Auto-Submit: Allen Webb <allenwebb@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
This is a followup of crrev.com/c/3931568.
Since `derive(serde_keyvalue::FromKeyValues)` used for `ExecutorKind` is
guarded by the serde_keyvalue's `argh_derive` feature, we need to
specify the feature in cros_async's Cargo.toml.
The original CL didn't break the crosvm build because our builder always
enable the `argh_derive` feature.
BUG=b:239154263
TEST=cargo test in /cros_async
Change-Id: I10c77373241271e3002fe574c69e0118059a7f1a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3945519
Auto-Submit: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
FromStr trait is manually implemented for ExecutorKind, but
serd_keyvalue should be preferred. This commits refactors ExecutorKind
so that derives serd_keyvalue.
BUG=None
TEST=`crosvm --async-executor (epoll|uring) run ...` works
TEST=test passes on linux and windows
Change-Id: I5aafb6704c20a17120b30c290e1a4069dff9e5ce
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3924880
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
backend"
This reverts commit 03fdffb447.
Now b:249692652 is fixed by crrev.com/c/3842811/, we can bring back the
command-line option to switch Executor backend.
This commit is identical with the original CL, except it resolving the
conflict with ccrev.com/c/3841945/; it renames the old `use_uring` in a
test to the new name, `is_uring_stable`.
(cros_async/src/sys/unix/uring_source.rs:652)
BUG=b:249692652
BUG=b:239154263
TEST=confirmed crosvm --async-executor correcly switchs the Executor for
`crosvm --async-executor epoll (or uring) device gpu --socket
/tmp/gpu.sock` command. Used additional debug log to tell which
executoris running, which is not commited)
TEST=all tests passed
Change-Id: Iafb6921c1a7594a178712eb46792dc54d5d99f71
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3931568
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
The `ReadAsync` and `WriteAsync` traits accept file offsets as an
`Option<u64>`. The epoll executor interprets `None` as "current file
position" and the io_uring executor interprets `None` as position zero.
This change updates the io_uring executor in favor of the epoll executor
behavior.
It is important to differentiate between position zero and "current file
position" when you don't know whether a file is seekable. For example,
console and serial devices should support both types of files as input.
BUG=b:243198718
TEST=presubmit --all
Change-Id: I01634e6bbdff352c0a6055419d0fafd10c0c40a6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3841945
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Steven Moreland <smoreland@google.com>
Reviewed-by: Takaya Saeki <takayas@chromium.org>
Make it official that setting a Timer's interval to 0 is the same as
setting it to None, meaning it will not be periodic. Fixed the windows
timer to behave this way again, and updated Timer docstring.
Also updated APIC timer to use None for interval which is clearer.
Bug: b:248612697
Test: tested with UserspaceIrqChip in dynamic tick mode
Change-Id: I57713c3ace345a8c64b9219a8e60f36c54a9481c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3925006
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Colin Downs-Razouk <colindr@google.com>
This reverts commit cb6e74a094.
Reason for revert: This CL itself is fine, but b:249692652 reported
that virtio-console has been broken with epoll executor. cb6e74a0 also makes this broken virtio-console, which is confusing for users of virtio-console. cb6e74a0 can be merged again with the workaround for the default executor or after b:249692652 is fixed.
Original change's description:
> cros_async: add a command-line option to switch Executor backend
>
> Currently Executor internally decides which backend to use, such as
> FdExecutor vs URingExecutor on Linux. This makes it hard to test both
> FdExecutor and URingExecutor. In addition, we came across the uring
> instability issue (b:238965061), thus we need a way to explicitly switch
> the polling backend to mitigate the issue.
>
> This commit adds a command-line option to explicitly configure the
> Executor's polling backend.
>
> BUG=b:239154263
> TEST=confirmed crosvm --async-executor correcly switchs the Executor for
> `crosvm --async-executor epoll (or uring) device gpu --socket
> /tmp/gpu.sock` command. Used additional debug log to tell which executor
> is running, which is not commited
>
> Change-Id: Ib310d9edc8ab6005f1b7f210b03668b9a45fa73f
> Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3811014
> Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
> Commit-Queue: Takaya Saeki <takayas@chromium.org>
> Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
> Reviewed-by: Noah Gold <nkgold@google.com>
Bug: b:239154263
Bug: b:249692652
Change-Id: I15f575597534cea6004bca4679245ef958bbe1e6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3925329
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Currently Executor internally decides which backend to use, such as
FdExecutor vs URingExecutor on Linux. This makes it hard to test both
FdExecutor and URingExecutor. In addition, we came across the uring
instability issue (b:238965061), thus we need a way to explicitly switch
the polling backend to mitigate the issue.
This commit adds a command-line option to explicitly configure the
Executor's polling backend.
BUG=b:239154263
TEST=confirmed crosvm --async-executor correcly switchs the Executor for
`crosvm --async-executor epoll (or uring) device gpu --socket
/tmp/gpu.sock` command. Used additional debug log to tell which executor
is running, which is not commited
Change-Id: Ib310d9edc8ab6005f1b7f210b03668b9a45fa73f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3811014
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Currently, the utility function `run_one` has its own logic to create
`FdExecutor` and `UringExecutor`. However, this logic duplicates that of
`Executor::new`. So when you update the logic of `Executor::new`, you
must also update `run_one`, which is easy to forget and error-prone.
This commit updates `run_one` to delegate the executor building logic to
`Executor::new`. Also, the error type of `run_one` has been updated to
match `Executor::new`.
BUG=None
TEST=`cargo build` and `cargo test -p cros_async` succeeds.
TEST=windows build succeeds
Change-Id: Ic90a064c4c9017ed14e2897bccf68f9e275e1fef
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3829220
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
This search/replace updates all copyright notices to drop the
"All rights reserved", Use "ChromiumOS" instead of "Chromium OS"
and drops the trailing dots.
This fulfills the request from legal and unifies our notices.
./tools/health-check has been updated to only accept this style.
BUG=b:246579983
TEST=./tools/health-check
Change-Id: I87a80701dc651f1baf4820e5cc42469d7c5f5bf7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3894243
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Brings up the master request handler on Windows. To stay within the
existing protocol, PackedTube was introduced, which is able to transmit
a Tube via a RawDescriptor.
BUG=b:221882601
TEST=presubmit & cargo test & tested downstream
Change-Id: Ibd4c2f55c42738392beed5b15b8d445e47890627
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3856828
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Idan Raiter <idanr@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
The functions being called do not have return values (they return the
unit value), so the `let _ =` does nothing and can be removed.
Fixes a new clippy lint:
<https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value>
BUG=b:243677117
TEST=tools/clippy # with Rust 1.62
Change-Id: I6003b162c36e7be1ee71e3edc4e304c86fdc5676
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3854970
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
`cargo test -p cros_async --doc` is failing with FdExecutor since the
example tries to seek a pipe. With UringExecutor, this error doesn't
happen by chance due to the difference in the implementation detail. So,
you need kernels prior to 5.10 to reproduce this issue.
This commit fixes the failure by not seeking a pipe.
BUG=b:242515595
FIXED=b:242515595
TEST=cargo test -p cros_async --doc succeeds.
Change-Id: Ifa97eb85d3b72061f1754e22108017df327c85d9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3831841
Tested-by: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
- Remove trailing ::{self} on all use statements
- Remove any resulting single-level use statements (e.g. use libc;)
- Reformat with `tools/fmt --nightly`
BUG=b:239937122
TEST=tools/dev_container tools/presubmit --all
Change-Id: I8afd1b0458ca6d08d9b41a24583f7d4148597ccb
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3798973
Auto-Submit: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
crosvm is switching the import style to use one import per line.
While more verbose, this will greatly reduce the occurence of merge
conflicts going forward.
Note: This is using a nightly feature of rustfmt. So it's a one-off
re-format only. We are considering adding a nightly toolchain to
enable the feature permanently.
BUG=b:239937122
TEST=CQ
Change-Id: Id2dd4dbdc0adfc4f8f3dd1d09da1daafa2a39992
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3784345
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Specify the type so cargo doc can find the right async_from function.
Fixes a warning when running tools/cargo-doc.
BUG=None
TEST=tools/cargo-doc
Change-Id: Ida4431cd663d4b5959a58a707c9c89d7569df986
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3764043
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
The test is flaky by nature since there is no API guarantee on how
long SetWaitableTimer will sleep. This is flaky on our Luci bots, so
increase the permitted deviation a bit.
BUG=b:238232551
TEST=./tools/run_tests --build-target=mingw64
Change-Id: Ib227be097a0a5559898571cb6b77c37e7fd61cff
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3749936
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Add an async_from_local() method, which does the same thing as
async_from() (create an IoSourceExt for asynchronous I/O), but without
the requirement that the passed argument implements Send. Async I/O
sources created that way can only be used in thread-local executors.
BUG=b:229554679
TEST=cargo build
Change-Id: Id4cd0cb177e5fed4b72fab12a6f82f964b9bac28
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3670773
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
We already exported PollContext as EventContext for use in the portable
WaitContext wrapper, so this makes no difference to callers.
A few remaining references to PollContext in documentation and errors
are fixed up as well to avoid confusion.
BUG=None
TEST=cargo build
Change-Id: I89943d4d9e135a0da7dd0fbff470b6a14efd669a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3687059
Reviewed-by: Noah Gold <nkgold@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Migrate the last few users of WatchingEvents to the equivalent EventType
values so we can remove the unix-specific WatchingEvents type.
BUG=b:213153157
TEST=cargo build
Change-Id: I215127309591125c4c64ff2963e7d2b8e1914cf3
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3687057
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
The main distinction between PollContext and EpollContext was that the
latter was safe to use from multiple threads. This was not true of the
more widely-used PollContext for two reasons:
1. The `events` array was stored inside the `PollContext` structure,
whereas `EpollContext` required the caller to pass their own storage
for events.
2. `PollContext` had a hangup detection feature used to debug busy
looping cases that result from a failure to remove hungup file
descriptors from the context.
Point 1 is resolved by returning a `SmallVec` of events that avoids
allocation for the normal case (and in fact it should avoid allocation
in all cases on Linux, where the maximum number of events returned from
a single `epoll_wait()` call is limited to the same size as the
preallocated `SmallVec`). This simplifies the API and also means that
there is no need for per-context storage.
Point 2 can't easily be resolved, since it would require the `wait` call
to mutate shared state (this could be done by adding a mutex around the
shared data, but that seems like too much overhead for the value of the
feature). Instead, this patch just removes the hangup detection code.
BUG=b:213153157
TEST=tools/dev_container tools/presubmit --all
Change-Id: Ia48c46de96976da27cb5387e3e5e8fcf92d0e85b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3633111
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Test verifies if operation is not executed if executor is dropped before
running. We do not currently guarantee that
BUG=none
TEST=cargo test
Change-Id: I56d98e4af869c26542c7ebacb127ed3bfd980553
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3655277
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Anton Romanov <romanton@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Wine does not support the FSCTL_SET_ZERO_DATA request:
fixme:ntdll:server_ioctl_file Unsupported ioctl 980c8 (device=9 access=2
func=32 method=0)
So ignore the tests that require it when using the mingw toolchain,
which we assume means testing in wine for now.
BUG=None
TEST=tools/dev_container ./tools/run_tests --target=host --arch=win64
Change-Id: I351758fac3b3e1eb44545bb1d2340ddec4461120
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3626021
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Anton Romanov <romanton@google.com>
A previous commit moved the creation of the executor for the vhost-user
device processes before the minijail jail_and_fork() call, and the file
descriptor of the executor was not marked for preservation when entering
the jail, so running the vhost-user-fs device would result in errors
like this:
[WARNING:cros_async/src/sys/unix/uring_executor.rs:343] Failed to
submit NOP for waking up executor: Failed to enter io uring: 9
This is because the io_uring context file descriptor was no longer
valid, so uring system calls would return 9 (EBADF).
Add AsRawDescriptors implementations to UringExecutor and FdExecutor so
we can preserve the fd across the minijail fork.
Other vhost-user devices were not affected because they don't enter a
jail; the fs device requires this so it can do uid/gid remapping.
BUG=b:231396155
TEST=crosvm device fs --socket /tmp/vhost.sock --tag test --shared-dir /
Fixes: d368c1520a ("devices: vhost-user: fs: remove global executor variable")
Change-Id: I38cf02034bbc28de2582e2ea92bdc13b1d6055d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3628292
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Lepton Wu <lepton@chromium.org>
