Commit graph

253 commits

Author SHA1 Message Date
Noah Gold
fc8a9dd071 cros_async: remove debug prints.
It looks like some debug code we introduced when adding executor type
selection might've stuck around in the tree. Let's remove it.

BUG=NONE
TEST=builds

Change-Id: I83fd648104d0052048f00444acd1a98acd8eb93b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5981506
Commit-Queue: Noah Gold <nkgold@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2024-10-31 18:07:12 +00:00
Daniel Verkamp
a43b9346f8 cros_async: remove ?Sized from Default bounds
Default already includes Sized, so the ?Sized is not useful.

Fixes a clippy warning in Rust 1.81.

BUG=b:365852007

Change-Id: I32a78e65153e63779cdafed4a35a542e22dd699e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5852615
Reviewed-by: Frederick Mayle <fmayle@google.com>
Reviewed-by: Richard Zhang <rizhang@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2024-09-27 19:33:20 +00:00
Daniel Verkamp
c2ebaa19e4 Fix bad indentation in Markdown around lists
Some doc comments were being incorrectly interpreted due to missing
indentation (for intentional line contintuations) or line breaks (for
new lines that were intended to be separate paragraphs).

Clippy warns about these as of Rust 1.80:
<https://rust-lang.github.io/rust-clippy/master/index.html#/doc_lazy_continuation>

Fix them along with some other nearby minor formatting cleanups.

TEST=tools/cargo-doc
TEST=tools/clippy # with rust-toolchain 1.80

Change-Id: Ice0b7cc3bd75d9ab08c10107a13f95ca9f87a0a3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5758934
Auto-Submit: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2024-09-10 02:05:59 +00:00
Noah Gold
e47e28c1c1 cros_async: fix Windows test failures.
With create_overlapped becoming a "must create" open call, the misuse of
it in a test where the file already existed became a hard failure. This
CL fixes that issue by moving to open_overlapped (as it should have been
before).

BUG=b:346381109
TEST=tested downstream

Change-Id: Id49739b8b32a23eabe78e8173d8c397a0b4607e2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5630351
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Noah Gold <nkgold@google.com>
2024-06-13 21:49:24 +00:00
Daniel Verkamp
e38524847d Windows clippy fixes for Rust 1.77
BUG=b:344974550
TEST=tools/presubmit clippy_mingw64

Change-Id: I1a4c8a59cb0618ed40ebe04129be359dfee5c5e4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5627831
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2024-06-13 01:07:13 +00:00
Frederick Mayle
7ec3293ab4 base_tokio: tokio compatible Event and Tube types
BUG=b:338274203

Change-Id: I6fbce1386c2c086d7a53a706018d959f4fadfada
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5507771
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2024-06-06 23:30:20 +00:00
Daniel Verkamp
8a4df3ac57 cros_async: replace .map(Arc::clone) with .clone()
Fixes upcoming clippy::map_clone warnings in Rust 1.78, and it's simpler
as well.

BUG=b:344974550
TEST=tools/clippy

Change-Id: Ibde9321b834687123785d68b37d5f490534ad894
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5598838
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2024-06-04 21:44:08 +00:00
Daniel Verkamp
52b8e42869 Cargo.toml: avoid "*" versions for external crates
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>
2024-05-22 01:01:42 +00:00
Frederick Mayle
bd32973373 base: windows: expose Tube's ReadNotifier and CloseNotifier Events
Those traits use RawDescriptor, which isn't useable directly in Windows
and so the users have to assume it is an Event undernearth and coerce it
back, usually using `EventAsync::clone_raw_without_reset`.

BUG=b:338274203

Change-Id: I2fbaa38e58cbd1ee154078f8d74e33c4dc61f0b9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5539776
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2024-05-16 16:49:05 +00:00
Frederick Mayle
784ed62943 base: move cros_async's WaitForHandle into base
This functionality will also be used by a pure Tokio library for crosvm.

The API is now a single async function and is named to match the
underlying Windows API. The unit tests have been rewritten to not
require an async executor and should have a bit more test coverage.

BUG=b:338274203

