Update linux-loader to 0.10.0

Update to align to the new Cmdline API and kernel image loader. While at it, update the load_kernel() tests as well. Signed-off-by: Alvise Rigo <[email protected]> Reviewed-by: Timos Ampelikiotis <[email protected]>
rust-vmm · Nov 9, 2023 · eae835f · eae835f
1 parent 09866dc
commit eae835f
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 26 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -21,5 +21,6 @@ lto = true
 panic = "abort"
 
 [patch.crates-io]
-# TODO: Using this patch until a version > 4.0 gets published.
-linux-loader = { git = "https://github.com/rust-vmm/linux-loader.git", rev = "9a9f071" }
+# TODO: Update with https://github.com/rust-vmm/linux-loader.git hash of commit
+# "add as_string() to the Cmdline crate"
+linux-loader = { version = "0.10.0", path = "../linux-loader" }
diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml
@@ -10,4 +10,4 @@ clap = "3.2.17"
 vmm = { path = "../vmm" }
 
 [dev-dependencies]
-linux-loader = "0.4.0"
+linux-loader = "0.10.0"
diff --git a/src/api/src/lib.rs b/src/api/src/lib.rs
@@ -211,7 +211,7 @@ mod tests {
         ])
         .is_err());
 
-        let mut foo_cmdline = Cmdline::new(4096);
+        let mut foo_cmdline = Cmdline::new(4096).unwrap();
         foo_cmdline.insert_str("\"foo=bar bar=foo\"").unwrap();
 
         // OK.

diff --git a/src/devices/Cargo.toml b/src/devices/Cargo.toml
@@ -9,7 +9,7 @@ license = "Apache-2.0 OR BSD-3-Clause"
 event-manager = { version = "0.3.0", features = ["remote_endpoint"] }
 kvm-ioctls = "0.13.0"
 libc = "0.2.76"
-linux-loader = "0.4.0"
+linux-loader = "0.10.0"
 log = "*"
 vm-memory = "0.13.1"
 vm-superio = "0.5.0"

