Design: Driver initialization argument handling #197

Wenzel · 2021-05-19T07:38:37Z

Opening a design issue to address the issue of passing initialization parameters to drivers.

Currently the library assumes that a domain_name parameter is required
https://github.com/Wenzel/libmicrovmi/blob/master/src/lib.rs#L30

and additional parameters can be passed via Option<DriverInitParam>.

adding Memflow comes with new requirements

The issue comes from integrating Memflow.

Memflow has multiple connectors to choose from

qemu_procfs
kvm
pcileech
coredump

each one having a different set of parameters:

qemu_procfs
- required: none (will select the first qemu process)
- optional: vm_name
kvm
- required: pid
pcileech
- required: device, default value: FPGA
- optional: memap, path to a file
coredump
- required: filepath

also, we might want to initialize the memflow driver without specifying the connector, and let it scan and select the appropriate connector.

Issue with the current implementation

we require a domain_name even though it might no be necessary with memflow
not related to memflow, but DriverInitParam can contain only a single enum for a single driver. It's impossible to pass multiple init args for multiple driver. (eg pass --kvmi-unix-socket=xxx and --memflow-param-xxx=yyy on the command line, in case one or the other is found)

Proposed solution

I'm in favor a removing the domain_name / DriverInitParam and switching to a HashMap<str, str> for connector arguments.
This solves both issues

passing a domain_name is not required
you can pack multiple init args for multiple drivers in the hashmap

Looking at Memflow's own implementation of ConnectorArgs, they wrapped a HashMap.
Also they added a convenient way to set a default value.

Question now, if we go this way, is how to reflect from the examples the arguments that each driver can take (required and optional) ?
Ideally, each driver would explain what its requirements are.

The text was updated successfully, but these errors were encountered:

Wenzel · 2021-06-15T12:44:36Z

An idea to allow each driver to describe what possible initialization parameters it can take:

api.rs

/// Describe a driver initialization parameter
#[derive(Debug)]
pub struct DriverInitParamDesc {
    name: String,
    description: String,
    required: bool,
}

pub trait Introspectable {
    /// Retrieve the possible initialization arguments for this Driver
    fn get_possible_args() -> Vec<DriverInitParam> where Self: Sized {
        // default: no parameters required
        Vec::new()
    }
}

lib.rs

fn possible_init_args() -> HashMap<DriverType, Vec<DriverInitParamDesc> {
   let map = HashMap::new();
    for drv_type in DriverType::into_enum_iter() {
         match drv_type {
              // pseudo code
              DriverType::KVM => {
                  map.insert(drv_type, KVM::get_possible_args())
              }
         }
    }
    map
}

On the user side:
example.rs

use microvmi::api::DriverParams;
use microvmi::{init_driver, possible_init_args}

fn main() {
	if ("--help") {
            // get all possible init args, to display them for --help usage
	    let poss_args = possible_init_args();
	    // display them
	    // for x in poss_args.iter() .... 
            //     x.display()
            return;
	}
	
	// prepare driver init
	let init_params = DriverParams::with_default("windows_10")
	    // add arg for KVM driver
            .insert("kvmi_unix_socket", "/tmp/introspector")
	    // add arg for memflow driver
	    .insert("memflow_pid", "45617")
	let driver = init_driver(None, &init_params)
}

rageagainsthepc · 2021-06-16T10:23:31Z

The idea looks good to me. I have one small note though: Why use a hash map? Wouldn't it be nicer to use a complex enum where each enum variant wraps an individual struct for those driver init params?

Wenzel · 2021-06-17T13:25:34Z

I like enums because their a give a strong type definition instead of a losely defined string that wouldn't be checked at compile-time.

However, it would be difficilt to express the init parameter combinatory with fully qualified enums:

Lets assume we want to initialize by passing an enum:

pub enum XenInitParams {
    // Xen can be initialized with a vm_name
    Default { vm_name: String },
}

use microvmi::init_driver;
use microvmi::api::XenInitParams;

let init_params  = XenInitParams::Default(String::from("Windows10"));
let driver = init_driver(None, &init_params);

The situation get less intuitive for KVM:

// both of the enum variant will have to take a vm_name mandatory field
// not very clear from an API user point of view, but okay
pub enum KVMInitParams {
	UnixSocket {vm_name: String, socket_path: String },
	VSock {vm_name: String, port: u32 },
}

But the problem arises with memflow.
Remember that a memflow driver can be initialized like this:

no connector name, no arguments
no connector name, arguments
connector name, no arguments
connector name, arguments

and each connector has a set of specific arguments:

qemu_procfs
- required: none (will select the first qemu process)
- optional: vm_name
kvm
- required: pid
pcileech
- required: device, default value: FPGA
- optional: memap, path to a file
coredump
- required: filepath

We could sketch an enum like this:

pub enum MemflowInitParams {
	Default { connector_name: Option<String>, connector_args: Option<HashMap<str, str>>},
}

but it loosely defines what parameters are actually accepted by the memflow driver.
Also, another issue here is that we can only pass one enum at init, and we forbid the possibility of supplying multiple initialization parameters for multiple drivers at the same time.

If I read from a configuration file, and I want to initialize KVM with a given unix socket and memflow with a QEMU pid, depending on each one of them will be detected at runtime, i would need to express that possibility.

Wenzel · 2021-06-17T13:30:12Z

Update: the memflow connector_args could be rewritten with a more expressive enum like this:

pub enum MemflowConnectorParams {
	// optional vm_name, otherwise will search for the first QEMU process
	QEMUProcFs { vm_name: Option<String>},
	KVM { pid: u32 },
	// default value for device: "FPGA"
	PCILeech { device: Option<String>, memmap: Option<Path> },
	Coredump { filepath: Path },
}

