From 5f79105e7613a0900dc4892f51b2ad270aed26e6 Mon Sep 17 00:00:00 2001 From: Patrick Mooney Date: Mon, 4 Dec 2023 13:07:27 -0600 Subject: [PATCH] Do not require casting for API version cmp --- bin/propolis-standalone/src/snapshot.rs | 4 ++-- crates/bhyve-api/src/lib.rs | 19 +++++++++++++++++++ crates/viona-api/src/lib.rs | 18 ++++++++++++++++++ lib/propolis/src/exits.rs | 4 ++-- lib/propolis/src/hw/virtio/viona.rs | 4 ++-- lib/propolis/src/vcpu.rs | 6 +++--- lib/propolis/src/vmm/mod.rs | 4 ++-- phd-tests/framework/src/host_api/kvm.rs | 4 ++-- 8 files changed, 50 insertions(+), 13 deletions(-) diff --git a/bin/propolis-standalone/src/snapshot.rs b/bin/propolis-standalone/src/snapshot.rs index 4d4b6d335..52a77225a 100644 --- a/bin/propolis-standalone/src/snapshot.rs +++ b/bin/propolis-standalone/src/snapshot.rs @@ -444,7 +444,7 @@ pub struct VmGlobalState { } fn export_global(hdl: &VmmHdl) -> std::io::Result { - if hdl.api_version()? > ApiVersion::V11 as u32 { + if hdl.api_version()? > ApiVersion::V11 { 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 as u32 { + if hdl.api_version()? > ApiVersion::V11 { let mut info = hdl.data_op(VDC_VMM_TIME, 1).read::()?; diff --git a/crates/bhyve-api/src/lib.rs b/crates/bhyve-api/src/lib.rs index 99efe55e2..0f9fcc38a 100644 --- a/crates/bhyve-api/src/lib.rs +++ b/crates/bhyve-api/src/lib.rs @@ -540,6 +540,7 @@ unsafe fn ioctl( /// Convenience constants to provide some documentation on what changes have /// been introduced in the various bhyve API versions. #[repr(u32)] +#[derive(Copy, Clone)] pub enum ApiVersion { /// VM Suspend behavior reworked, `VM_VCPU_BARRIER` ioctl added V16 = 16, @@ -588,6 +589,17 @@ impl ApiVersion { } } +impl PartialEq for u32 { + fn eq(&self, other: &ApiVersion) -> bool { + *self == *other as u32 + } +} +impl PartialOrd for u32 { + fn partial_cmp(&self, other: &ApiVersion) -> Option { + Some(self.cmp(&(*other as u32))) + } +} + #[cfg(test)] mod test { use super::*; @@ -597,4 +609,11 @@ mod test { let cur = ApiVersion::current(); assert_eq!(VMM_CURRENT_INTERFACE_VERSION, cur as u32); } + + #[test] + fn u32_comparisons() { + assert!(4u32 < ApiVersion::V5); + assert!(5u32 == ApiVersion::V5); + assert!(6u32 > ApiVersion::V5); + } } diff --git a/crates/viona-api/src/lib.rs b/crates/viona-api/src/lib.rs index e5f00b630..0f6b44fb0 100644 --- a/crates/viona-api/src/lib.rs +++ b/crates/viona-api/src/lib.rs @@ -105,6 +105,7 @@ unsafe fn ioctl( /// Convenience constants to provide some documentation on what changes have /// been introduced in the various viona API versions. #[repr(u32)] +#[derive(Copy, Clone)] pub enum ApiVersion { /// Adds support for non-vnic datalink devices V2 = 2, @@ -117,6 +118,16 @@ impl ApiVersion { Self::V2 } } +impl PartialEq for u32 { + fn eq(&self, other: &ApiVersion) -> bool { + *self == *other as u32 + } +} +impl PartialOrd for u32 { + fn partial_cmp(&self, other: &ApiVersion) -> Option { + Some(self.cmp(&(*other as u32))) + } +} #[cfg(test)] mod test { @@ -127,4 +138,11 @@ mod test { let cur = ApiVersion::current(); assert_eq!(VIONA_CURRENT_INTERFACE_VERSION, cur as u32); } + + #[test] + fn u32_comparisons() { + assert!(1u32 < ApiVersion::V2); + assert!(2u32 == ApiVersion::V2); + assert!(3u32 > ApiVersion::V2); + } } diff --git a/lib/propolis/src/exits.rs b/lib/propolis/src/exits.rs index fa49ae1bb..bc3b79dde 100644 --- a/lib/propolis/src/exits.rs +++ b/lib/propolis/src/exits.rs @@ -212,7 +212,7 @@ impl VmExitKind { vm_exitcode::VM_EXITCODE_DEPRECATED2 => { // Prior to v16, this was REQIDLE, which can be translated into // a BOGUS exit. - if api_version < bhyve_api::ApiVersion::V16 as u32 { + if api_version < bhyve_api::ApiVersion::V16 { VmExitKind::Bogus } else { // At or after v16, we do not expect to see this code @@ -268,7 +268,7 @@ impl VmExitKind { // Prior to v16, the only field in vm_exit.u.suspend was `how`. // The `source` and `when` fields are valid in v16 or later. let valid_detail = - api_version >= bhyve_api::ApiVersion::V16 as u32; + api_version >= bhyve_api::ApiVersion::V16; let kind = match vm_suspend_how::from_repr(detail.how as u32) { Some(vm_suspend_how::VM_SUSPEND_RESET) => Suspend::Reset, Some(vm_suspend_how::VM_SUSPEND_POWEROFF) diff --git a/lib/propolis/src/hw/virtio/viona.rs b/lib/propolis/src/hw/virtio/viona.rs index 226063076..3ac14b851 100644 --- a/lib/propolis/src/hw/virtio/viona.rs +++ b/lib/propolis/src/hw/virtio/viona.rs @@ -686,10 +686,10 @@ 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 as u32; + let compare = viona_api::ApiVersion::V2; if vers < compare { - Err(crate::api_version::Error::Mismatch("viona", vers, compare)) + Err(crate::api_version::Error::Mismatch("viona", vers, compare as u32)) } else { Ok(()) } diff --git a/lib/propolis/src/vcpu.rs b/lib/propolis/src/vcpu.rs index 3653a6218..07a38d578 100644 --- a/lib/propolis/src/vcpu.rs +++ b/lib/propolis/src/vcpu.rs @@ -316,7 +316,7 @@ impl Vcpu { let api_version = self.hdl.api_version()?; if exit_when_consistent { - if api_version >= ApiVersion::V15 as u32 { + if api_version >= ApiVersion::V15 { entry.cmd |= bhyve_api::vm_entry_cmds::VEC_FLAG_EXIT_CONSISTENT as u32; } else { @@ -334,7 +334,7 @@ impl Vcpu { /// Issue a "barrier" for the vCPU, forcing an exit from guest context pub fn barrier(&self) -> Result<()> { - if self.hdl.api_version()? >= ApiVersion::V16 as u32 { + if self.hdl.api_version()? >= ApiVersion::V16 { // Use the official barrier operation, if available self.hdl .ioctl_usize(bhyve_api::VM_VCPU_BARRIER, self.id as usize)?; @@ -952,7 +952,7 @@ pub mod migrate { // When hosts with illumos#15143 integrated become common, the // overall required version for propolis can grow to encompass V10 // and this check can be elided. - if bhyve_api::api_version()? >= ApiVersion::V10 as u32 { + if bhyve_api::api_version()? >= ApiVersion::V10 { vcpu.hdl .data_op(bhyve_api::VDC_VMM_ARCH, 1) .for_vcpu(vcpu.id) diff --git a/lib/propolis/src/vmm/mod.rs b/lib/propolis/src/vmm/mod.rs index bc7171b6a..2a10ee1ba 100644 --- a/lib/propolis/src/vmm/mod.rs +++ b/lib/propolis/src/vmm/mod.rs @@ -19,10 +19,10 @@ 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 as u32; + let compare = bhyve_api::ApiVersion::V8; if vers < compare { - return Err(crate::api_version::Error::Mismatch("vmm", vers, compare)); + return Err(crate::api_version::Error::Mismatch("vmm", vers, compare as u32)); } Ok(()) diff --git a/phd-tests/framework/src/host_api/kvm.rs b/phd-tests/framework/src/host_api/kvm.rs index 963ad4655..c0ca3c142 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 as u32 { + if ver < ApiVersion::V13 { guards.push(Box::new(KernelValueGuard::new( "vmm_allow_state_writes", 1u32, )?)); } - if ver < ApiVersion::V8 as u32 { + if ver < ApiVersion::V8 { // Enable global dirty tracking bit on systems where it exists. if let Ok(gpt_track_dirty) = KernelValueGuard::new("gpt_track_dirty", 1u8)