From ea0a07e5d6545744834c3d7f37f3230f5e856324 Mon Sep 17 00:00:00 2001
From: Yang Xiaohe <yangxiaohe2@huawei.com>
Date: Wed, 2 Dec 2020 11:10:35 +0800
Subject: [PATCH 1/2] machine_manager: update machine config.

1. Move guest name to Vmconfig.
2. Add machine type field in MachineConfig.
3. Rename `omit_vm_memory` to `dump_guest_core`, and move it to MachineConfig.

Signed-off-by: Yang Xiaohe <yangxiaohe2@huawei.com>
---
 address_space/src/host_mmap.rs               | 14 ++---
 device_model/src/micro_vm/cmdline.rs         | 28 +++-------
 device_model/src/micro_vm/mod.rs             |  2 +-
 docs/config_guidebook.md                     | 56 ++++++++++----------
 machine_manager/src/config/machine_config.rs | 46 ++++++++--------
 machine_manager/src/config/mod.rs            | 13 ++++-
 6 files changed, 80 insertions(+), 79 deletions(-)

diff --git a/address_space/src/host_mmap.rs b/address_space/src/host_mmap.rs
index 25a79fd..f172f46 100644
--- a/address_space/src/host_mmap.rs
+++ b/address_space/src/host_mmap.rs
@@ -82,11 +82,11 @@ impl FileBackend {
 ///
 /// * `ranges` - The guest address range that will be mapped.
 /// * `file_path` - The path of backend-file.
-/// * `omit_vm_memory` - Dump guest memory in core file or not.
+/// * `dump_guest_core` - Include guest memory in core file or not.
 pub fn create_host_mmaps(
     ranges: &[(u64, u64)],
     file_path: Option<&str>,
-    omit_vm_memory: bool,
+    dump_guest_core: bool,
 ) -> Result<Vec<Arc<HostMemMapping>>> {
     let mut mappings = Vec::new();
 
@@ -101,7 +101,7 @@ pub fn create_host_mmaps(
                     range.1,
                     f_back.file.as_raw_fd(),
                     f_back.offset,
-                    omit_vm_memory,
+                    dump_guest_core,
                 )?));
 
                 f_back.offset += range.1;
@@ -114,7 +114,7 @@ pub fn create_host_mmaps(
                     range.1,
                     -1,
                     0,
-                    omit_vm_memory,
+                    dump_guest_core,
                 )?));
             }
         }