Change-Id: Iff65ca088c74ce5c5ce396fdf7dcd9f1550ea545
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5536313
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
2024-05-14 17:34:54 +00:00
Frederick Mayle
5381cc2b66 base: don't require &mut for WriteZeroesAt methods
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>
2024-05-02 19:32:21 +00:00
Daniel Verkamp
664abe979e cros_async: tokio: replace Vec with slices when possible
Also rename the functions to more closely match the new parameter types.

This fixes a new (Rust 1.77+) clippy warning.

https://rust-lang.github.io/rust-clippy/master/index.html#/ptr_arg

Change-Id: Ic4759714c927fc9ebb41a6b698d70a0fa995fe54
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5467560
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Reviewed-by: Cody Schuffelen <schuffelen@google.com>
2024-04-22 08:22:40 +00:00
A. Cody Schuffelen
706b28b357 base: Separate TimerTrait::reset into 2 functions
See prior comment thread:

https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5454793/6..9//COMMIT_MSG#b12

`TimerTrait::reset` offers a very similar API to the `timerfd_settime`
system call, but in practice is almost used to set either a one-shot
timer, or a repeating timer with an initial timeout the same as the
repeating timeout.

`kqueue`'s timer functionality only supports the cases that are actually
used in practice. This change aligns the API with the way it is used,
and makes the distinction between the behavior of the arguments more
obvious.

Test: tools/presubmit
Test: cargo test -p base timer -- --ignored
Bug: 335486579
Change-Id: I1873ab9cccaf7a4b988431839964b1c253f76a34
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5463100
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2024-04-18 09:23:31 +00:00
Daniel Verkamp
2f5e0c0e91 Cargo.lock: switch to tokio 1.29.1
ChromeOS only has tokio version 1.29.1 vendored currently, so switch the
required version in Cargo.toml and update Cargo.lock to ensure we can
build against that version.

Since this is only a difference in minor version, newer versions of
tokio should also work for other downstream crosvm users.

The Cargo.lock update was done by:
  cargo update -p tokio --precise 1.29.1

BUG=b:334109381
TEST=emerge-$BOARD

Change-Id: I5f67367258cd51145a1ff74246cb4e0f02f6a8be
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5450247
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
2024-04-12 17:58:35 +00:00
A. Cody Schuffelen
b548048c51 cros_async: Tokio IoSource for windows
Bug: b/320603688
Test: tools/presubmit
Test: cargo build --features=cros_async/tokio --target=x86_64-pc-windows-gnu
Change-Id: Idd0824b2664bf6cab577296f34de8954b2495fd1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5171898
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
2024-04-12 02:27:38 +00:00
A. Cody Schuffelen
f59edf616b cros_async: Add AsyncFd to Tokio linux implementation
This uses tokio's utilities to re-schedule IO that would fail with a
"would block" error. The `IoSource` is chosen by the Executor based
on whether the file descriptor can be successfully added to the internal
epoll scheduler.

Specifically, AsyncFd::async_io is used for system calls that can fail
with EWOULDBLOCK, and spawn_blocking is used for system calls that will
always block.

Bug: b/320603688
Test: tools/presubmit
Test: cargo build --features=cros_async/tokio --target=x86_64-unknown-linux-gnu
Change-Id: Ibafbbe65d67cb88b5fcd0f18b00f504430c785c6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5174967
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
2024-04-12 02:10:07 +00:00
A. Cody Schuffelen
8407e84291 cros_async: Blocking tokio executor for linux
This is selectable with a new ExecutorKind. Rather than using the
common_executor backend, it uses tokio as the backend.

Bug: b/320603688
Test: tools/presubmit
Test: cargo build --features=cros_async/tokio --target=x86_64-unknown-linux-gnu
Change-Id: Ibf65b17077048f4ed1cce07fa985787b1eb1b6a3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5174966
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
2024-04-12 01:48:30 +00:00
A. Cody Schuffelen
874aa5d2b9 cros_async: Move concurrency to Overlapped enum
`Executor::with_kind_and_concurrency` exists only for the benefit of the
`ExecutorKindSys::Overlapped` enum variant, it's not used outside of
windows code and not used for `ExecutorKindSys::Handle`.

`DiskOption`'s `io_concurrency` value is used to override the
concurrency setting in the `ExecutorKind`.