diff --git a/src/devices/src/virtio/block/device.rs b/src/devices/src/virtio/block/device.rs
@@ -191,7 +191,7 @@ mod tests {
         assert_eq!(block.device_type(), BLOCK_DEVICE_ID);
 
         assert_eq!(
-            mock.kernel_cmdline.as_str(),
+            mock.kernel_cmdline.as_string().unwrap(),
             format!(
                 "virtio_mmio.device=4K@0x{:x}:{} root=/dev/vda ro",
                 mock.mmio_cfg.range.base().0,

diff --git a/src/devices/src/virtio/mod.rs b/src/devices/src/virtio/mod.rs
@@ -332,7 +332,7 @@ pub(crate) mod tests {
                 mmio_mgr: IoManager::new(),
                 mmio_cfg,
                 // `4096` seems large enough for testing.
-                kernel_cmdline: Cmdline::new(4096),
+                kernel_cmdline: Cmdline::new(4096).unwrap(),
             }
         }
         pub fn env(&mut self) -> Env<MockMem, &mut IoManager> {
@@ -387,7 +387,7 @@ pub(crate) mod tests {
         assert_eq!(bus_range.size(), range.size());
 
         assert_eq!(
-            mock.kernel_cmdline.as_str(),
+            mock.kernel_cmdline.as_string().unwrap(),
             format!(
                 "virtio_mmio.device=4K@0x{:x}:{}",
                 range.base().0,
@@ -396,7 +396,11 @@ pub(crate) mod tests {
         );
 
         mock.env().insert_cmdline_str("ending_string").unwrap();
-        assert!(mock.kernel_cmdline.as_str().ends_with("ending_string"));
+        assert!(mock
+            .kernel_cmdline
+            .as_string()
+            .unwrap()
+            .ends_with("ending_string"));
     }
 
     #[test]

diff --git a/src/vmm/Cargo.toml b/src/vmm/Cargo.toml
@@ -9,7 +9,7 @@ event-manager = "0.3.0"
 kvm-bindings = { version = "0.6.0", features = ["fam-wrappers"] }
 kvm-ioctls = "0.13.0"
 libc = "0.2.91"
-linux-loader = { version = "0.4.0", features = ["bzimage", "elf"] }
+linux-loader = { version = "0.10.0", features = ["bzimage", "elf"] }
 vm-allocator = "0.1.0"
 vm-memory = { version = "0.13.1", features = ["backend-mmap"] }
 vm-superio = "0.5.0"

diff --git a/src/vmm/src/config/mod.rs b/src/vmm/src/config/mod.rs
@@ -147,7 +147,7 @@ pub struct KernelConfig {
 impl KernelConfig {
     /// Return the default kernel command line used by the Vmm.
     pub fn default_cmdline() -> Cmdline {
-        let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY);
+        let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY).unwrap();
 
         // It's ok to use `unwrap` because the initial capacity of `cmdline` should be
         // sufficient to accommodate the default kernel cmdline.
@@ -181,7 +181,7 @@ impl TryFrom<&str> for KernelConfig {
             .map_err(ConversionError::new_kernel)?
             .unwrap_or_else(|| DEFAULT_KERNEL_CMDLINE.to_string());
 
-        let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY);
+        let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY).unwrap();
         cmdline
             .insert_str(cmdline_str)
             .map_err(|_| ConversionError::new_kernel("Kernel cmdline capacity error"))?;
@@ -282,7 +282,7 @@ mod tests {
         // Check that additional commas in the kernel string do not cause a panic.
         let kernel_str = r#"path=/foo/bar,cmdline="foo=bar",kernel_load_addr=42,"#;
 
-        let mut foo_cmdline = Cmdline::new(128);
+        let mut foo_cmdline = Cmdline::new(128).unwrap();
         foo_cmdline.insert_str("\"foo=bar\"").unwrap();
 
         let expected_kernel_config = KernelConfig {

diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs
@@ -458,12 +458,12 @@ impl Vmm {
 
         // Add the kernel command line to the boot parameters.
         bootparams.hdr.cmd_line_ptr = CMDLINE_START as u32;
-        bootparams.hdr.cmdline_size = self.kernel_cfg.cmdline.as_str().len() as u32 + 1;
+        bootparams.hdr.cmdline_size = self.kernel_cfg.cmdline.as_string().unwrap().len() as u32 + 1;
 
         // Load the kernel command line into guest memory.
-        let mut cmdline = Cmdline::new(4096);
+        let mut cmdline = Cmdline::new(4096).unwrap();
         cmdline
-            .insert_str(self.kernel_cfg.cmdline.as_str())
+            .insert_str(self.kernel_cfg.cmdline.as_string().unwrap())
             .map_err(Error::Cmdline)?;
 
         load_cmdline(
@@ -724,7 +724,7 @@ impl Vmm {
         let cmdline = &self.kernel_cfg.cmdline;
         let fdt = self
             .fdt_builder
-            .with_cmdline(cmdline.as_str().to_string())
+            .with_cmdline(cmdline.as_string().unwrap())
             .with_num_vcpus(self.num_vcpus.try_into().unwrap())
             .with_mem_size(mem_size)
             .create_fdt()
@@ -750,6 +750,7 @@ mod tests {
     #[cfg(target_arch = "x86_64")]
     use std::path::Path;
     use std::path::PathBuf;
+    use std::fs::write;
     #[cfg(target_arch = "x86_64")]
     use vm_memory::{
         bytes::ByteValued,
@@ -956,20 +957,38 @@ mod tests {
             matches!(vmm.load_kernel().unwrap_err(), Error::IO(e) if e.kind() == ErrorKind::NotFound)
         );
 
-        // Test case: kernel image is invalid.
+        // Test case: kernel image is invalid. This test has two flavors. In the first
+        // we try to load an empty file, in the second a file which has all zeros.
         let mut vmm_config = default_vmm_config();
         let temp_file = TempFile::new().unwrap();
         vmm_config.kernel_config.path = PathBuf::from(temp_file.as_path());
         let mut vmm = mock_vmm(vmm_config);
 
         let err = vmm.load_kernel().unwrap_err();
+        #[cfg(target_arch = "x86_64")]
+        assert!(matches!(
+            err,
+            Error::KernelLoad(loader::Error::Elf(loader::elf::Error::ReadElfHeader))
+        ));
+        #[cfg(target_arch = "aarch64")]
+        assert!(matches!(
+            err,
+            Error::KernelLoad(loader::Error::Pe(loader::pe::Error::ReadImageHeader))
+        ));
+
+        let temp_file_path = PathBuf::from(temp_file.as_path());
+        let buffer: Vec<u8> = vec![0_u8; 1024];
+        write(temp_file_path, buffer).unwrap();
+        let err = vmm.load_kernel().unwrap_err();
+
         #[cfg(target_arch = "x86_64")]
         assert!(matches!(
             err,
             Error::KernelLoad(loader::Error::Bzimage(
                 loader::bzimage::Error::InvalidBzImage
             ))
         ));
+
         #[cfg(target_arch = "aarch64")]
         assert!(matches!(
             err,
@@ -1011,15 +1030,16 @@ mod tests {
         let mut vmm_config = default_vmm_config();
         vmm_config.kernel_config.path = default_elf_path();
         let mut vmm = mock_vmm(vmm_config);
-        assert_eq!(vmm.kernel_cfg.cmdline.as_str(), DEFAULT_KERNEL_CMDLINE);
+        assert_eq!(vmm.kernel_cfg.cmdline.as_string().unwrap(), DEFAULT_KERNEL_CMDLINE);
         vmm.add_serial_console().unwrap();
         #[cfg(target_arch = "x86_64")]
-        assert!(vmm.kernel_cfg.cmdline.as_str().contains("console=ttyS0"));
+        assert!(vmm.kernel_cfg.cmdline.as_string().unwrap().contains("console=ttyS0"));
         #[cfg(target_arch = "aarch64")]
         assert!(vmm
             .kernel_cfg
             .cmdline
-            .as_str()
+            .as_string()
+            .unwrap()
             .contains("earlycon=uart,mmio"));
     }
 
@@ -1118,7 +1138,7 @@ mod tests {
         assert_eq!(vmm.block_devices.len(), 1);
         #[cfg(target_arch = "aarch64")]
         assert_eq!(vmm.fdt_builder.virtio_device_len(), 1);
-        assert!(vmm.kernel_cfg.cmdline.as_str().contains("virtio"));
+        assert!(vmm.kernel_cfg.cmdline.as_string().unwrap().contains("virtio"));
 
         let invalid_block_config = BlockConfig {
             // Let's create the tempfile directly here so that it gets out of scope immediately
@@ -1151,7 +1171,7 @@ mod tests {
             assert_eq!(vmm.net_devices.len(), 1);
             #[cfg(target_arch = "aarch64")]
             assert_eq!(vmm.fdt_builder.virtio_device_len(), 1);
-            assert!(vmm.kernel_cfg.cmdline.as_str().contains("virtio"));
+            assert!(vmm.kernel_cfg.cmdline.as_string().unwrap().contains("virtio"));
         }
     }
 
@@ -1172,7 +1192,7 @@ mod tests {
             let cmdline = &vmm.kernel_cfg.cmdline;
             let fdt = vmm
                 .fdt_builder
-                .with_cmdline(cmdline.as_str().to_string())
+                .with_cmdline(cmdline.as_string().unwrap().to_string())
                 .with_num_vcpus(vmm.num_vcpus.try_into().unwrap())
                 .with_mem_size(mem_size)
                 .with_serial_console(0x40000000, 0x1000)