From 5da71539a9cf41e3a1e277611429599b4b2042af Mon Sep 17 00:00:00 2001 From: Patrick Mooney Date: Wed, 27 Sep 2023 13:42:42 -0500 Subject: [PATCH] Use `strum` crate for enum-related utilities The functionality consumed from `num_enum` and `enum-iterator` is available in a consolidated manner from `strum`. --- Cargo.lock | 36 ++++--------------- Cargo.toml | 4 +-- bin/propolis-server/Cargo.toml | 3 +- bin/propolis-server/src/lib/migrate/codec.rs | 10 +++--- .../src/lib/migrate/protocol.rs | 8 ++--- bin/propolis-standalone/Cargo.toml | 2 +- bin/propolis-standalone/src/main.rs | 12 +++---- bin/propolis-standalone/src/snapshot.rs | 36 +++++++++---------- bin/propolis-utils/src/bin/savex.rs | 3 +- crates/bhyve-api/Cargo.toml | 4 +-- crates/bhyve-api/header-check/Cargo.toml | 1 + crates/bhyve-api/src/lib.rs | 7 ++-- crates/bhyve-api/sys/Cargo.toml | 2 +- crates/bhyve-api/sys/src/enums.rs | 6 ++-- crates/dladm/Cargo.toml | 2 +- crates/dladm/src/lib.rs | 14 +++----- crates/dladm/src/sys.rs | 6 ++-- crates/propolis-standalone-config/Cargo.toml | 2 +- crates/propolis-standalone-config/src/lib.rs | 4 +-- crates/viona-api/Cargo.toml | 1 - crates/viona-api/src/lib.rs | 5 +-- lib/propolis/Cargo.toml | 6 ++-- lib/propolis/src/cpuid.rs | 8 +---- lib/propolis/src/exits.rs | 19 +++++----- lib/propolis/src/hw/pci/bar.rs | 3 +- lib/propolis/src/hw/pci/mod.rs | 8 ++--- lib/propolis/src/hw/virtio/viona.rs | 2 +- lib/propolis/src/vcpu.rs | 2 +- lib/propolis/src/vmm/mod.rs | 2 +- phd-tests/framework/src/host_api/kvm.rs | 4 +-- xtask/src/task_clippy.rs | 3 ++ 31 files changed, 93 insertions(+), 132 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a7bc82541..9cf5a51e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -329,7 +329,7 @@ version = "0.0.0" dependencies = [ "bhyve_api_sys", "libc", - "num_enum 0.5.11", + "strum", ] [[package]] @@ -337,7 +337,7 @@ name = "bhyve_api_sys" version = "0.0.0" dependencies = [ "libc", - "num_enum 0.5.11", + "strum", ] [[package]] @@ -946,7 +946,7 @@ name = "dladm" version = "0.0.0" dependencies = [ "libc", - "num_enum 0.5.11", + "strum", ] [[package]] @@ -1102,26 +1102,6 @@ dependencies = [ "syn 1.0.109", ] -[[package]] -name = "enum-iterator" -version = "1.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7add3873b5dd076766ee79c8e406ad1a472c385476b9e38849f8eec24f1be689" -dependencies = [ - "enum-iterator-derive", -] - -[[package]] -name = "enum-iterator-derive" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eecf8589574ce9b895052fa12d69af7a233f99e6107f5cb8dd1044f2a17bfdcb" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.29", -] - [[package]] name = "env_logger" version = "0.9.3" @@ -3042,7 +3022,6 @@ dependencies = [ "crucible-client-types", "dladm", "dlpi", - "enum-iterator", "erased-serde", "futures", "ispf", @@ -3063,6 +3042,7 @@ dependencies = [ "slog-async", "slog-term", "softnpu", + "strum", "tempfile", "thiserror", "tokio", @@ -3142,7 +3122,6 @@ dependencies = [ "const_format", "crucible-client-types", "dropshot", - "enum-iterator", "erased-serde", "expectorate", "futures", @@ -3153,7 +3132,6 @@ dependencies = [ "lazy_static", "mockall", "nexus-client", - "num_enum 0.5.11", "omicron-common", "oximeter", "oximeter-producer", @@ -3173,6 +3151,7 @@ dependencies = [ "slog-bunyan", "slog-dtrace", "slog-term", + "strum", "thiserror", "tokio", "tokio-tungstenite", @@ -3206,7 +3185,6 @@ dependencies = [ "erased-serde", "futures", "libc", - "num_enum 0.5.11", "propolis", "propolis-standalone-config", "serde", @@ -3216,6 +3194,7 @@ dependencies = [ "slog-bunyan", "slog-dtrace", "slog-term", + "strum", "tokio", "toml 0.7.6", "uuid", @@ -3226,9 +3205,9 @@ name = "propolis-standalone-config" version = "0.0.0" dependencies = [ "cpuid_profile_config", - "num_enum 0.5.11", "serde", "serde_derive", + "strum", "toml 0.7.6", ] @@ -4966,7 +4945,6 @@ name = "viona_api" version = "0.0.0" dependencies = [ "libc", - "num_enum 0.5.11", "viona_api_sys", ] diff --git a/Cargo.toml b/Cargo.toml index bd5f42f5c..0cc33e382 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -100,7 +100,6 @@ crucible = { git = "https://github.com/oxidecomputer/crucible", rev = "aeb69dda2 crucible-client-types = { git = "https://github.com/oxidecomputer/crucible", rev = "aeb69dda26c7e1a8b6eada425670cd4b83f91c07" } ctrlc = "3.2" dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main" } -enum-iterator = "1.4.1" erased-serde = "0.3" errno = "0.2.8" expectorate = "1.0.5" @@ -113,7 +112,7 @@ inventory = "0.3.0" lazy_static = "1.4" libc = "0.2" mockall = "0.11" -num_enum = "0.5" +num_enum = "0.5.11" proc-macro2 = "1.0" progenitor = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" } quote = "1.0" @@ -133,6 +132,7 @@ slog-async = "2.8" slog-bunyan = "2.4.0" slog-dtrace = "0.2.3" slog-term = "2.8" +strum = "0.25" syn = "1.0" tempfile = "3.2" thiserror = "1.0" diff --git a/bin/propolis-server/Cargo.toml b/bin/propolis-server/Cargo.toml index 814b64954..cce239419 100644 --- a/bin/propolis-server/Cargo.toml +++ b/bin/propolis-server/Cargo.toml @@ -29,7 +29,6 @@ clap = { workspace = true, features = ["derive"] } const_format.workspace = true crucible-client-types.workspace = true dropshot = { workspace = true, features = ["usdt-probes"] } -enum-iterator.workspace = true erased-serde.workspace = true futures.workspace = true http.workspace = true @@ -37,7 +36,6 @@ hyper.workspace = true internal-dns.workspace = true lazy_static.workspace = true nexus-client.workspace = true -num_enum.workspace = true omicron-common.workspace = true oximeter-producer.workspace = true oximeter.workspace = true @@ -55,6 +53,7 @@ slog-async.workspace = true slog-bunyan.workspace = true slog-dtrace.workspace = true slog-term.workspace = true +strum = { workspace = true, features = ["derive"] } propolis = { workspace = true, features = ["crucible-full", "oximeter"] } propolis-client = { workspace = true, features = ["generated"] } propolis-server-config.workspace = true diff --git a/bin/propolis-server/src/lib/migrate/codec.rs b/bin/propolis-server/src/lib/migrate/codec.rs index f4bc319ab..4212ebb61 100644 --- a/bin/propolis-server/src/lib/migrate/codec.rs +++ b/bin/propolis-server/src/lib/migrate/codec.rs @@ -21,10 +21,10 @@ //! for that. use super::MigrateError; + use bytes::{Buf, BufMut, Bytes}; -use num_enum::{IntoPrimitive, TryFromPrimitive}; use slog::error; -use std::convert::TryFrom; +use strum::FromRepr; use thiserror::Error; use tokio_tungstenite::tungstenite; @@ -90,7 +90,7 @@ pub(crate) enum Message { /// identifying frame types. They are an implementation detail of /// the wire format, and not used elsewhere. However, they must be /// kept in bijection with Message, above. -#[derive(Debug, PartialEq, IntoPrimitive, TryFromPrimitive)] +#[derive(Debug, PartialEq, FromRepr)] #[repr(u8)] enum MessageType { Okay, @@ -179,8 +179,8 @@ impl std::convert::TryInto for tungstenite::Message { tungstenite::Message::Binary(mut v) => { // If the tag byte is absent or invalid, don't bother looking at the message. let tag_byte = v.pop().ok_or(ProtocolError::EmptyMessage)?; - let tag = MessageType::try_from(tag_byte) - .map_err(|_| ProtocolError::InvalidMessageType(tag_byte))?; + let tag = MessageType::from_repr(tag_byte) + .ok_or(ProtocolError::InvalidMessageType(tag_byte))?; let mut src = Bytes::from(v); // At this point, we have a valid message of a known type, and // the remaining bytes are the message contents. diff --git a/bin/propolis-server/src/lib/migrate/protocol.rs b/bin/propolis-server/src/lib/migrate/protocol.rs index b98df5791..70993e1c8 100644 --- a/bin/propolis-server/src/lib/migrate/protocol.rs +++ b/bin/propolis-server/src/lib/migrate/protocol.rs @@ -14,14 +14,14 @@ use std::{fmt::Display, iter::Peekable, num::ParseIntError, str::FromStr}; -use enum_iterator::Sequence; use lazy_static::lazy_static; use serde::{Deserialize, Serialize}; +use strum::{EnumIter, IntoEnumIterator}; use thiserror::Error; /// The complete set of protocols supported by this version of the migration /// library. -#[derive(Debug, Clone, Copy, Sequence)] +#[derive(Debug, Clone, Copy, EnumIter)] pub enum Protocol { RonV0, } @@ -171,7 +171,7 @@ impl FromStr for ProtocolParts { lazy_static! { static ref PROTOCOL_PARTS: Vec = - enum_iterator::all::().map(ProtocolParts::from).collect(); + Protocol::iter().map(ProtocolParts::from).collect(); } /// Constructs a protocol offer string from a peekable protocol iterator. @@ -194,7 +194,7 @@ fn make_protocol_offers_from_parts< /// Constructs a protocol offer string from the static supported protocol set. pub(super) fn make_protocol_offer() -> String { make_protocol_offers_from_parts( - enum_iterator::all::().map(ProtocolParts::from).peekable(), + Protocol::iter().map(ProtocolParts::from).peekable(), ) } diff --git a/bin/propolis-standalone/Cargo.toml b/bin/propolis-standalone/Cargo.toml index 80c4d3e4d..2c8b3b994 100644 --- a/bin/propolis-standalone/Cargo.toml +++ b/bin/propolis-standalone/Cargo.toml @@ -31,7 +31,7 @@ slog-async.workspace = true slog-dtrace.workspace = true slog-bunyan.workspace = true slog-term.workspace = true -num_enum.workspace = true +strum = { workspace = true, features = ["derive"] } uuid.workspace = true [features] diff --git a/bin/propolis-standalone/src/main.rs b/bin/propolis-standalone/src/main.rs index 5cbf4f37e..f98f8b2c9 100644 --- a/bin/propolis-standalone/src/main.rs +++ b/bin/propolis-standalone/src/main.rs @@ -13,20 +13,20 @@ use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::Context; use clap::Parser; use futures::future::BoxFuture; +use slog::{o, Drain}; +use strum::IntoEnumIterator; +use tokio::runtime; + use propolis::chardev::{BlockingSource, Sink, Source, UDSock}; use propolis::hw::chipset::{i440fx, Chipset}; use propolis::hw::ibmpc; use propolis::hw::ps2::ctrl::PS2Ctrl; use propolis::hw::uart::LpcUart; use propolis::intr_pins::FuncPin; +use propolis::usdt::register_probes; use propolis::vcpu::Vcpu; use propolis::vmm::{Builder, Machine}; use propolis::*; -use tokio::runtime; - -use propolis::usdt::register_probes; - -use slog::{o, Drain}; mod config; mod snapshot; @@ -934,7 +934,7 @@ fn setup_instance( ) .with_vcpuid(vcpu.id) .with_cache_topo() - .clear_cpu_topo(cpuid::TopoKind::all()) + .clear_cpu_topo(cpuid::TopoKind::iter()) .execute(profile.clone()) .context("failed to specialize cpuid profile")? } else { diff --git a/bin/propolis-standalone/src/snapshot.rs b/bin/propolis-standalone/src/snapshot.rs index 80ffff53a..4d4b6d335 100644 --- a/bin/propolis-standalone/src/snapshot.rs +++ b/bin/propolis-standalone/src/snapshot.rs @@ -17,7 +17,7 @@ //! 3 - Low Mem //! 4 - High Mem -use std::convert::{TryFrom, TryInto}; +use std::convert::TryInto; use std::path::Path; use std::sync::Arc; @@ -172,17 +172,17 @@ pub(crate) async fn save( info!(log, "Writing VM config..."); let config_bytes = toml::to_string(config)?.into_bytes(); - file.write_u8(SnapshotTag::Config.into()).await?; + file.write_u8(SnapshotTag::Config as u8).await?; file.write_u64(config_bytes.len().try_into()?).await?; file.write_all(&config_bytes).await?; info!(log, "Writing global state..."); - file.write_u8(SnapshotTag::Global.into()).await?; + file.write_u8(SnapshotTag::Global as u8).await?; file.write_u64(global_state.len().try_into()?).await?; file.write_all(&global_state).await?; info!(log, "Writing device state..."); - file.write_u8(SnapshotTag::Device.into()).await?; + file.write_u8(SnapshotTag::Device as u8).await?; file.write_u64(device_states.len().try_into()?).await?; file.write_all(&device_states).await?; @@ -190,14 +190,14 @@ pub(crate) async fn save( // Low Mem // Note `pwrite` doesn't update the current position, so we do it manually - file.write_u8(SnapshotTag::Lowmem.into()).await?; + file.write_u8(SnapshotTag::Lowmem as u8).await?; file.write_u64(lo.try_into()?).await?; let offset = file.stream_position().await?.try_into()?; lo_mapping.pwrite(file.get_ref(), lo, offset)?; // Blocks; not great file.seek(std::io::SeekFrom::Current(lo.try_into()?)).await?; // High Mem - file.write_u8(SnapshotTag::Himem.into()).await?; + file.write_u8(SnapshotTag::Himem as u8).await?; if let (Some(hi), Some(hi_mapping)) = (hi, hi_mapping) { file.write_u64(hi.try_into()?).await?; let offset = file.stream_position().await?.try_into()?; @@ -229,8 +229,8 @@ pub(crate) async fn restore( // First off we need the config let config: Config = { - match SnapshotTag::try_from(file.read_u8().await?) { - Ok(SnapshotTag::Config) => {} + match SnapshotTag::from_repr(file.read_u8().await?) { + Some(SnapshotTag::Config) => {} _ => anyhow::bail!("Expected VM config"), } let config_len = file.read_u64().await?; @@ -266,8 +266,8 @@ pub(crate) async fn restore( { // Grab the global VM state - match SnapshotTag::try_from(file.read_u8().await?) { - Ok(SnapshotTag::Global) => {} + match SnapshotTag::from_repr(file.read_u8().await?) { + Some(SnapshotTag::Global) => {} _ => anyhow::bail!("Expected VM config"), } let state_len = file.read_u64().await?; @@ -283,8 +283,8 @@ pub(crate) async fn restore( // Next are the devices let device_states = { - match SnapshotTag::try_from(file.read_u8().await?) { - Ok(SnapshotTag::Device) => {} + match SnapshotTag::from_repr(file.read_u8().await?) { + Some(SnapshotTag::Device) => {} _ => anyhow::bail!("Expected VM config"), } let state_len = file.read_u64().await?; @@ -296,8 +296,8 @@ pub(crate) async fn restore( // Finally we have our RAM // Get low mem length and offset - match SnapshotTag::try_from(file.read_u8().await?) { - Ok(SnapshotTag::Lowmem) => {} + match SnapshotTag::from_repr(file.read_u8().await?) { + Some(SnapshotTag::Lowmem) => {} _ => anyhow::bail!("Expected VM config"), } let lo_mem: usize = file.read_u64().await?.try_into()?; @@ -305,8 +305,8 @@ pub(crate) async fn restore( // Seek past low mem blob and get high mem length and offset file.seek(std::io::SeekFrom::Current(lo_mem.try_into()?)).await?; - match SnapshotTag::try_from(file.read_u8().await?) { - Ok(SnapshotTag::Himem) => {} + match SnapshotTag::from_repr(file.read_u8().await?) { + Some(SnapshotTag::Himem) => {} _ => anyhow::bail!("Expected VM config"), } let hi_mem: usize = file.read_u64().await?.try_into()?; @@ -444,7 +444,7 @@ pub struct VmGlobalState { } fn export_global(hdl: &VmmHdl) -> std::io::Result { - if hdl.api_version()? > ApiVersion::V11.into() { + if hdl.api_version()? > ApiVersion::V11 as u32 { let info = hdl.data_op(VDC_VMM_TIME, 1).read::()?; Ok(VmGlobalState { boot_hrtime: info.vt_boot_hrtime }) @@ -460,7 +460,7 @@ fn export_global(hdl: &VmmHdl) -> std::io::Result { } } fn import_global(hdl: &VmmHdl, state: &VmGlobalState) -> std::io::Result<()> { - if hdl.api_version()? > ApiVersion::V11.into() { + if hdl.api_version()? > ApiVersion::V11 as u32 { let mut info = hdl.data_op(VDC_VMM_TIME, 1).read::()?; diff --git a/bin/propolis-utils/src/bin/savex.rs b/bin/propolis-utils/src/bin/savex.rs index 2e338f878..47e4e6427 100644 --- a/bin/propolis-utils/src/bin/savex.rs +++ b/bin/propolis-utils/src/bin/savex.rs @@ -34,7 +34,8 @@ fn find_region(fp: &mut File, target: SnapshotTag) -> anyhow::Result { let mut buf = [0u8; 9]; loop { fp.read_exact(&mut buf)?; - let tag = SnapshotTag::try_from(buf[0])?; + let tag = SnapshotTag::from_repr(buf[0]) + .ok_or(anyhow::anyhow!("invalid snapshot tag"))?; // The tokio writer uses BE, apparently let len = u64::from_be_bytes(buf[1..].try_into().unwrap()); if tag == target { diff --git a/crates/bhyve-api/Cargo.toml b/crates/bhyve-api/Cargo.toml index e465f09bc..53e6e4d71 100644 --- a/crates/bhyve-api/Cargo.toml +++ b/crates/bhyve-api/Cargo.toml @@ -8,6 +8,6 @@ edition = "2021" doctest = false [dependencies] -libc.workspace = true bhyve_api_sys.workspace = true -num_enum.workspace = true +libc.workspace = true +strum = { workspace = true, features = ["derive"] } diff --git a/crates/bhyve-api/header-check/Cargo.toml b/crates/bhyve-api/header-check/Cargo.toml index 98dd50ed3..46b281fb0 100644 --- a/crates/bhyve-api/header-check/Cargo.toml +++ b/crates/bhyve-api/header-check/Cargo.toml @@ -8,6 +8,7 @@ publish = false [dependencies] bhyve_api_sys = { path = "../sys" } libc = "0.2" +strum = "0.25" [build-dependencies] cc = "1" diff --git a/crates/bhyve-api/src/lib.rs b/crates/bhyve-api/src/lib.rs index f3868af40..db3681bb8 100644 --- a/crates/bhyve-api/src/lib.rs +++ b/crates/bhyve-api/src/lib.rs @@ -11,8 +11,6 @@ use std::path::PathBuf; use std::sync::atomic::{AtomicI64, Ordering}; use std::time::Duration; -use num_enum::IntoPrimitive; - pub use bhyve_api_sys::*; pub const VMM_PATH_PREFIX: &str = "/dev/vmm"; @@ -218,7 +216,7 @@ impl VmmFd { /// Arguments: /// - `time`: Duration since `UNIX_EPOCH` pub fn rtc_settime(&self, time: Duration) -> Result<()> { - if self.api_version()? >= ApiVersion::V12.into() { + if self.api_version()? >= ApiVersion::V12 as u32 { let mut ts = libc::timespec { tv_sec: time.as_secs() as i64, tv_nsec: time.subsec_nanos() as i64, @@ -541,7 +539,6 @@ unsafe fn ioctl( /// Convenience constants to provide some documentation on what changes have /// been introduced in the various bhyve API versions. #[repr(u32)] -#[derive(IntoPrimitive)] pub enum ApiVersion { /// Add flag for exit-when-consistent as part of `VM_RUN` V15 = 15, @@ -594,6 +591,6 @@ mod test { #[test] fn latest_api_version() { let cur = ApiVersion::current(); - assert_eq!(VMM_CURRENT_INTERFACE_VERSION, cur.into()); + assert_eq!(VMM_CURRENT_INTERFACE_VERSION, cur as u32); } } diff --git a/crates/bhyve-api/sys/Cargo.toml b/crates/bhyve-api/sys/Cargo.toml index 90b2ddc00..72944cbf3 100644 --- a/crates/bhyve-api/sys/Cargo.toml +++ b/crates/bhyve-api/sys/Cargo.toml @@ -10,4 +10,4 @@ doctest = false [dependencies] libc.workspace = true -num_enum.workspace = true +strum = { workspace = true, features = ["derive"] } diff --git a/crates/bhyve-api/sys/src/enums.rs b/crates/bhyve-api/sys/src/enums.rs index 5dc295aaf..bf0b83e14 100644 --- a/crates/bhyve-api/sys/src/enums.rs +++ b/crates/bhyve-api/sys/src/enums.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use num_enum::TryFromPrimitive; +use strum::FromRepr; #[repr(C)] #[allow(non_camel_case_types, unused)] @@ -59,7 +59,7 @@ pub enum vm_reg_name { #[repr(i32)] #[allow(non_camel_case_types, unused)] -#[derive(TryFromPrimitive, Debug)] +#[derive(FromRepr, Debug)] pub enum vm_exitcode { VM_EXITCODE_INOUT, VM_EXITCODE_VMX, @@ -117,7 +117,7 @@ pub enum vm_cap_type { #[repr(u32)] #[allow(non_camel_case_types, unused)] -#[derive(TryFromPrimitive)] +#[derive(FromRepr)] pub enum vm_suspend_how { VM_SUSPEND_NONE, VM_SUSPEND_RESET, diff --git a/crates/dladm/Cargo.toml b/crates/dladm/Cargo.toml index 7800d0732..847545946 100644 --- a/crates/dladm/Cargo.toml +++ b/crates/dladm/Cargo.toml @@ -10,4 +10,4 @@ doctest = false [dependencies] libc.workspace = true -num_enum.workspace = true +strum = { workspace = true, features = ["derive"] } diff --git a/crates/dladm/src/lib.rs b/crates/dladm/src/lib.rs index 10ebacae8..272623f60 100644 --- a/crates/dladm/src/lib.rs +++ b/crates/dladm/src/lib.rs @@ -2,10 +2,6 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -extern crate libc; -extern crate num_enum; - -use std::convert::TryFrom; use std::ffi::CString; use std::io::{BufRead, BufReader, Error, ErrorKind, Result}; use std::process::{Command, Stdio}; @@ -42,17 +38,17 @@ impl Handle { ) })?; - match datalink_class::try_from(class) { - Ok(datalink_class::DATALINK_CLASS_VNIC) => { + match datalink_class::from_repr(class) { + Some(datalink_class::DATALINK_CLASS_VNIC) => { // acceptable value } - Ok(c) => { + Some(c) => { return Err(Error::new( ErrorKind::InvalidInput, format!("{} is not vnic class, but {:?}", name, c), )); } - Err(_) => { + None => { return Err(Error::new( ErrorKind::InvalidInput, format!("{} is of invalid class {:x}", name, class), @@ -121,7 +117,7 @@ impl Handle { } fn handle_dladm_err(v: i32) -> Result<()> { - match dladm_status::try_from(v) + match dladm_status::from_repr(v) .unwrap_or(dladm_status::DLADM_STATUS_FAILED) { dladm_status::DLADM_STATUS_OK => Ok(()), diff --git a/crates/dladm/src/sys.rs b/crates/dladm/src/sys.rs index 185d66761..c40d90991 100644 --- a/crates/dladm/src/sys.rs +++ b/crates/dladm/src/sys.rs @@ -3,7 +3,7 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use libc::c_int; -use num_enum::TryFromPrimitive; +use strum::FromRepr; #[cfg(target_os = "illumos")] #[link(name = "dladm")] @@ -54,7 +54,7 @@ pub enum dladm_handle {} pub type dladm_handle_t = *mut dladm_handle; pub type datalink_id_t = u32; -#[derive(Copy, Clone, Debug, Eq, PartialEq, TryFromPrimitive)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, FromRepr)] #[repr(i32)] pub enum datalink_class { DATALINK_CLASS_PHYS = 0x01, @@ -68,7 +68,7 @@ pub enum datalink_class { DATALINK_CLASS_PART = 0x100, } -#[derive(Copy, Clone, Debug, Eq, PartialEq, TryFromPrimitive)] +#[derive(Copy, Clone, Debug, Eq, PartialEq, FromRepr)] #[repr(i32)] pub enum dladm_status { DLADM_STATUS_OK = 0, diff --git a/crates/propolis-standalone-config/Cargo.toml b/crates/propolis-standalone-config/Cargo.toml index b5eadccbd..7ffd6e8ac 100644 --- a/crates/propolis-standalone-config/Cargo.toml +++ b/crates/propolis-standalone-config/Cargo.toml @@ -10,7 +10,7 @@ doctest = false [dependencies] cpuid_profile_config.workspace = true -num_enum.workspace = true serde.workspace = true serde_derive.workspace = true +strum = { workspace = true, features = ["derive"] } toml.workspace = true diff --git a/crates/propolis-standalone-config/src/lib.rs b/crates/propolis-standalone-config/src/lib.rs index b37f39b60..ce45e5456 100644 --- a/crates/propolis-standalone-config/src/lib.rs +++ b/crates/propolis-standalone-config/src/lib.rs @@ -4,12 +4,12 @@ use std::collections::BTreeMap; -use num_enum::{IntoPrimitive, TryFromPrimitive}; use serde::{Deserialize, Serialize}; +use strum::FromRepr; pub use cpuid_profile_config::*; -#[derive(TryFromPrimitive, IntoPrimitive, Eq, PartialEq)] +#[derive(FromRepr, Eq, PartialEq)] #[repr(u8)] pub enum SnapshotTag { Config = 0, diff --git a/crates/viona-api/Cargo.toml b/crates/viona-api/Cargo.toml index 4ada2764f..dd4a84972 100644 --- a/crates/viona-api/Cargo.toml +++ b/crates/viona-api/Cargo.toml @@ -10,4 +10,3 @@ doctest = false [dependencies] libc.workspace = true viona_api_sys.workspace = true -num_enum.workspace = true diff --git a/crates/viona-api/src/lib.rs b/crates/viona-api/src/lib.rs index 351f7f96e..e5f00b630 100644 --- a/crates/viona-api/src/lib.rs +++ b/crates/viona-api/src/lib.rs @@ -6,8 +6,6 @@ use std::fs::{File, OpenOptions}; use std::io::{Error, ErrorKind, Result}; use std::os::fd::*; -use num_enum::IntoPrimitive; - pub use viona_api_sys::*; pub const VIONA_DEV_PATH: &str = "/dev/viona"; @@ -107,7 +105,6 @@ unsafe fn ioctl( /// Convenience constants to provide some documentation on what changes have /// been introduced in the various viona API versions. #[repr(u32)] -#[derive(IntoPrimitive)] pub enum ApiVersion { /// Adds support for non-vnic datalink devices V2 = 2, @@ -128,6 +125,6 @@ mod test { #[test] fn latest_api_version() { let cur = ApiVersion::current(); - assert_eq!(VIONA_CURRENT_INTERFACE_VERSION, cur.into()); + assert_eq!(VIONA_CURRENT_INTERFACE_VERSION, cur as u32); } } diff --git a/lib/propolis/Cargo.toml b/lib/propolis/Cargo.toml index aec678b2a..ce9ce75df 100644 --- a/lib/propolis/Cargo.toml +++ b/lib/propolis/Cargo.toml @@ -11,8 +11,7 @@ bitflags.workspace = true bitstruct.workspace = true byteorder.workspace = true lazy_static.workspace = true -enum-iterator.workspace = true -num_enum.workspace = true +num_enum = { workspace = true, optional = true } thiserror.workspace = true bhyve_api.workspace = true dladm.workspace = true @@ -28,6 +27,7 @@ serde.workspace = true serde_arrays.workspace = true erased-serde.workspace = true serde_json.workspace = true +strum = { workspace = true, features = ["derive"] } uuid.workspace = true crucible-client-types = { workspace = true, optional = true } crucible = { workspace = true, optional = true } @@ -52,5 +52,5 @@ rand.workspace = true [features] default = [] crucible-full = ["crucible", "crucible-client-types", "oximeter", "nexus-client"] -falcon = ["libloading", "p9ds", "dlpi", "ispf", "rand", "softnpu-lib"] +falcon = ["libloading", "p9ds", "dlpi", "ispf", "rand", "softnpu-lib", "num_enum"] omicron-build = [] diff --git a/lib/propolis/src/cpuid.rs b/lib/propolis/src/cpuid.rs index b35ecda1e..7ae85fb2d 100644 --- a/lib/propolis/src/cpuid.rs +++ b/lib/propolis/src/cpuid.rs @@ -11,7 +11,6 @@ use std::num::NonZeroU8; use std::ops::Bound; use bhyve_api::vcpu_cpuid_entry; -use enum_iterator::Sequence; /// Values for a cpuid leaf #[derive(Copy, Clone, Debug)] @@ -389,7 +388,7 @@ impl Specializer { } /// Flavors of CPU topology information -#[derive(Clone, Copy, Ord, PartialOrd, Eq, PartialEq, Sequence)] +#[derive(Clone, Copy, Ord, PartialOrd, Eq, PartialEq, strum::EnumIter)] pub enum TopoKind { /// Leaf 0xB AMD (and legacy on Intel) StdB = 0xb, @@ -398,11 +397,6 @@ pub enum TopoKind { /// LEAF 0x8000001E (AMD) Ext1E = 0x8000001e, } -impl TopoKind { - pub fn all() -> enum_iterator::All { - enum_iterator::all() - } -} /// Flavors of CPU vendor for cpuid specialization #[derive(Clone, Copy)] diff --git a/lib/propolis/src/exits.rs b/lib/propolis/src/exits.rs index 7505b8bdc..98b4b92e6 100644 --- a/lib/propolis/src/exits.rs +++ b/lib/propolis/src/exits.rs @@ -4,7 +4,6 @@ //! Describes transitions from VMs to the VMM. -use std::convert::TryFrom; use std::os::raw::c_void; use bhyve_api::{ @@ -199,9 +198,9 @@ impl VmExitKind { } impl From<&vm_exit> for VmExitKind { fn from(exit: &vm_exit) -> Self { - let code = match vm_exitcode::try_from(exit.exitcode) { - Err(_) => return VmExitKind::Unknown(exit.exitcode), - Ok(c) => c, + let code = match vm_exitcode::from_repr(exit.exitcode) { + None => return VmExitKind::Unknown(exit.exitcode), + Some(c) => c, }; match code { vm_exitcode::VM_EXITCODE_BOGUS => VmExitKind::Bogus, @@ -252,18 +251,18 @@ impl From<&vm_exit> for VmExitKind { } vm_exitcode::VM_EXITCODE_SUSPENDED => { let detail = unsafe { exit.u.suspend }; - match vm_suspend_how::try_from(detail as u32) { - Ok(vm_suspend_how::VM_SUSPEND_RESET) => { + match vm_suspend_how::from_repr(detail as u32) { + Some(vm_suspend_how::VM_SUSPEND_RESET) => { VmExitKind::Suspended(Suspend::Reset) } - Ok(vm_suspend_how::VM_SUSPEND_POWEROFF) - | Ok(vm_suspend_how::VM_SUSPEND_HALT) => { + Some(vm_suspend_how::VM_SUSPEND_POWEROFF) + | Some(vm_suspend_how::VM_SUSPEND_HALT) => { VmExitKind::Suspended(Suspend::Halt) } - Ok(vm_suspend_how::VM_SUSPEND_TRIPLEFAULT) => { + Some(vm_suspend_how::VM_SUSPEND_TRIPLEFAULT) => { VmExitKind::Suspended(Suspend::TripleFault) } - Ok(vm_suspend_how::VM_SUSPEND_NONE) | Err(_) => { + Some(vm_suspend_how::VM_SUSPEND_NONE) | None => { panic!("invalid vm_suspend_how: {}", detail); } } diff --git a/lib/propolis/src/hw/pci/bar.rs b/lib/propolis/src/hw/pci/bar.rs index d01bdfb8d..46dd3e29d 100644 --- a/lib/propolis/src/hw/pci/bar.rs +++ b/lib/propolis/src/hw/pci/bar.rs @@ -314,7 +314,6 @@ pub mod migrate { #[cfg(test)] mod tests { use super::*; - use std::convert::TryFrom; fn setup() -> Bars { let bar_defs = [ @@ -371,7 +370,7 @@ mod tests { fn limits() { let mut bars = setup(); for i in 0..=5u8 { - bars.reg_write(BarN::try_from(i).unwrap(), 0xffffffff); + bars.reg_write(BarN::from_repr(i).unwrap(), 0xffffffff); } assert_eq!(bars.reg_read(BarN::BAR0), 0x0000ff01); assert_eq!(bars.reg_read(BarN::BAR1), 0xfffe0000); diff --git a/lib/propolis/src/hw/pci/mod.rs b/lib/propolis/src/hw/pci/mod.rs index 4e9afe511..5524ed78a 100644 --- a/lib/propolis/src/hw/pci/mod.rs +++ b/lib/propolis/src/hw/pci/mod.rs @@ -12,7 +12,7 @@ use std::sync::{Arc, Mutex}; use crate::common::*; use crate::intr_pins::IntrPin; -use num_enum::TryFromPrimitive; +use strum::FromRepr; pub mod bar; pub mod bits; @@ -168,9 +168,7 @@ impl Display for Bdf { } } -#[derive( - Copy, Clone, Eq, PartialEq, Debug, Ord, PartialOrd, TryFromPrimitive, -)] +#[derive(Copy, Clone, Eq, PartialEq, Debug, Ord, PartialOrd, FromRepr)] #[repr(u8)] pub enum BarN { BAR0 = 0, @@ -192,7 +190,7 @@ impl Iterator for BarIter { type Item = BarN; fn next(&mut self) -> Option { - let res = BarN::try_from(self.n).ok()?; + let res = BarN::from_repr(self.n)?; self.n += 1; Some(res) } diff --git a/lib/propolis/src/hw/virtio/viona.rs b/lib/propolis/src/hw/virtio/viona.rs index e048d9ca9..226063076 100644 --- a/lib/propolis/src/hw/virtio/viona.rs +++ b/lib/propolis/src/hw/virtio/viona.rs @@ -686,7 +686,7 @@ pub(crate) fn check_api_version() -> Result<(), crate::api_version::Error> { let vers = fd.api_version()?; // viona only requires the V2 bits for now - let compare = viona_api::ApiVersion::V2.into(); + let compare = viona_api::ApiVersion::V2 as u32; if vers < compare { Err(crate::api_version::Error::Mismatch("viona", vers, compare)) diff --git a/lib/propolis/src/vcpu.rs b/lib/propolis/src/vcpu.rs index 9a69c184b..2a04896c9 100644 --- a/lib/propolis/src/vcpu.rs +++ b/lib/propolis/src/vcpu.rs @@ -312,7 +312,7 @@ impl Vcpu { let mut entry = entry.to_raw(self.id, &mut exit); if exit_when_consistent { - if self.hdl.api_version()? >= bhyve_api::ApiVersion::V15.into() { + if self.hdl.api_version()? >= bhyve_api::ApiVersion::V15 as u32 { entry.cmd |= bhyve_api::vm_entry_cmds::VEC_FLAG_EXIT_CONSISTENT as u32; } else { diff --git a/lib/propolis/src/vmm/mod.rs b/lib/propolis/src/vmm/mod.rs index 44ee4c87c..bc7171b6a 100644 --- a/lib/propolis/src/vmm/mod.rs +++ b/lib/propolis/src/vmm/mod.rs @@ -19,7 +19,7 @@ pub(crate) fn check_api_version() -> Result<(), crate::api_version::Error> { let vers = ctl.api_version()?; // propolis only requires the bits provided by V8, currently - let compare = bhyve_api::ApiVersion::V8.into(); + let compare = bhyve_api::ApiVersion::V8 as u32; if vers < compare { return Err(crate::api_version::Error::Mismatch("vmm", vers, compare)); diff --git a/phd-tests/framework/src/host_api/kvm.rs b/phd-tests/framework/src/host_api/kvm.rs index a89071f63..963ad4655 100644 --- a/phd-tests/framework/src/host_api/kvm.rs +++ b/phd-tests/framework/src/host_api/kvm.rs @@ -304,14 +304,14 @@ pub fn set_vmm_globals() -> Result>> { let ver = bhyve_api::api_version()?; - if ver < ApiVersion::V13.into() { + if ver < ApiVersion::V13 as u32 { guards.push(Box::new(KernelValueGuard::new( "vmm_allow_state_writes", 1u32, )?)); } - if ver < ApiVersion::V8.into() { + if ver < ApiVersion::V8 as u32 { // Enable global dirty tracking bit on systems where it exists. if let Ok(gpt_track_dirty) = KernelValueGuard::new("gpt_track_dirty", 1u8) diff --git a/xtask/src/task_clippy.rs b/xtask/src/task_clippy.rs index 9857bff46..88a942373 100644 --- a/xtask/src/task_clippy.rs +++ b/xtask/src/task_clippy.rs @@ -43,6 +43,9 @@ pub(crate) fn cmd_clippy(strict: bool) -> Result<()> { failed |= run_clippy(&["-p", "propolis-standalone", "--features", "crucible"])?; + // Check PHD bits + failed |= run_clippy(&["-p", "phd-runner"])?; + if failed { bail!("Clippy failures detected") }