Test: tools/presubmit
Change-Id: Idd59d78283038ab6f986953f846cd70ef25d4324
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5315960
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
2024-04-11 22:59:45 +00:00
A. Cody Schuffelen
b7ca74d516 cros_async: Combine platform-specific Executors
Instead of a dispatch enum defined for windows and a dispatch enum
defined for linux, there is now one dispatch enum with branches defined
for the two platforms. This makes it so defining the tokio executor
later only has to be added in one place.

Bug: b/320603688
Test: tools/dev_container tools/presubmit
Test: tools/dev_container tools/presubmit clippy
Change-Id: I6eaf46710bbb54a9684cb1622da36c3026906a5c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5208332
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
2024-03-26 16:54:35 +00:00
A. Cody Schuffelen
2df30d9221 Align linux and windows Executor types with ExecutorTrait
ExecutorTrait is not object safe, but this still forces some type
signature alignment.

Bug: b/320603688
Test: tools/presubmit && tools/presubmit clippy
Change-Id: I241d61b8c519aaf01264c1b82de3d496d0e1626f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5251105
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
2024-03-26 16:21:53 +00:00
A. Cody Schuffelen
23dbfa872b Align RawExecutor with a new ExecutorTrait based on Executor
ExecutorTrait is not object safe, but it can be used together with a
library like enum_dispatch to still have a kind of polymorphism.

Bug: b/320603688
Test: tools/presubmit
Change-Id: Ied449fcc497426e4d9fb0381f9f800a70d884f49
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5251104
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
2024-03-26 15:31:50 +00:00
A. Cody Schuffelen
288e1fe382 cros_async: Combine platform-specific TaskHandles
This follows the pattern used for IoSource to define a dispatch enum
that selects between implementations which may be on different
platforms.

Bug: b/320603688
Test: tools/presubmit
Change-Id: I8c0b1ddadb17d7f3f5dd45734333a9f6ef596cab
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5207890
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
2024-03-21 20:48:58 +00:00
Frederick Mayle
5fc2f8463d cros_async: iocp: fix condvar usage
Before the sequence was something like

    1. take lock
    2. compute a condition
    3. release lock
    4. check the condition
    5. take lock again
    6. wait on condvar

Between steps 3 and 5 someone else could take the lock and send a
notification that would then never be seen by step 6, potentially
resulting a deadlock.

BUG=b:321975174

Change-Id: I1bd329d3de43bf9b68feae80c5b87c9b0eb9aef4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5320156
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Richard Zhang <rizhang@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2024-03-04 03:32:48 +00:00
A. Cody Schuffelen
60ff4996ab cros_async: Separate ExecutorKind and ExecutorKindSys
Until the tokio executor is implemented, there are only
platform-specific executors.

Bug: b/320603688
Test: tools/presubmit
Change-Id: I380a6b61f6f6a0cca2fbd3e89d63a2c9c8d051ef
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5201658
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
2024-02-21 23:23:07 +00:00
Kaiyi Li
c28067d1d9 Reformat comments
Test: presubmit
Change-Id: I39c261d9985989873b698213c5d8b653fc13757b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5299850
Auto-Submit: Kaiyi Li <kaiyili@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2024-02-15 23:30:13 +00:00
Daniel Verkamp
58a06581bc base: linux: remove pipe() close_on_exec argument
Every caller passes `true`. If a hypothetical future caller wants pipe
descriptors that are not O_CLOEXEC, then they can use `pipe()` plus
`clear_descriptor_cloexec()`.

BUG=None
TEST=tools/dev_container tools/presubmit

Change-Id: If9aa62653e00a163824157819ddd2fce849fe348
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5251099
Reviewed-by: Frederick Mayle <fmayle@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2024-02-01 19:33:27 +00:00
Frederick Mayle
c7541ef925 cros_async: don't use relaxed atomics for executor state
I don't know if the relaxed ops are wrong, but they aren't clearly
correct.

Change-Id: Ie67b8ea3b4913e91182f6863514a5d47beb1615e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5246698
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
2024-01-30 19:41:00 +00:00
Richard Zhang
51e9899a32 cros_async: Implement "into_source" for OverlappedSource
This will fix a bug where the block device on Windows will panic when the CrosVm is being shutdown.

