kata-ctl: Refactor kernel module check

Adding vhost and vhost-net to the kernel modules. These do not require
any kernel module parameters to be checked. Currently, kernel params is
a required field. Make this as optional. Could make this as <Option>,
but making this a slice instead, as a module could have multiple kernel
params. Refactor the function that checks are for kernel modules into
two with one specifically checking if the module is loaded and other
checking for module parameters.

Refactor some of the tests to take into account these changes.

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>

src/tools/kata-ctl/src/arch/x86_64/mod.rs

@@ -59,17 +59,29 @@ mod arch_specific {
     static MODULE_LIST: &[KernelModule] = &[
         KernelModule {
             name: "kvm",
-            parameter: KernelParam {
+            params: &[KernelParam {
                 name: "kvmclock_periodic_sync",
                 value: KernelParamType::Simple("Y"),
-            },
+            }],
         },
         KernelModule {
             name: "kvm_intel",
-            parameter: KernelParam {
+            params: &[KernelParam {
                 name: "unrestricted_guest",
                 value: KernelParamType::Predicate(unrestricted_guest_param_check),
-            },
+            }],
+        },
+        KernelModule {
+            name: "vhost",
+            params: &[],
+        },
+        KernelModule {
+            name: "vhost_net",
+            params: &[],
+        },
+        KernelModule {
+            name: "vhost_vsock",
+            params: &[],
         },
     ];
 
@@ -253,6 +265,38 @@ mod arch_specific {
         }
     }
 
