From dd2c0a932a7e995a2fc5fe0d70036ade48f13bf3 Mon Sep 17 00:00:00 2001 From: Pujun Lun Date: Tue, 20 Sep 2022 14:48:39 -0700 Subject: [PATCH] crosvm: move enum ProcessType to win_util. This is to keep consistency with Windows downstream code. The conversion from ProcessType to EmulatorProcessType is moved to the metrics crate as it is only used for generating metrics. BUG=b:213146388 TEST=presubmit Change-Id: Ia62f76835a1f162dd8bbc9e53fd671968c368473 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3908370 Reviewed-by: Daniel Verkamp Commit-Queue: Pujun Lun --- Cargo.lock | 3 +++ common/cros_asyncv2/Cargo.lock | 13 +++++++++++++ metrics/Cargo.toml | 1 + metrics/src/sys/windows.rs | 15 +++++++++++++++ src/crosvm/sys/windows/broker.rs | 2 +- src/crosvm/sys/windows/config.rs | 29 ----------------------------- src/crosvm/sys/windows/exit.rs | 3 +-- win_util/Cargo.toml | 2 ++ win_util/src/lib.rs | 18 ++++++++++++++++++ 9 files changed, 54 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eab95fda7b..17cf32d8b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1168,6 +1168,7 @@ dependencies = [ "serde", "serde_json", "sync", + "win_util", "winapi", "wmi", ] @@ -1972,8 +1973,10 @@ name = "win_util" version = "0.1.0" dependencies = [ "anyhow", + "enumn", "libc", "once_cell", + "serde", "winapi", "windows", ] diff --git a/common/cros_asyncv2/Cargo.lock b/common/cros_asyncv2/Cargo.lock index d409f422ab..b29dde8318 100644 --- a/common/cros_asyncv2/Cargo.lock +++ b/common/cros_asyncv2/Cargo.lock @@ -170,6 +170,17 @@ dependencies = [ "winapi", ] +[[package]] +name = "enumn" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "038b1afa59052df211f9efd58f8b1d84c242935ede1c3dbaed26b018a9e06ae2" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "env_logger" version = "0.9.0" @@ -722,8 +733,10 @@ name = "win_util" version = "0.1.0" dependencies = [ "anyhow", + "enumn", "libc", "once_cell", + "serde", "winapi", "windows", ] diff --git a/metrics/Cargo.toml b/metrics/Cargo.toml index 96e9dc446f..143e0f8be6 100644 --- a/metrics/Cargo.toml +++ b/metrics/Cargo.toml @@ -20,6 +20,7 @@ sync = { path = "../common/sync", optional = true } [target.'cfg(windows)'.dependencies] chrono = { version = "*" } winapi = { version = "*" } +win_util = { path = "../win_util" } wmi = { version = "^0.9" } diff --git a/metrics/src/sys/windows.rs b/metrics/src/sys/windows.rs index d0f7923843..6814a47645 100644 --- a/metrics/src/sys/windows.rs +++ b/metrics/src/sys/windows.rs @@ -8,6 +8,9 @@ pub(crate) mod system_metrics; pub mod wmi; pub use gpu_metrics::*; +use win_util::ProcessType; + +use crate::event_details_proto::EmulatorProcessType; pub const METRIC_UPLOAD_INTERVAL_SECONDS: i64 = 60; pub const API_GUEST_ANGLE_VK_ENUM_NAME: &str = "API_GUEST_ANGLE_VK"; @@ -22,3 +25,15 @@ pub enum Error { } pub type Result = std::result::Result; + +impl From for EmulatorProcessType { + fn from(process_type: ProcessType) -> Self { + match process_type { + ProcessType::Block => EmulatorProcessType::PROCESS_TYPE_BLOCK, + ProcessType::Main => EmulatorProcessType::PROCESS_TYPE_MAIN, + ProcessType::Metrics => EmulatorProcessType::PROCESS_TYPE_METRICS, + ProcessType::Net => EmulatorProcessType::PROCESS_TYPE_NET, + ProcessType::Slirp => EmulatorProcessType::PROCESS_TYPE_SLIRP, + } + } +} diff --git a/src/crosvm/sys/windows/broker.rs b/src/crosvm/sys/windows/broker.rs index 32d7f1d885..ad08c2cad7 100644 --- a/src/crosvm/sys/windows/broker.rs +++ b/src/crosvm/sys/windows/broker.rs @@ -71,11 +71,11 @@ use tube_transporter::TubeToken; use tube_transporter::TubeTransferData; use tube_transporter::TubeTransporter; use win_util::get_exit_code_process; +use win_util::ProcessType; use winapi::shared::winerror::ERROR_ACCESS_DENIED; use winapi::um::processthreadsapi::TerminateProcess; use crate::bail_exit_code; -use crate::crosvm::sys::config::ProcessType; use crate::crosvm::sys::windows::exit::to_process_type_error; use crate::crosvm::sys::windows::exit::Exit; use crate::crosvm::sys::windows::exit::ExitCode; diff --git a/src/crosvm/sys/windows/config.rs b/src/crosvm/sys/windows/config.rs index 592eacaccb..09e5b7a98f 100644 --- a/src/crosvm/sys/windows/config.rs +++ b/src/crosvm/sys/windows/config.rs @@ -21,7 +21,6 @@ use devices::virtio::DEFAULT_REFRESH_RATE; #[cfg(feature = "audio")] use devices::Ac97Parameters; use devices::SerialParameters; -use metrics::event_details_proto::EmulatorProcessType; #[cfg(feature = "gpu")] use rutabaga_gfx::calculate_context_mask; #[cfg(feature = "gpu")] @@ -486,34 +485,6 @@ pub(crate) fn validate_gpu_config(cfg: &mut Config) -> Result<(), String> { Ok(()) } -/// Each type of process should have its own type here. This affects both exit -/// handling and sandboxing policy. -/// -/// WARNING: do NOT change the values items in this enum. The enum value is used in our exit codes, -/// and relied upon by metrics analysis. The max value for this enum is 0x1F = 31 as it is -/// restricted to five bits per `crate::crosvm::sys::windows::exit::to_process_type_error`. -#[derive(Clone, Copy, PartialEq, Debug, enumn::N)] -#[repr(u8)] -pub enum ProcessType { - Block = 1, - Main = 2, - Metrics = 3, - Net = 4, - Slirp = 5, -} - -impl From for EmulatorProcessType { - fn from(process_type: ProcessType) -> Self { - match process_type { - ProcessType::Block => EmulatorProcessType::PROCESS_TYPE_BLOCK, - ProcessType::Main => EmulatorProcessType::PROCESS_TYPE_MAIN, - ProcessType::Metrics => EmulatorProcessType::PROCESS_TYPE_METRICS, - ProcessType::Net => EmulatorProcessType::PROCESS_TYPE_NET, - ProcessType::Slirp => EmulatorProcessType::PROCESS_TYPE_SLIRP, - } - } -} - #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub enum IrqChipKind { /// All interrupt controllers are emulated in the kernel. diff --git a/src/crosvm/sys/windows/exit.rs b/src/crosvm/sys/windows/exit.rs index d38ede2717..5ccd0f3c05 100644 --- a/src/crosvm/sys/windows/exit.rs +++ b/src/crosvm/sys/windows/exit.rs @@ -9,8 +9,7 @@ use std::fmt::Display; use std::fmt::Formatter; use anyhow::Context; - -use crate::crosvm::sys::config::ProcessType; +use win_util::ProcessType; pub type ExitCode = i32; diff --git a/win_util/Cargo.toml b/win_util/Cargo.toml index 95eda47c6b..e975fd6fef 100644 --- a/win_util/Cargo.toml +++ b/win_util/Cargo.toml @@ -6,9 +6,11 @@ edition = "2021" [dependencies] anyhow = "*" +enumn = "0.1.0" winapi = { version = "*", features = ["everything", "std", "impl-default"] } libc = "*" once_cell = "1.7" +serde = { version = "1", features = [ "derive" ] } windows = "0.10.0" [build-dependencies] diff --git a/win_util/src/lib.rs b/win_util/src/lib.rs index eeea03fb6c..38fe0f9986 100644 --- a/win_util/src/lib.rs +++ b/win_util/src/lib.rs @@ -31,6 +31,8 @@ use std::slice; use std::sync::Once; use libc::c_ulong; +use serde::Deserialize; +use serde::Serialize; use winapi::shared::minwindef::DWORD; use winapi::shared::minwindef::FALSE; use winapi::shared::minwindef::TRUE; @@ -317,6 +319,22 @@ mod bindings { #[cfg(target_env = "msvc")] pub use bindings::Windows::Win32::Globalization::ImmDisableIME; +/// Each type of process should have its own type here. This affects both exit +/// handling and sandboxing policy. +/// +/// WARNING: do NOT change the values items in this enum. The enum value is used in our exit codes, +/// and relied upon by metrics analysis. The max value for this enum is 0x1F = 31 as it is +/// restricted to five bits per `crate::crosvm::sys::windows::exit::to_process_type_error`. +#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, Serialize, Deserialize, enumn::N)] +#[repr(u8)] +pub enum ProcessType { + Block = 1, + Main = 2, + Metrics = 3, + Net = 4, + Slirp = 5, +} + #[cfg(test)] mod tests { use super::*;