Bug=b:319154589
TEST=tested downstream, verified there is no panic

Change-Id: I73d9a63d51a1d8d7c9fe44a8bb85488f5f4304dd
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5214790
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Richard Zhang <rizhang@google.com>
2024-01-19 01:20:31 +00:00
Frederick Mayle
a478007d69 cros_async: delete Drop impl for HandleSource
CancellableBlockingPool's Drop already did the same thing, except it
didn't print the error.

This allows changing `source: Option<T>` to `source: T`.

BUG=b:319154589

Change-Id: I66b1edd0f04c9bca31d4535ae33bed00c3e131dc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5207539
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Richard Zhang <rizhang@google.com>
2024-01-18 03:56:38 +00:00
A. Cody Schuffelen
def2897c18 cros_async: Split off AsyncErrorSys from AsyncError
This more closely follows the style guide for platform-specific enums:
https://crosvm.dev/book/contributing/style_guide_platform_specific_code.html#enum

Bug: b/320603688
Test: tools/presubmit
Change-Id: Iaec4485620414e6032fbbbf4ce31f347d32b419d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5201655
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
2024-01-17 20:25:09 +00:00
Richard Zhang
ff28517db3 cros_async: Implement "into_source" for HandleSource and remove the
multi-handle feature

Without the "into_source" implementation, the blk device will crash
for Windows.

BUG=b:319154589
TEST=tested downstream to verify this works on Windows

Change-Id: Ida383104e7a05f0f891f4ee99b3ce322e7eda64d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5201258
Commit-Queue: Richard Zhang <rizhang@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
2024-01-17 19:53:21 +00:00
Frederick Mayle
bafb341ed5 cros_async: assert IoSource is Send and Sync
Nothing needs an IO future to be `Send` at the moment, but that is
mostly a legacy of cros_async not supporting it until recently. This
compile time test will keep it from regressing.

BUG=b:271297810

Change-Id: Ib966740e1c563e675982326edd3bc055363b193f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5124626
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
2023-12-15 19:40:13 +00:00
Frederick Mayle
1cad723426 cros_async: move punch hole special case into disk crate
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>
2023-12-15 19:40:13 +00:00
Frederick Mayle
e66920249d cros_async: make overlapped IO sources Send
Futures that use `IoSource` methods can't be sent across threads because
they might be backed by an overlapped op, which contains raw pointers.
This makes `Executor::spawn` virtually unusable.

`OVERLAPPED` creation has been moved into `OverlappedOperation::new` so
that it is harder to keep one on the stack of an async function (and
inadvertently make it `!Send`).

A new type, `BoxedOverlapped`, is has been added as a workaround to mark
`OVERLAPPED` as `Send` and `Sync`. The `Pin` wasn't needed and has been
removed.

There is a similar issue on the linux side. I'll add a test after it is
fixed.

BUG=b:271297810

Change-Id: Ibfb5a9a47bdf731e0647d4813c7bb2f4a3db8299
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5086672
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
2023-12-14 22:29:30 +00:00
Vikram Auradkar
2768f223ee clippy: enforce safety block comments
BUG=b:316174930
TEST=none

Change-Id: I5c7811b2c548155aa003e4b71a54bbc16e2f2588
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5120567
Commit-Queue: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2023-12-14 18:21:54 +00:00
Frederick Mayle
69a9a0e4a0 cros_async: remove EventAsync's redundant unsafe Send impl
All the fields are `Send`, so `EventAsync` is implicitly `Send`.

This was probably added originally because the predecessor to `IoSource`
was always `!Send` even when the underlying type was `Send`.

BUG=b:271297810

Change-Id: If72de71c6f37c72f5b791391c657d1234d9f765f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5086674
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2023-12-05 21:04:39 +00:00
Daniel Verkamp
fcc28f5573 data_model: move IoBuf and VolatileMemory to base
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>
2023-11-27 21:42:49 +00:00
Takaya Saeki
5c5483d0ea base: linux: replace AsRawFd with AsRawDescriptor in base
`AsRawDescriptor` is preferred over `AsRawFd` everywhere. This commit
replaces `AsRawFd` of the file related helpers in base except functions
which explicitly intend to handle FDs such as `validate_raw_fd` or
`get_fd_flags`.

