Skip to content

Commit

Permalink
Do not require casting for API version cmp
Browse files Browse the repository at this point in the history
  • Loading branch information
pfmooney committed Dec 4, 2023
1 parent da5ac5a commit 5f79105
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 13 deletions.
4 changes: 2 additions & 2 deletions bin/propolis-standalone/src/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ pub struct VmGlobalState {
}

fn export_global(hdl: &VmmHdl) -> std::io::Result<VmGlobalState> {
if hdl.api_version()? > ApiVersion::V11 as u32 {
if hdl.api_version()? > ApiVersion::V11 {
let info = hdl.data_op(VDC_VMM_TIME, 1).read::<vdi_time_info_v1>()?;

Ok(VmGlobalState { boot_hrtime: info.vt_boot_hrtime })
Expand All @@ -460,7 +460,7 @@ fn export_global(hdl: &VmmHdl) -> std::io::Result<VmGlobalState> {
}
}
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::<vdi_time_info_v1>()?;

Expand Down
19 changes: 19 additions & 0 deletions crates/bhyve-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -588,6 +589,17 @@ impl ApiVersion {
}
}

impl PartialEq<ApiVersion> for u32 {
fn eq(&self, other: &ApiVersion) -> bool {
*self == *other as u32
}
}
impl PartialOrd<ApiVersion> for u32 {
fn partial_cmp(&self, other: &ApiVersion) -> Option<std::cmp::Ordering> {
Some(self.cmp(&(*other as u32)))
}
}

#[cfg(test)]
mod test {
use super::*;
Expand All @@ -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);
}
}
18 changes: 18 additions & 0 deletions crates/viona-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -117,6 +118,16 @@ impl ApiVersion {
Self::V2
}
}
impl PartialEq<ApiVersion> for u32 {
fn eq(&self, other: &ApiVersion) -> bool {
*self == *other as u32
}
}
impl PartialOrd<ApiVersion> for u32 {
fn partial_cmp(&self, other: &ApiVersion) -> Option<std::cmp::Ordering> {
Some(self.cmp(&(*other as u32)))
}
}

#[cfg(test)]
mod test {
Expand All @@ -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);
}
}
4 changes: 2 additions & 2 deletions lib/propolis/src/exits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/propolis/src/hw/virtio/viona.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
6 changes: 3 additions & 3 deletions lib/propolis/src/vcpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)?;
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/propolis/src/vmm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
4 changes: 2 additions & 2 deletions phd-tests/framework/src/host_api/kvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,14 @@ pub fn set_vmm_globals() -> Result<Vec<Box<dyn std::any::Any>>> {

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)
Expand Down

0 comments on commit 5f79105

Please sign in to comment.