+    fn check_kernel_params(kernel_module: &KernelModule) -> Result<()> {
+        const MODULES_PATH: &str = "/sys/module";
+
+        for param in kernel_module.params {
+            let module_param_path = format!(
+                "{}/{}/parameters/{}",
+                MODULES_PATH, kernel_module.name, param.name
+            );
+
+            // Here the currently loaded kernel parameter value
+            // is retrieved and returned on success
+            let param_value_host = std::fs::read_to_string(module_param_path)
+                .map(|val| val.replace('\n', ""))
+                .map_err(|_err| {
+                    anyhow!(
+                        "'{:}' kernel module parameter `{:}` not found.",
+                        kernel_module.name,
+                        param.name
+                    )
+                })?;
+
+            check_kernel_param(
+                kernel_module.name,
+                param.name,
+                &param_value_host,
+                param.value.clone(),
+            )
+            .map_err(|e| anyhow!(e.to_string()))?;
+        }
+        Ok(())
+    }
+
     fn check_kernel_param(
         module: &str,
         param_name: &str,
@@ -282,19 +326,12 @@ mod arch_specific {
         info!(sl!(), "check kernel modules for: x86_64");
 
         for module in MODULE_LIST {
-            let module_loaded =
+            let module_loaded = check::check_kernel_module_loaded(module);
-                check::check_kernel_module_loaded(module.name, module.parameter.name);
 
             match module_loaded {
-                Ok(param_value_host) => {
+                Ok(_) => {
-                    let parameter_check = check_kernel_param(
+                    let check = check_kernel_params(module);
-                        module.name,
+                    match check {
-                        module.parameter.name,
-                        &param_value_host,
-                        module.parameter.value.clone(),
-                    );
-
-                    match parameter_check {
                         Ok(_v) => info!(sl!(), "{} Ok", module.name),
                         Err(e) => return Err(e),
                     }

src/tools/kata-ctl/src/check.rs

@@ -5,6 +5,7 @@
 
 // Contains checks that are not architecture-specific
 
+use crate::types::KernelModule;
 use anyhow::{anyhow, Result};
 use nix::fcntl::{open, OFlag};
 use nix::sys::stat::Mode;
@@ -324,17 +325,16 @@ pub fn check_official_releases() -> Result<()> {
 }
 
 #[cfg(any(target_arch = "x86_64"))]
-pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<String, String> {
+pub fn check_kernel_module_loaded(kernel_module: &KernelModule) -> Result<(), String> {
     const MODPROBE_PARAMETERS_DRY_RUN: &str = "--dry-run";
     const MODPROBE_PARAMETERS_FIRST_TIME: &str = "--first-time";
-    const MODULES_PATH: &str = "/sys/module";
 
     let status_modinfo_success;
 
     // Partial check w/ modinfo
     // verifies that the module exists
     match Command::new(MODINFO_PATH)
-        .arg(module)
+        .arg(kernel_module.name)
         .stdout(Stdio::piped())
         .output()
     {
@@ -361,7 +361,7 @@ pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<Strin
     match Command::new(MODPROBE_PATH)
         .arg(MODPROBE_PARAMETERS_DRY_RUN)
         .arg(MODPROBE_PARAMETERS_FIRST_TIME)
-        .arg(module)
+        .arg(kernel_module.name)
         .stdout(Stdio::piped())
         .output()
     {
@@ -371,8 +371,8 @@ pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<Strin
 
             if status_modprobe_success && status_modinfo_success {
                 // This condition is true in the case that the module exist, but is not already loaded
-                let msg = format!("The kernel module `{:}` exist but is not already loaded. Try reloading it using 'modprobe {:}=Y'",
+                let msg = format!("The kernel module `{:}` exist but is not already loaded. Try reloading it using 'modprobe {:}'",
-                        module, module
+                kernel_module.name, kernel_module.name
                     );
                 return Err(msg);
             }
@@ -386,27 +386,14 @@ pub fn check_kernel_module_loaded(module: &str, parameter: &str) -> Result<Strin
             return Err(msg);
         }
     }
-
+    Ok(())
-    let module_path = format!("{}/{}/parameters/{}", MODULES_PATH, module, parameter);
-
-    // Here the currently loaded kernel parameter value
-    // is retrieved and returned on success
-    match read_file_contents(&module_path) {
-        Ok(result) => Ok(result.replace('\n', "")),
-        Err(_e) => {
-            let msg = format!(
-                "'{:}' kernel module parameter `{:}` not found.",
-                module, parameter
-            );
-            Err(msg)
-        }
-    }
 }
 
 #[cfg(any(target_arch = "s390x", target_arch = "x86_64"))]
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::types::{KernelModule, KernelParam, KernelParamType};
     use semver::Version;
     use slog::warn;
     use std::fs;
@@ -612,12 +599,13 @@ mod tests {
     #[test]
     fn check_module_loaded() {
         #[allow(dead_code)]
-        #[derive(Debug)]
+
         struct TestData<'a> {
             module_name: &'a str,
             param_name: &'a str,
+            kernel_module: &'a KernelModule<'a>,
             param_value: &'a str,
-            result: Result<String>,
+            result: Result<()>,
         }
 
         let tests = &[
@@ -625,45 +613,58 @@ mod tests {
             TestData {
                 module_name: "",
                 param_name: "",
+                kernel_module: &KernelModule {
+                    name: "",
+                    params: &[KernelParam {
+                        name: "",
+                        value: KernelParamType::Simple("Y"),
+                    }],
+                },
                 param_value: "",
                 result: Err(anyhow!("modinfo: ERROR: Module {} not found.", "")),
             },
-            TestData {
-                module_name: "kvm",
-                param_name: "",
-                param_value: "",
-                result: Err(anyhow!(
-                    "'{:}' kernel module parameter `{:}` not found.",
-                    "kvm",
-                    ""
-                )),
-            },
             // Success scenarios
+            TestData {
+                module_name: "kvm",
+                param_name: "",
+                kernel_module: &KernelModule {
+                    name: "kvm",
+                    params: &[KernelParam {
+                        name: "nonexistantparam",
+                        value: KernelParamType::Simple("Y"),
+                    }],
+                },
+                param_value: "",
+                result: Ok(()),
+            },
             TestData {
                 module_name: "kvm",
                 param_name: "kvmclock_periodic_sync",
+                kernel_module: &KernelModule {
+                    name: "kvm",
+                    params: &[KernelParam {
+                        name: "kvmclock_periodic_sync",
+                        value: KernelParamType::Simple("Y"),
+                    }],
+                },
                 param_value: "Y",
-                result: Ok("Y".to_string()),
+                result: Ok(()),
             },
         ];
 
         for (i, d) in tests.iter().enumerate() {
-            let msg = format!("test[{}]: {:?}", i, d);
+            let msg = format!("test[{}]", i);
-            let result = check_kernel_module_loaded(d.module_name, d.param_name);
+            let result = check_kernel_module_loaded(d.kernel_module);
             let msg = format!("{}, result: {:?}", msg, result);
 
             if d.result.is_ok() {
-                assert_eq!(
+                assert_eq!(result, Ok(()));
-                    result.as_ref().unwrap(),
-                    d.result.as_ref().unwrap(),
-                    "{}",
-                    msg
-                );
                 continue;
             }
 
             let expected_error = format!("{}", &d.result.as_ref().unwrap_err());
             let actual_error = result.unwrap_err().to_string();
+            println!("testing for {}", d.module_name);
             assert!(actual_error == expected_error, "{}", msg);
         }
     }

src/tools/kata-ctl/src/types.rs

@@ -69,5 +69,5 @@ pub struct KernelParam<'a> {
 #[allow(dead_code)]
 pub struct KernelModule<'a> {
     pub name: &'a str,
-    pub parameter: KernelParam<'a>,
+    pub params: &'a [KernelParam<'a>],
 }