BUG=None
TEST=./tools/presubmit

Change-Id: Ia0f1b149bf281a97dfe754fd20e3d9d898539df1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5020382
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
2023-11-20 09:56:30 +00:00
Takaya Saeki
3b4d1a6f5c cros_async: fix punch_hole so that it works for a block file
Currently, `punch_hole` operation is translated to
`fallocate(PUNCH_HOLE|KEEP_SIZE)`. However, this does not work as
expected if the target file is a block device file such as an LVM
partition block file. It does not succeed and the area will not be
reclaimed, which is causing a problem on ARCVM.

The right way to punch-hole a block device file is calling
`ioctl(BLKDISCARD)`. This change fixes `PollSource`, `UringSource`,
and the default implementation of `PunchHole` trait.

BUG=b:302437024
TEST=./tools/presubmit
TEST=`strace -f -e ioctl crosvm run ... --block \
path=/dev/glinux_20230808/test_lv kernel` and `blkdiscard /dev/vda`
inside, then confirmed BLKDISCARD with epoll and io_uring

Change-Id: Ibedecd2c401bbb9ab25d7f0bfe18a3334f79ea85
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5019243
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
2023-11-20 09:53:12 +00:00
Takaya Saeki
85f1221536 cros_async: remove fallocate from IoSource and delegate it to base
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>
2023-11-14 07:12:11 +00:00
Daniel Verkamp
d362e2ea56 Fix new clippy 1.72 unnecessary_cast warnings
Remove pointer casts where the target type is the same as the original.

Change-Id: Ibb7bda2e4afcb5df2cd7c0bd9a075d0288b69f67
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5005512
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-11-06 19:16:18 +00:00
Vikram Auradkar
0762e46e75 cros_async: windows: make overlapped io concurrent
Windows needs N threads to poll to complete N IOs concurrently.
The patch createn 'n' concurrent threads that are passed as DiskOption

Bug: 303249046
Test: Ran tests downstream to verify that N ios are running in parallel

Change-Id: I9c1c101993e6ecbf99213218341cc77544fbd6bc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4985161
Auto-Submit: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Vikram Auradkar <auradkar@google.com>
2023-11-01 03:17:58 +00:00
A. Cody Schuffelen
18a1cc91a2 Convert some ::platform:: imports into ::linux:: or ::windows::
Change-Id: I447d9316ffd9c5662103d3458536304a272873b4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4996143
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
2023-11-01 00:48:00 +00:00
Vikram Auradkar
a35c24c3d0 block: introduce a way to use overlapped source
BUG=b:303249046
TEST=tested downstream

Change-Id: If436c5bff6a44d6dbebb6e0aa2bf9e743dd25f59
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4950198
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Vikram Auradkar <auradkar@google.com>
2023-10-25 16:29:00 +00:00
Daniel Verkamp
a5f4ea09ad cros_async: tweak timeouts in cancellable_pool tests
The timeout for all pool shutdowns was set to 100 ms in a previous
change in order to make the tests run faster:
<https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004299>

However, this can lead to flakiness if the system is under load while
the tests are executing. This change makes some more careful timeout
choices to improve reliability without extending the overall time it
takes to run the tests.

Most of the shutdown_with_timeout(100ms) calls are replaced with the
normal shutdown() function, which uses a 5-second timeout. These
shutdown() calls are expected to return immediately (either because of a
non-timeout error condition or because all pool tasks are already
finished), and we can be sure that this 5-second timeout is never
relevant because all of the cros_async tests execute in under 5 seconds.
This should fix a flaky test that was observed in practice where the
disarm_with_pending_work() test would not finish shutting down in the
(rather short) 100 ms timeout previously allowed.

The shutdown_with_blocked_work timeout is reduced from the original 100
ms to 1 ms, since it should not affect the correctness of the test and
will help it run even faster. This test is also renamed to match the
actual behavior - it used to mention "panic", but the shutdown function
simply returns an error and does not panic.

