cros_async: Move --async-executor to run/device/devices subcommands

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>
This commit is contained in:
Takaya Saeki 2022-10-27 06:55:41 +00:00 committed by crosvm LUCI
parent 4db19f1baa
commit 303c285b7f
7 changed files with 43 additions and 11 deletions

View file

@ -10,6 +10,7 @@ use base::AsRawDescriptors;
use base::RawDescriptor;
use once_cell::sync::OnceCell;
use serde::Deserialize;
use serde::Serialize;
use thiserror::Error as ThisError;
use super::poll_source::Error as PollError;
@ -161,7 +162,9 @@ pub enum Executor {
}
/// An enum to express the kind of the backend of `Executor`
#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, serde_keyvalue::FromKeyValues)]
#[derive(
Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, serde_keyvalue::FromKeyValues,
)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
pub enum ExecutorKind {
Uring,

View file

@ -7,6 +7,7 @@ use std::future::Future;
use async_task::Task;
use once_cell::sync::OnceCell;
use serde::Deserialize;
use serde::Serialize;
use thiserror::Error as ThisError;
use super::HandleExecutor;
@ -126,7 +127,9 @@ pub enum Executor {
}
/// An enum to express the kind of the backend of `Executor`
#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, serde_keyvalue::FromKeyValues)]
#[derive(
Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, serde_keyvalue::FromKeyValues,
)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
pub enum ExecutorKind {
Handle,

View file

@ -113,10 +113,6 @@ pub struct CrosvmCmdlineArgs {
#[argh(switch)]
/// disable output to syslog
pub no_syslog: bool,
/// configure async executor backend; "uring" or "epoll" on Linux, "handle" on Windows.
/// If this option is omitted on Linux, "epoll" is used by default.
#[argh(option, arg_name = "EXECUTOR")]
pub async_executor: Option<ExecutorKind>,
#[argh(subcommand)]
pub command: Command,
}
@ -389,6 +385,11 @@ pub struct VfioCrosvmCommand {
#[argh(subcommand, name = "device")]
/// Start a device process
pub struct DeviceCommand {
/// configure async executor backend; "uring" or "epoll" on Linux, "handle" on Windows.
/// If this option is omitted on Linux, "epoll" is used by default.
#[argh(option, arg_name = "EXECUTOR")]
pub async_executor: Option<ExecutorKind>,
#[argh(subcommand)]
pub command: DeviceSubcommand,
}
@ -590,6 +591,11 @@ pub struct RunCommand {
/// path to Android fstab
pub android_fstab: Option<PathBuf>,
/// configure async executor backend; "uring" or "epoll" on Linux, "handle" on Windows.
/// If this option is omitted on Linux, "epoll" is used by default.
#[argh(option, arg_name = "EXECUTOR")]
pub async_executor: Option<ExecutorKind>,
#[argh(option, arg_name = "N")]
#[serde(skip)] // TODO(b/255223604)
/// amount to bias balance of memory between host and guest as the balloon inflates, in mib.
@ -1725,6 +1731,8 @@ impl TryFrom<RunCommand> for super::config::Config {
cfg.android_fstab = cmd.android_fstab;
cfg.async_executor = cmd.async_executor;
cfg.params.extend(cmd.params);
cfg.per_vm_core_scheduling = cmd.per_vm_core_scheduling;

View file

@ -18,6 +18,7 @@ use arch::Pstore;
use arch::VcpuAffinity;
use base::debug;
use base::pagesize;
use cros_async::ExecutorKind;
use devices::serial_device::SerialHardware;
use devices::serial_device::SerialParameters;
use devices::virtio::block::block::DiskOption;
@ -1095,6 +1096,7 @@ pub struct Config {
pub ac97_parameters: Vec<Ac97Parameters>,
pub acpi_tables: Vec<PathBuf>,
pub android_fstab: Option<PathBuf>,
pub async_executor: Option<ExecutorKind>,
pub balloon: bool,
pub balloon_bias: i64,
pub balloon_control: Option<PathBuf>,
@ -1295,6 +1297,7 @@ impl Default for Config {
ac97_parameters: Vec::new(),
acpi_tables: Vec::new(),
android_fstab: None,
async_executor: None,
balloon: true,
balloon_bias: 0,
balloon_control: None,

View file

@ -1391,6 +1391,11 @@ fn get_default_hypervisor() -> Result<HypervisorKind> {
}
pub fn run_config(cfg: Config) -> Result<ExitState> {
if let Some(async_executor) = cfg.async_executor {
Executor::set_default_executor_kind(async_executor)
.context("Failed to set the default async executor")?;
}
let components = setup_vm_components(&cfg)?;
let guest_mem_layout =
@ -3014,6 +3019,11 @@ fn start_vhost_user_control_server(
}
pub fn start_devices(opts: DevicesCommand) -> anyhow::Result<()> {
if let Some(async_executor) = opts.async_executor {
Executor::set_default_executor_kind(async_executor)
.context("Failed to set the default async executor")?;
}
struct DeviceJailInfo {
// Unique name for the device, in the form `foomatic-0`.
name: String,

View file

@ -5,6 +5,7 @@
use std::path::PathBuf;
use argh::FromArgs;
use cros_async::ExecutorKind;
use devices::virtio::block::block::DiskOption;
use devices::virtio::vhost::user::device;
use devices::virtio::vhost::user::VhostUserParams;
@ -41,6 +42,10 @@ fn parse_vu_serial_options(s: &str) -> Result<VhostUserParams<SerialParameters>,
#[argh(subcommand, name = "devices")]
/// Start one or several jailed device processes.
pub struct DevicesCommand {
/// configure async executor backend to "uring" or "epoll" (default).
#[argh(option, arg_name = "EXECUTOR")]
pub async_executor: Option<ExecutorKind>,
#[argh(switch)]
/// disable sandboxing. Will nullify the --jail option if it was present.
pub disable_sandbox: bool,

View file

@ -398,6 +398,11 @@ fn create_qcow2(cmd: cmdline::CreateQcow2Command) -> std::result::Result<(), ()>
}
fn start_device(opts: cmdline::DeviceCommand) -> std::result::Result<(), ()> {
if let Some(async_executor) = opts.async_executor {
cros_async::Executor::set_default_executor_kind(async_executor)
.map_err(|e| error!("Failed to set the default async executor: {:#}", e))?;
}
let result = match opts.command {
cmdline::DeviceSubcommand::CrossPlatform(command) => match command {
CrossPlatformDevicesCommands::Block(cfg) => run_block_device(cfg),
@ -588,11 +593,6 @@ fn crosvm_main<I: IntoIterator<Item = String>>(args: I) -> Result<CommandStatus>
..Default::default()
};
if let Some(async_executor) = args.async_executor {
cros_async::Executor::set_default_executor_kind(async_executor)
.context("Failed to set the default async executor")?;
}
let ret = match args.command {
Command::CrossPlatform(command) => {
// Past this point, usage of exit is in danger of leaking zombie processes.