Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

enum: Replacing current type conversion by enums #9

Closed
wants to merge 1 commit into from
Closed

enum: Replacing current type conversion by enums #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions examples/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ extern crate virt;

use std::{env, io};

use virt::connect::{Connect, ConnectCredential, ConnectAuth};
use virt::connect::{Connect, ConnectCredential, ConnectAuth, ConnectCredentialType};

fn main() {
let uri = match env::args().nth(1) {
Expand All @@ -55,11 +55,11 @@ fn main() {

println!("{}:", cred.prompt);
match cred.typed {
::virt::connect::VIR_CRED_AUTHNAME => {
ConnectCredentialType::Authname => {
io::stdin().read_line(&mut input).expect("");
cred.result = Some(String::from(input.trim()));
}
::virt::connect::VIR_CRED_PASSPHRASE => {
ConnectCredentialType::Passphrase => {
io::stdin().read_line(&mut input).expect("");
cred.result = Some(String::from(input.trim()));
}
Expand All @@ -69,8 +69,8 @@ fn main() {
}
}
};
let mut auth = ConnectAuth::new(vec![::virt::connect::VIR_CRED_AUTHNAME,
::virt::connect::VIR_CRED_PASSPHRASE],
let mut auth = ConnectAuth::new(vec![ConnectCredentialType::Authname,
ConnectCredentialType::Passphrase],
callback);

println!("Attempting to connect to hypervisor: '{}'...", uri);
Expand Down
3 changes: 2 additions & 1 deletion examples/hello.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use std::env;

use virt::connect::Connect;
use virt::error::Error;
use virt::domain::DomainNumatuneMemMode;


fn show_hypervisor_info(conn: &Connect) -> Result<(), Error> {
Expand Down Expand Up @@ -87,7 +88,7 @@ fn show_domains(conn: &Connect) -> Result<(), Error> {
println!("NUMA:");
println!(" Node Set: {}",
numa.node_set.unwrap_or(String::from("")));
println!(" Mode: {}", numa.mode.unwrap_or(0));
println!(" Mode: {}", numa.mode.unwrap_or(DomainNumatuneMemMode::Strict));
}
}
}
Expand Down
39 changes: 26 additions & 13 deletions src/connect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,16 +338,28 @@ pub type BaselineCPUFlags = self::libc::c_int;
pub const VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES: BaselineCPUFlags = (1 << 0);
pub const VIR_CONNECT_BASELINE_CPU_MIGRATABLE: BaselineCPUFlags = (1 << 1);

pub type ConnectCredentialType = self::libc::c_int;
pub const VIR_CRED_USERNAME: ConnectCredentialType = 1;
pub const VIR_CRED_AUTHNAME: ConnectCredentialType = 2;
pub const VIR_CRED_LANGUAGE: ConnectCredentialType = 3;
pub const VIR_CRED_CNONCE: ConnectCredentialType = 4;
pub const VIR_CRED_PASSPHRASE: ConnectCredentialType = 5;
pub const VIR_CRED_ECHOPROMPT: ConnectCredentialType = 6;
pub const VIR_CRED_NOECHOPROMPT: ConnectCredentialType = 7;
pub const VIR_CRED_REALM: ConnectCredentialType = 8;
pub const VIR_CRED_EXTERNAL: ConnectCredentialType = 9;
virt_enum! {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a very nice work but I dont think we should use a macro. We don't have so much constants to convert so we should implement them correctly using an enum.

Then about to do the conversion I'm thinkin about using Procedural Macros so perhaps at the end to have something like:

#[Derive(ConvertToC)]
pub enum Something {
  Value1 = 1,
  Value2 = 2,
  ...
}

Also it's very nice to have started documenting the fields, if you really want do that, the best is to reuse what we have in the header of libvirt, like: http://libvirt.org/git/?p=libvirt.git;a=blob;f=include/libvirt/libvirt-domain.h;h=45f939a8cc37e8e29bea13316a222338b53774fe;hb=HEAD#l291

Also a last question what about to keep the name in uppercase?

#[Derive(ConvertToC)]
pub enum Something {
  VALUE1 = 1,
  VALUE1 = 2,
  ...
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also work.

#[repr(C)]
pub enum Test {
  Value = 1
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how I would see the thing:

https://github.com/sahid/libvirt-rs/blob/poc/enum/src/error.rs#L45

What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be great.

Copy link
Author

@farodin91 farodin91 Jun 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combined solution would be wrapping these into an macro to auto create From traits until derive is implemented.

impl_enum!{
  #[derive(Debug, PartialEq)]
  pub enum ErrorLevel {
    /// No error
    None = 0,
    /// A simple warning.
    Warning = 1,
    /// An error.
    Error = 2,
  }
}

Normally enums in Rust are CamelCase.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/guide.md#enums

Copy link
Owner

@sahid sahid Jun 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is we should do it by procedural macro and so use derive or we should do it by hands (it's not all the constants that needs to be converted from).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@farodin91 please let me know if you still want to continue to work on this issue, if not I could take the lead :)

Thanks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could take it.

ConnectCredentialType {
/// Username
Username -> 1,
/// Authname
Authname -> 2,
/// Language
Language -> 3,
/// Cnonce
Cnonce -> 4,
/// Passphrase
Passphrase -> 5,
/// Echoprompt
Echoprompt -> 6,
/// Noechoprompt
Noechoprompt -> 7,
/// Realm
Realm -> 8,
/// External
External -> 9,
}
}

#[derive(Clone, Debug)]
pub struct NodeInfo {
Expand Down Expand Up @@ -380,7 +392,7 @@ pub type ConnectAuthCallback = fn(creds: &mut Vec<ConnectCredential>);
#[derive(Clone, Debug)]
pub struct ConnectCredential {
/// One of `ConnectCredentialType` constants
pub typed: i32,
pub typed: ConnectCredentialType,
/// Prompt to show to user.
pub prompt: String,
/// Additional challenge to show.
Expand All @@ -399,7 +411,7 @@ impl ConnectCredential {
default = c_chars_to_string!((*cred).defresult, nofree);
}
ConnectCredential {
typed: (*cred).typed,
typed: (*cred).typed.into(),
prompt: c_chars_to_string!((*cred).prompt, nofree),
challenge: c_chars_to_string!((*cred).challenge, nofree),
def_result: default,
Expand Down Expand Up @@ -448,8 +460,9 @@ impl ConnectAuth {
}

unsafe {
let mut cred = self.creds[0].clone().into();
sys::virConnectAuth {
credtype: &mut self.creds[0],
credtype: &mut cred,
ncredtype: self.creds.len() as u32,
cb: wrapper,
cbdata: mem::transmute(self.callback),
Expand Down
82 changes: 53 additions & 29 deletions src/domain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,18 @@ extern "C" {
-> libc::c_int;
}

pub type DomainXMLFlags = self::libc::c_uint;
pub const VIR_DOMAIN_XML_SECURE: DomainXMLFlags = 1 << 0;
pub const VIR_DOMAIN_XML_INACTIVE: DomainXMLFlags = 1 << 1;
pub const VIR_DOMAIN_XML_UPDATE_CPU: DomainXMLFlags = 1 << 2;
pub const VIR_DOMAIN_XML_MIGRATABLE: DomainXMLFlags = 1 << 3;
virt_enum! {
DomainXMLFlags {
/// Secure
Secure -> 1,
/// Inactive
Inactive -> 2,
/// UpdateCpu
UpdateCpu -> 4,
/// Migratable
Migratable -> 8,
}
}

pub type DomainCreateFlags = self::libc::c_uint;
pub const VIR_DOMAIN_NONE: DomainCreateFlags = 0;
Expand Down Expand Up @@ -445,20 +452,37 @@ pub const VIR_DOMAIN_SAVE_BYPASS_CACHE: DomainSaveRestoreFlags = 1 << 0;
pub const VIR_DOMAIN_SAVE_RUNNING: DomainSaveRestoreFlags = 1 << 1;
pub const VIR_DOMAIN_SAVE_PAUSED: DomainSaveRestoreFlags = 1 << 2;

pub type DomainNumatuneMemMode = self::libc::c_int;
pub const VIR_DOMAIN_NUMATUNE_MEM_STRICT: DomainNumatuneMemMode = 0;
pub const VIR_DOMAIN_NUMATUNE_MEM_PREFERRED: DomainNumatuneMemMode = 1;
pub const VIR_DOMAIN_NUMATUNE_MEM_INTERLEAVE: DomainNumatuneMemMode = 2;

pub type DomainState = self::libc::c_uint;
pub const VIR_DOMAIN_NOSTATE: DomainState = 0;
pub const VIR_DOMAIN_RUNNING: DomainState = 1;
pub const VIR_DOMAIN_BLOCKED: DomainState = 2;
pub const VIR_DOMAIN_PAUSED: DomainState = 3;
pub const VIR_DOMAIN_SHUTDOWN: DomainState = 4;
pub const VIR_DOMAIN_SHUTOFF: DomainState = 5;
pub const VIR_DOMAIN_CRASHED: DomainState = 6;
pub const VIR_DOMAIN_PMSUSPENDED: DomainState = 7;
virt_enum! {
DomainNumatuneMemMode {
/// Strict
Strict -> 0,
/// Preferred
Preferred -> 1,
/// Interleave
Interleave -> 2,
}
}

virt_enum! {
DomainState {
/// Nostate
Nostate -> 0,
/// Running
Running -> 1,
/// Blocked
Blocked -> 2,
/// Paused
Paused -> 3,
/// Shutdown
Shutdown -> 4,
/// Shutoff
Shutoff -> 5,
/// Crashed
Crashed -> 6,
/// PmSuspended
PmSuspended -> 7,
}
}

#[derive(Clone, Debug)]
pub struct DomainInfo {
Expand All @@ -478,7 +502,7 @@ impl DomainInfo {
pub fn from_ptr(ptr: sys::virDomainInfoPtr) -> DomainInfo {
unsafe {
DomainInfo {
state: (*ptr).state as DomainState,
state: ((*ptr).state as u32).into(),
max_mem: (*ptr).maxMem as u64,
memory: (*ptr).memory as u64,
nr_virt_cpu: (*ptr).nrVirtCpu as u32,
Expand Down Expand Up @@ -568,7 +592,7 @@ impl NUMAParameters {
"numa_nodeset" => {
ret.node_set = Some(c_chars_to_string!(param.value as *mut libc::c_char))
}
"numa_mode" => ret.mode = Some(param.value as libc::c_int),
"numa_mode" => ret.mode = Some(DomainNumatuneMemMode::from(param.value as libc::c_int)),
unknow => panic!("Field not implemented for NUMAParameters, {:?}", unknow),
}
}
Expand Down Expand Up @@ -704,7 +728,7 @@ impl Domain {
if ret == -1 {
return Err(Error::new());
}
return Ok((state as DomainState, reason as i32));
return Ok(((state as u32).into(), reason as i32));
}
}

Expand Down Expand Up @@ -1683,28 +1707,28 @@ impl Domain {
if params.hard_limit.is_some() {
cparams.push(virTypedParameter {
field: to_arr("hard_limit\0"),
typed: ::typedparam::VIR_TYPED_PARAM_ULLONG,
typed: ::typedparam::TypedParameterType::Ullong.into(),
value: params.hard_limit.unwrap(),
})
}
if params.soft_limit.is_some() {
cparams.push(virTypedParameter {
field: to_arr("soft_limit\0"),
typed: ::typedparam::VIR_TYPED_PARAM_ULLONG,
typed: ::typedparam::TypedParameterType::Ullong.into(),
value: params.soft_limit.unwrap(),
})
}
if params.min_guarantee.is_some() {
cparams.push(virTypedParameter {
field: to_arr("min_guarantee\0"),
typed: ::typedparam::VIR_TYPED_PARAM_ULLONG,
typed: ::typedparam::TypedParameterType::Ullong.into(),
value: params.min_guarantee.unwrap(),
})
}
if params.swap_hard_limit.is_some() {
cparams.push(virTypedParameter {
field: to_arr("swap_hard_limit\0"),
typed: ::typedparam::VIR_TYPED_PARAM_ULLONG,
typed: ::typedparam::TypedParameterType::Ullong.into(),
value: params.swap_hard_limit.unwrap(),
})
}
Expand Down Expand Up @@ -1836,15 +1860,15 @@ impl Domain {
if params.node_set.is_some() {
cparams.push(virTypedParameter {
field: to_arr("numa_nodeset\0"),
typed: ::typedparam::VIR_TYPED_PARAM_STRING,
typed: ::typedparam::TypedParameterType::String.into(),
value: string_to_mut_c_chars!(params.node_set.unwrap()) as u64,
})
}
if params.mode.is_some() {
cparams.push(virTypedParameter {
field: to_arr("numa_mode\0"),
typed: ::typedparam::VIR_TYPED_PARAM_INT,
value: params.mode.unwrap() as u64,
typed: ::typedparam::TypedParameterType::Int.into(),
value: params.mode.unwrap().into(),
})
}

Expand Down
24 changes: 9 additions & 15 deletions src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,14 @@ extern "C" {
fn virGetLastError() -> sys::virErrorPtr;
}

#[derive(Debug, PartialEq)]
pub enum ErrorLevel {
None,
Warning,
Error,
}

impl ErrorLevel {
pub fn new(level: u32) -> ErrorLevel {
match level {
0 => ErrorLevel::None,
1 => ErrorLevel::Warning,
_ => ErrorLevel::Error,
}
virt_enum! {
ErrorLevel {
/// None
None -> 0,
/// Warning
Warning -> 1,
/// Error
Error -> 2,
}
}

Expand All @@ -78,7 +72,7 @@ impl Error {
code: (*ptr).code,
domain: (*ptr).domain,
message: c_chars_to_string!((*ptr).message, nofree),
level: ErrorLevel::new((*ptr).level as u32),
level: ((*ptr).level as u32).into(),
}
}
}
Expand Down
75 changes: 75 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,81 @@ macro_rules! string_to_mut_c_chars {
($x:expr) => (::std::ffi::CString::new($x).unwrap().into_raw())
}

macro_rules! virt_enum {
($name:ident { $(#[$meta:meta] $tag:ident -> $value:expr),*, }) => (virt_enum!($name { $(#[$meta] $tag -> $value),* }););
($name:ident { $(#[$meta:meta] $tag:ident -> $value:expr),* }) => {

#[derive(Clone, Debug, PartialEq)]
pub enum $name {
$(#[$meta]
$tag),*,
Custom(u32),
}

impl ::std::fmt::Display for $name {
fn fmt(&self, f: &mut ::std::fmt::Formatter) -> Result<(), ::std::fmt::Error> {
write!(f, "{:?}", self)
}
}

impl From<u32> for $name {
fn from(flag: u32) -> Self {
match flag {
$($value => $name::$tag),*,
s => $name::Custom(s),
}
}
}

impl From<$name> for u32 {
fn from(flag: $name) -> Self {
match flag {
$($name::$tag => $value),*,
$name::Custom(s) => s,
}
}
}

impl From<i32> for $name {
fn from(flag: i32) -> Self {
match flag as u32 {
$($value => $name::$tag),*,
s => $name::Custom(s),
}
}
}

impl From<$name> for i32 {
fn from(flag: $name) -> Self {
let flag = match flag {
$($name::$tag => $value),*,
$name::Custom(s) => s,
};
flag as i32
}
}

impl From<u64> for $name {
fn from(flag: u64) -> Self {
match flag as u32 {
$($value => $name::$tag),*,
s => $name::Custom(s),
}
}
}

impl From<$name> for u64 {
fn from(flag: $name) -> Self {
let flag = match flag {
$($name::$tag => $value),*,
$name::Custom(s) => s,
};
flag as u64
}
}
};
}

pub mod typedparam;
pub mod connect;
pub mod domain;
Expand Down
Loading