@@ -146,7 +146,7 @@ impl HostMemMapping {
     /// * `size` - Size of memory that will be mapped.
     /// * `file_back` - The file's raw fd that backs memory,
     /// * `file_offset` - Offset in the file that backs memory.
-    /// * `omit_vm_memory` - Dump guest memory in core file or not.
+    /// * `dump_guest_core` - Include guest memory in core file or not.
     ///
     /// # Errors
     ///
@@ -156,7 +156,7 @@ impl HostMemMapping {
         size: u64,
         file_back: RawFd,
         file_offset: u64,
-        omit_vm_memory: bool,
+        dump_guest_core: bool,
     ) -> Result<HostMemMapping> {
         let mut flags = libc::MAP_PRIVATE | libc::MAP_NORESERVE;
         if file_back == -1 {
@@ -178,7 +178,7 @@ impl HostMemMapping {
             hva
         };
 
-        if omit_vm_memory {
+        if !dump_guest_core {
             unsafe {
                 let madvise_res = libc::madvise(
                     host_addr as *mut libc::c_void,
diff --git a/device_model/src/micro_vm/cmdline.rs b/device_model/src/micro_vm/cmdline.rs
index ebf10b5..cbea6fc 100644
--- a/device_model/src/micro_vm/cmdline.rs
+++ b/device_model/src/micro_vm/cmdline.rs
@@ -82,6 +82,13 @@ pub fn create_args_parser<'a>() -> ArgParser<'a> {
                 .help("set the name of the guest.")
                 .takes_value(true),
         )
+        .arg(
+            Arg::with_name("machine")
+                .long("machine")
+                .value_name("[type=]name[,dump_guest_core=on|off]")
+                .help("selects emulated machine")
+                .takes_value(true),
+        )
         .arg(
             Arg::with_name("smp")
                 .long("smp")
@@ -209,13 +216,6 @@ pub fn create_args_parser<'a>() -> ArgParser<'a> {
                 .takes_value(false)
                 .required(false),
         )
-        .arg(
-            Arg::with_name("omit_vm_memory")
-                .long("omit_vm_memory")
-                .help("not dump guest memory in core file")
-                .takes_value(false)
-                .required(false),
-        )
         // Below cmdline is adapted for Kata/Qemu, no use.
         .arg(
             Arg::with_name("uuid")
@@ -232,13 +232,6 @@ pub fn create_args_parser<'a>() -> ArgParser<'a> {
                 .takes_value(true)
                 .hidden(true),
         )
-        .arg(
-            Arg::with_name("machine")
-                .long("machine")
-                .help("selects emulated machine")
-                .takes_value(true)
-                .hidden(true),
-        )
         .arg(
             Arg::with_name("global_property")
                 .long("global")
@@ -346,6 +339,7 @@ pub fn create_vmconfig(args: &ArgMatches) -> Result<VmConfig> {
 
     // Parse cmdline args which need to set in VmConfig
     update_args_to_config!((args.value_of("name")), vm_cfg, update_name);
+    update_args_to_config!((args.value_of("machine")), vm_cfg, update_machine);
     update_args_to_config!((args.value_of("memory")), vm_cfg, update_memory);
     update_args_to_config!((args.value_of("smp")), vm_cfg, update_cpu);
     update_args_to_config!((args.value_of("kernel")), vm_cfg, update_kernel);
@@ -361,12 +355,6 @@ pub fn create_vmconfig(args: &ArgMatches) -> Result<VmConfig> {
     update_args_to_config_multi!((args.values_of("device")), vm_cfg, update_vsock);
     update_args_to_config_multi!((args.values_of("netdev")), vm_cfg, update_net);
     update_args_to_config_multi!((args.values_of("chardev")), vm_cfg, update_console);
-    update_args_to_config!(
-        (args.is_present("omit_vm_memory")),
-        vm_cfg,
-        update_omit_vm_memory,
-        bool
-    );
 
     // Check the mini-set for Vm to start is ok
     vm_cfg
diff --git a/device_model/src/micro_vm/mod.rs b/device_model/src/micro_vm/mod.rs
index 06a320d..eb0df3a 100644
--- a/device_model/src/micro_vm/mod.rs
+++ b/device_model/src/micro_vm/mod.rs
@@ -229,7 +229,7 @@ impl LightMachine {
         // Define ram-region ranges according to architectures
         let ram_ranges = Self::arch_ram_ranges(vm_config.machine_config.mem_size);
         let mem_mappings =
-            create_host_mmaps(&ram_ranges, None, vm_config.machine_config.omit_vm_memory)?;
+            create_host_mmaps(&ram_ranges, None, vm_config.machine_config.dump_guest_core)?;
         for mmap in mem_mappings.iter() {
             sys_mem.root().add_subregion(
                 Region::init_ram_region(mmap.clone()),
diff --git a/docs/config_guidebook.md b/docs/config_guidebook.md
index 65e1a35..665a7f5 100644
--- a/docs/config_guidebook.md
+++ b/docs/config_guidebook.md
@@ -6,7 +6,30 @@ StratoVirt supports json configuration file and cmdline arguments. If you set th
  json configuration file and cmdline arguments, cmdline arguments will override settings in json
  configuration file.
 
-### 1.1 Cpu Number
+### 1.1 Machine Config
+
+General configuration of machine, including
+* type: The machine type of machine, StratoVirt only support MicroVm yet.
+* dump-guest-core: Including guest memory in coredump file or not, default value is true.
+
+This feature is closed by default. There are two ways to open it:
+
+```shell
+# cmdline
+-machine [type=]name[,dump-guest-core=on|off]
+
+# json
+{
+    "machine-config": {
+        "type": "MicroVm",
+        "dump_guest_core": false,
+        ...
+    },
+    ...
+}
+```
+
+### 1.2 Cpu Number
 
 StratoVirt supports to set the number of VCPUs(**nr_vcpus**).
 
@@ -28,7 +51,7 @@ By default, after booted, VM will online all CPUs you set.
 }
 ```
 
-### 1.2 Memory Size
+### 1.3 Memory Size
 
 StratoVirt supports to set the size of VM's memory in cmdline.
 
@@ -54,7 +77,7 @@ But unfortunately, in json configuration file, only `byte` is supported as unit.
 }
 ```
 
-### 1.3 Kernel and Kernel Parameters
+### 1.4 Kernel and Kernel Parameters
 
 StratoVirt supports to launch PE or bzImage (only x86_64) format linux kernel 4.19 and can also set kernel
  parameters for VM.
@@ -79,7 +102,7 @@ And the given kernel parameters will be actually analyzed by boot loader.
 }
 ```
 
-### 1.4 Initrd Configuration
+### 1.5 Initrd Configuration
 
 StratoVirt supports to launch VM by a initrd (boot loader initialized RAM disk) as well.
 
@@ -493,28 +516,3 @@ You can enable StratoVirt's logging by:
 
 StratoVirt's log-level depends on env `STRATOVIRT_LOG_LEVEL`.
 StratoVirt supports four log-levels: `trace`, `debug`, `info`, `warn`, `error`. The default level is `error`.
-
-### 4.4 Omit_vm_memory
-
-When StratoVirt aborts unexpectedly, you can get a related core file in which the guest machine's
-whole memory are dumped. However, in most cases, such memory dumping is worthless and may consume
-a lot of system memory for storing.
-
-StratoVirt provides feature `omit_vm_memory` to avoid dumping vm's memory in the core file.
-
-This feature is closed by default. There are two ways to open it:
-
-```shell
-# cmdline
--omit_vm_memory
-
-# json
-{
-    "machine-config": {
-        ...
-        "omit_vm_memory": true,
-        ...
-    },
-    ...
-}
-```
diff --git a/machine_manager/src/config/machine_config.rs b/machine_manager/src/config/machine_config.rs
index 6f7ebf1..f9c8675 100644
--- a/machine_manager/src/config/machine_config.rs
+++ b/machine_manager/src/config/machine_config.rs
@@ -24,7 +24,6 @@ const MAX_NR_CPUS: u8 = 254;
 const MIN_NR_CPUS: u8 = 1;
 const MAX_MEMSIZE: u64 = 549_755_813_888;
 const MIN_MEMSIZE: u64 = 134_217_728;
-const MAX_STRING_LENGTH: usize = 255;
 const M: u64 = 1024 * 1024;
 const G: u64 = 1024 * 1024 * 1024;
 
@@ -32,20 +31,20 @@ const G: u64 = 1024 * 1024 * 1024;
 /// Contains some basic Vm config about cpu, memory, name.
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct MachineConfig {
-    pub name: String,
+    pub mach_type: String,
     pub nr_cpus: u8,
     pub mem_size: u64,
-    pub omit_vm_memory: bool,
+    pub dump_guest_core: bool,
 }
 
 impl Default for MachineConfig {
     /// Set default config for `machine-config`.
     fn default() -> Self {
         MachineConfig {
-            name: "StratoVirt".to_string(),
+            mach_type: "MicroVm".to_string(),
             nr_cpus: DEFAULT_CPUS,
             mem_size: DEFAULT_MEMSIZE * M,
-            omit_vm_memory: false,
+            dump_guest_core: true,
         }
     }
 }
@@ -58,8 +57,8 @@ impl MachineConfig {
     /// * `Value` - structure can be gotten by `json_file`.
     pub fn from_value(value: &serde_json::Value) -> Self {
         let mut machine_config = MachineConfig::default();
-        if value.get("name") != None {
-            machine_config.name = value["name"].to_string();
+        if value.get("type") != None {
+            machine_config.mach_type = value["type"].to_string();
         }
         if value.get("vcpu_count") != None {
             machine_config.nr_cpus = value["vcpu_count"].to_string().parse::<u8>().unwrap();
@@ -67,9 +66,11 @@ impl MachineConfig {
         if value.get("mem_size") != None {
             machine_config.mem_size = value["mem_size"].to_string().parse::<u64>().unwrap();
         }
-        if value.get("omit_vm_memory") != None {
-            machine_config.omit_vm_memory =
-                value["omit_vm_memory"].to_string().parse::<bool>().unwrap();
+        if value.get("dump_guest_core") != None {
+            machine_config.dump_guest_core = value["dump_guest_core"]
+                .to_string()
+                .parse::<bool>()
+                .unwrap();
         }
         machine_config
     }
@@ -77,12 +78,6 @@ impl MachineConfig {
 
 impl ConfigCheck for MachineConfig {
     fn check(&self) -> Result<()> {
-        if self.name.len() > MAX_STRING_LENGTH {
-            return Err(
-                ErrorKind::StringLengthTooLong("name".to_string(), MAX_STRING_LENGTH).into(),
-            );
-        }
-
         if self.nr_cpus < MIN_NR_CPUS || self.nr_cpus > MAX_NR_CPUS {
             return Err(ErrorKind::NrcpusError.into());
         }
@@ -96,6 +91,20 @@ impl ConfigCheck for MachineConfig {
 }
 
 impl VmConfig {
+    /// Update argument `name` to `VmConfig`.
+    ///
+    /// # Arguments
+    ///
+    /// * `name` - The name `String` updated to `VmConfig`.
+    pub fn update_machine(&mut self, mach_config: String) {
+        let cmd_params: CmdParams = CmdParams::from_str(mach_config);
+        if let Some(mach_type) = cmd_params.get("type") {
+            self.machine_config.mach_type = mach_type.value;
+        }
+        if let Some(dump_guest) = cmd_params.get("dump-guest-core") {
+            self.machine_config.dump_guest_core = dump_guest.to_bool();
+        }
+    }
     /// Update '-m' memory config to `VmConfig`.
     pub fn update_memory(&mut self, mem_config: String) {
         let cmd_params: CmdParams = CmdParams::from_str(mem_config);
@@ -127,11 +136,6 @@ impl VmConfig {
             self.machine_config.nr_cpus = cpu_num.value_to_u8();
         }
     }
-
-    /// Update '-omit_vm_memory' config to 'VmConfig'.
-    pub fn update_omit_vm_memory(&mut self) {
-        self.machine_config.omit_vm_memory = true;
-    }
 }
 
 fn get_inner<T>(outer: Option<T>) -> T {
diff --git a/machine_manager/src/config/mod.rs b/machine_manager/src/config/mod.rs
index aff5bfd..26af313 100644
--- a/machine_manager/src/config/mod.rs
+++ b/machine_manager/src/config/mod.rs
@@ -71,6 +71,7 @@ pub mod errors {
 
 /// `MAX_VCPUS`: the most cpu number Vm support.
 pub static MAX_VCPUS: u8 = 128_u8;
+const MAX_STRING_LENGTH: usize = 255;
 
 /// Macro: From serde_json: Value $y to get member $z, use $s's from_value
 /// function to convert.
@@ -91,6 +92,7 @@ macro_rules! config_parse {
 /// This main config structure for Vm, contains Vm's basic configuration and devices.
 #[derive(Serialize, Deserialize, Clone, Default, Debug)]
 pub struct VmConfig {
+    pub guest_name: String,
     pub machine_config: MachineConfig,
     pub boot_source: BootSource,
     pub drives: Option<Vec<DriveConfig>>,
@@ -125,6 +127,7 @@ impl VmConfig {
         config_parse!(serial, value, "serial", SerialConfig);
 
         Ok(VmConfig {
+            guest_name: "StratoVirt".to_string(),
             machine_config,
             boot_source,
             drives,
@@ -140,6 +143,14 @@ impl VmConfig {
         self.boot_source.check()?;
         self.machine_config.check()?;
 
+        if self.guest_name.len() > MAX_STRING_LENGTH {
+            return Err(self::errors::ErrorKind::StringLengthTooLong(
+                "name".to_string(),
+                MAX_STRING_LENGTH,
+            )
+            .into());
+        }
+
         if self.drives.is_some() {
             for drive in self.drives.as_ref().unwrap() {
                 drive.check()?;
@@ -179,7 +190,7 @@ impl VmConfig {
     ///
     /// * `name` - The name `String` updated to `VmConfig`.
     pub fn update_name(&mut self, name: String) {
-        self.machine_config.name = name;
+        self.guest_name = name;
     }
 }
 
-- 
2.27.0