Finally, the shutdown_in_progress() test now uses two different
timeouts. This test spawns a thread that waits until shutdown is in
progress, then calls the normal shutdown() function to ensure it returns
the correct error; this shutdown() call should return immediately, so
again the 5-second timeout is not relevant. The main thread of the test
now uses a timeout of 200 ms, which should be sufficient to ensure the
other thread has time to start up and reach the shutdown() call while
observing the shutdown_in_progress flag. This extends the test time
somewhat relative to the previous 100 ms timeout, but that is mostly
traded off with the timeout reduction in the blocked work test, so the
overall test time should be about the same as before. This is probably
the most likely test to be flaky going forward, since it relies on OS
thread scheduling behavior; if any problems are observed, we can
consider making the timeout longer, although this will slow down the
tests.

BUG=b:306726068
TEST=tools/dev_container tools/presubmit
TEST=time cargo test -p cros_async

Change-Id: Ic6ec117d616ae7c99a8e3c5a4fe6dd1169c72286
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4961353
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2023-10-22 04:05:37 +00:00
Vikram Auradkar
bbc0c03dc1 base: add helper to blockingly read overlapped file
BUG=b:303249046
TEST=none

Change-Id: I4265c3b25c026f24cb3a0677da142a54448e5ce6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4939046
Commit-Queue: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
Auto-Submit: Vikram Auradkar <auradkar@google.com>
2023-10-16 19:38:09 +00:00
A. Cody Schuffelen
4748c54b95 Rename "unix" to "linux" in code and docs
$ for DIR in $(find . -name "unix"); do mv $DIR $(echo $DIR | sed "s/unix/linux/"); done
$ for FILE in $(find . -name "unix.rs"); do mv $FILE $(echo $FILE | sed "s/unix/linux/"); done
$ find . -type f -not -path '*/\.git/*' | xargs -I {} sed -E -i "s/mod unix/mod linux/g" {}
$ find . -type f -not -path '*/\.git/*' -not -path '*/third_party/perfetto/*' | xargs -I {} sed -E -i "s/([^o][^s])::unix/\1::linux/g" {}
$ find . -type f -not -path '*/\.git/*' | xargs -I {} sed -E -i "s/use unix::/use linux::/g" {}
$ find . -type f -not -path '*/\.git/*' -not -path '*/third_party/perfetto/*' | xargs -I {} sed -E -i "s/sys::unix/sys::linux/g" {}
$ find . -type f -not -path '*/\.git/*' | xargs -I {} sed -E -i "s/use unix as platform/use linux as platform/g" {}

Test: ./tools/dev_container ./tools/presubmit
Bug: b/298269162
Change-Id: I2c8acb14d77a5588dab4eae124f4a9afbb9025f5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4909060
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
2023-10-11 01:15:07 +00:00
A. Cody Schuffelen
97dff044f8 Replace #[cfg(unix)] with #[cfg(any(target_os = "android", target_os = "linux"))]
Updates are made to source and documentation.

This more accurately represents the currently supported platforms of
Android/Linux and Windows, without unexpectedly including other
unix-like operating systems.

Command to reproduce:
$ find . -type f -not -path '*/\.git/*' | xargs -I {} sed -i 's/cfg(unix)/cfg(any(target_os = "android", target_os = "linux"))/g' {}
$ cargo fmt

md files manually updated to fix line lengths.

Renaming `unix` modules to `linux` will be done in a later CL.

Test: ./tools/dev_container ./tools/presubmit
Bug: b/298269162
Change-Id: I42c1bf0abf80b9a0df25551613910293217c7295
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4909059
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
2023-10-11 00:43:29 +00:00
Daniel Verkamp
d5cd443dc7 Fix some cargo doc warnings in Windows-only code
Change-Id: I0ff72840853db764db6dfc8145581447dff8795d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4883336
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Junichi Uekawa <uekawa@chromium.org>
2023-09-22 00:59:46 +00:00
Daniel Verkamp
41f57f8f89 Apply nightly rustfmt import style
As usual, some unsorted and grouped imports have appeared.

Change-Id: I79b51e4c52cee38f5b8c238e46dfe3193c753554
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4847980
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-09-06 22:13:26 +00:00