pub enum MemflowInitParams {
	Default { connector_name: Option<String>, connector_args: Option<MemflowConnectorParams>},
}

Wenzel · 2021-06-17T14:01:23Z

Update, thinking again on your proposal, to reflect the possibility of passing mutiple initialization parameters for multiple drivers:

use std::path::PathBuf;

/// Describes Xen initialization parameters
pub enum XenInitParams {
    Default { vm_name: String },
}

/// Describes KVM initialization parameters
pub enum KVMInitParams {
    UnixSocket {
        vm_name: String,
        socket_path: PathBuf,
    },
    VSock {
        vm_name: String,
        port: u32,
    },
}

/// Describes Memflow connector parameters
pub enum MemflowConnectorParams {
    // optional vm_name, otherwise will search for the first QEMU process
    QEMUProcFs {
        vm_name: Option<String>,
    },
    KVM {
        pid: u32,
    },
    // default value for device: "FPGA"
    PCILeech {
        device: Option<String>,
        memmap: Option<PathBuf>,
    },
    Coredump {
        filepath: PathBuf,
    },
}


/// Describes Memflow initialization parameters
pub enum MemflowInitParams {
    Default {
        connector_name: Option<String>,
        connector_args: Option<MemflowConnectorParams>,
    },
}

/// This struct is used to pass all initialization parameters for all drivers
/// when calling init_driver()
#[derive(Default)]
pub struct MicrovmiInitParams {
    xen: Option<XenInitParams>,
    kvm: Option<KVMInitParams>,
    memflow: Option<MemflowInitParams>,
}

fn main() {
    let init_params_kvm = KVMInitParams::UnixSocket {
        vm_name: String::from("windows10"),
        socket_path: PathBuf::from(r"/tmp/introspector"),
    };
    let init_params_xen = XenInitParams::Default {
        vm_name: String::from("windows10"),
    };
    let init_params = MicrovmiInitParams {
        kvm: Some(init_params_kvm),
        xen: Some(init_params_xen),
        ..Default::default()
    };
    let driver = init_driver(None, init_params);
}

Wenzel · 2021-06-17T14:15:07Z

Update: we can pack the common parameters inside the MicrovmiInitParams struct:

use std::error::Error;
use std::path::PathBuf;

pub enum DriverType {}

pub fn init_driver(
    driver_type: Option<DriverType>,
    driver_init_params: Option<MicrovmiInitParams>,
) -> Result<(), Box<dyn Error>> {
    unimplemented!();
}

/// Describes Xen initialization parameters
pub enum XenInitParams {}

/// Describes KVM initialization parameters
pub enum KVMInitParams {
    UnixSocket { socket_path: PathBuf },
    VSock { port: u32 },
}

/// Describes Memflow connector parameters
pub enum MemflowConnectorParams {
    // optional vm_name, otherwise will search for the first QEMU process
    QEMUProcFs {
        vm_name: Option<String>,
    },
    KVM {
        pid: u32,
    },
    // default value for device: "FPGA"
    PCILeech {
        device: Option<String>,
        memmap: Option<PathBuf>,
    },
    Coredump {
        filepath: PathBuf,
    },
}

/// Describes Memflow initialization parameters
pub enum MemflowInitParams {
    Default {
        connector_name: Option<String>,
        connector_args: Option<MemflowConnectorParams>,
    },
}

pub struct CommonInitParams {
    vm_name: String,
}

/// This struct is used to pass all initialization parameters for all drivers
/// when calling init_driver()
#[derive(Default)]
pub struct MicrovmiInitParams {
    common: Option<CommonInitParams>,
    xen: Option<XenInitParams>,
    kvm: Option<KVMInitParams>,
    memflow: Option<MemflowInitParams>,
}

fn main() {
    let common_init_params = CommonInitParams {
        vm_name: String::from("windows10"),
    };
    let init_params_kvm = KVMInitParams::UnixSocket {
        socket_path: PathBuf::from(r"/tmp/introspector"),
    };
    let init_params = MicrovmiInitParams {
        common: Some(common_init_params),
        kvm: Some(init_params_kvm),
        ..Default::default()
    };
    let driver = init_driver(None, Some(init_params));
}

You can find the design code on rust playground

Wenzel added the Design label May 19, 2021

Wenzel mentioned this issue Jun 17, 2021

Refactor driver initialization parameters API #201

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design: Driver initialization argument handling #197

Design: Driver initialization argument handling #197

Wenzel commented May 19, 2021 •

edited

Loading

Wenzel commented Jun 15, 2021 •

edited

Loading

rageagainsthepc commented Jun 16, 2021

Wenzel commented Jun 17, 2021 •

edited

Loading

Wenzel commented Jun 17, 2021

Wenzel commented Jun 17, 2021 •

edited

Loading

Wenzel commented Jun 17, 2021

Design: Driver initialization argument handling #197

Design: Driver initialization argument handling #197

Comments

Wenzel commented May 19, 2021 • edited Loading

adding Memflow comes with new requirements

Issue with the current implementation

Proposed solution

Wenzel commented Jun 15, 2021 • edited Loading

rageagainsthepc commented Jun 16, 2021

Wenzel commented Jun 17, 2021 • edited Loading

Wenzel commented Jun 17, 2021

Wenzel commented Jun 17, 2021 • edited Loading

Wenzel commented Jun 17, 2021

Wenzel commented May 19, 2021 •

edited

Loading

Wenzel commented Jun 15, 2021 •

edited

Loading

Wenzel commented Jun 17, 2021 •

edited

Loading

Wenzel commented Jun 17, 2021 •

edited

Loading