Design: Driver initialization argument handling

[Wenzel]

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

1. passing a domain_name is not required
2. 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.

[Wenzel]

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]

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]

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]

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]

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]

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