drive: add support for specifying...

location of root partition through the
PARTUUID kernel parameter.

Signed-off-by: Diana Popa <[email protected]>

Author: dianpopa

api_server/src/http_service.rs

@@ -1089,6 +1089,7 @@ mod tests {
             path_on_host: String::from("/foo/bar"),
             state: DeviceState::Attached,
             is_root_device: true,
+            partuuid: None,
             permissions: DrivePermissions::ro,
             rate_limiter: None,
         };

api_server/src/request/sync/drive.rs

@@ -26,6 +26,8 @@ pub struct DriveDescription {
     pub is_root_device: bool,
     pub permissions: DrivePermissions,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub partuuid: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub rate_limiter: Option<RateLimiterDescription>,
 }
 
@@ -129,6 +131,7 @@ mod tests {
                 state: DeviceState::Attached,
                 is_root_device: true,
                 permissions: DrivePermissions::ro,
+                partuuid: None,
                 rate_limiter: None,
             }.is_read_only()
         );
@@ -204,6 +207,7 @@ mod tests {
             state: DeviceState::Attached,
             is_root_device: true,
             permissions: DrivePermissions::ro,
+            partuuid: None,
             rate_limiter: None,
         };
 

vmm/src/device_config/drive.rs

@@ -13,14 +13,22 @@ pub struct BlockDeviceConfig {
     pub drive_id: String,
     pub path_on_host: PathBuf,
     pub is_root_device: bool,
+    pub partuuid: Option<String>,
     pub is_read_only: bool,
     pub rate_limiter: Option<RateLimiterDescription>,
 }
 
+impl BlockDeviceConfig {
+    pub fn get_partuuid(&self) -> Option<&String> {
+        self.partuuid.as_ref()
+    }
+}
+
 // Wrapper for the collection that holds all the Block Devices Configs
 pub struct BlockDeviceConfigs {
     pub config_list: LinkedList<BlockDeviceConfig>,
     has_root_block: bool,
+    has_partuuid_root: bool,
     read_only_root: bool,
 }
 
@@ -31,6 +39,7 @@ impl From<DriveDescription> for BlockDeviceConfig {
             drive_id: item.drive_id,
             path_on_host: PathBuf::from(item.path_on_host),
             is_root_device: item.is_root_device,
+            partuuid: item.partuuid,
             is_read_only,
             rate_limiter: item.rate_limiter,
         }
@@ -42,6 +51,7 @@ impl BlockDeviceConfigs {
         BlockDeviceConfigs {
             config_list: LinkedList::<BlockDeviceConfig>::new(),
             has_root_block: false,
+            has_partuuid_root: false,
             read_only_root: false,
         }
     }
@@ -54,6 +64,10 @@ impl BlockDeviceConfigs {
         self.read_only_root
     }
 
+    pub fn has_partuuid_root(&self) -> bool {
+        self.has_partuuid_root
+    }
+
     pub fn contains_drive_path(&self, drive_path: PathBuf) -> bool {
         for drive_config in self.config_list.iter() {
             if drive_config.path_on_host == drive_path {
@@ -92,7 +106,10 @@ impl BlockDeviceConfigs {
             } else {
                 self.has_root_block = true;
                 self.read_only_root = block_device_config.is_read_only;
-                // Root Device should be the first in the list
+                self.has_partuuid_root = block_device_config.partuuid.is_some();
+                // Root Device should be the first in the list whether or not PARTUUID is specified
+                // in order to avoid bugs in case of switching from partuuid boot scenarios to
+                // /dev/vda boot type.
                 self.config_list.push_front(block_device_config);
             }
         } else {
@@ -118,7 +135,7 @@ impl BlockDeviceConfigs {
     /// This function updates a Block Device Config prior to the guest boot. The update fails if it
     /// would result in two root block devices.
     pub fn update(&mut self, block_device_config: &BlockDeviceConfig) -> Result<()> {
-        // Check if the path exists
+        // Check if the path exists.
         if !block_device_config.path_on_host.exists() {
             return Err(DriveError::InvalidBlockDevicePath);
         }
@@ -127,23 +144,27 @@ impl BlockDeviceConfigs {
         for cfg in self.config_list.iter_mut() {
             if cfg.drive_id == block_device_config.drive_id {
                 if cfg.is_root_device {
-                    // Check if the root block device is being updated
+                    // Check if the root block device is being updated.
                     self.has_root_block = block_device_config.is_root_device;
                     self.read_only_root =
                         block_device_config.is_root_device && block_device_config.is_read_only;
+                    self.has_partuuid_root = block_device_config.partuuid.is_some();
                 } else if block_device_config.is_root_device {
-                    // Check if a second root block device is being added
+                    // Check if a second root block device is being added.
                     if root_id.is_some() {
                         return Err(DriveError::RootBlockDeviceAlreadyAdded);
                     } else {
-                        // One of the non-root blocks is becoming root
+                        // One of the non-root blocks is becoming root.
                         self.has_root_block = true;
                         self.read_only_root = block_device_config.is_read_only;
+                        self.has_partuuid_root = block_device_config.partuuid.is_some();
                     }
                 }
                 cfg.is_root_device = block_device_config.is_root_device;
                 cfg.path_on_host = block_device_config.path_on_host.clone();
                 cfg.is_read_only = block_device_config.is_read_only;
+                cfg.rate_limiter = block_device_config.rate_limiter.clone();
+                cfg.partuuid = block_device_config.partuuid.clone();
 
                 return Ok(());
             }
@@ -177,6 +198,7 @@ mod tests {
         let dummy_block_device = BlockDeviceConfig {
             path_on_host: dummy_path.clone(),
             is_root_device: false,
+            partuuid: None,
             is_read_only: false,
             drive_id: dummy_id.clone(),
             rate_limiter: None,
@@ -206,6 +228,7 @@ mod tests {
         let dummy_block_device = BlockDeviceConfig {
             path_on_host: dummy_path,
             is_root_device: true,
+            partuuid: None,
             is_read_only: true,
             drive_id: String::from("1"),
             rate_limiter: None,
@@ -232,6 +255,7 @@ mod tests {
         let root_block_device_1 = BlockDeviceConfig {
             path_on_host: dummy_path_1,
             is_root_device: true,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("1"),
             rate_limiter: None,
@@ -242,6 +266,7 @@ mod tests {
         let root_block_device_2 = BlockDeviceConfig {
             path_on_host: dummy_path_2,
             is_root_device: true,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("2"),
             rate_limiter: None,
@@ -263,6 +288,7 @@ mod tests {
         let root_block_device = BlockDeviceConfig {
             path_on_host: dummy_path_1,
             is_root_device: true,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("1"),
             rate_limiter: None,
@@ -273,6 +299,7 @@ mod tests {
         let dummy_block_device_2 = BlockDeviceConfig {
             path_on_host: dummy_path_2,
             is_root_device: false,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("2"),
             rate_limiter: None,
@@ -283,6 +310,7 @@ mod tests {
         let dummy_block_device_3 = BlockDeviceConfig {
             path_on_host: dummy_path_3,
             is_root_device: false,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("3"),
             rate_limiter: None,
@@ -319,6 +347,7 @@ mod tests {
         let root_block_device = BlockDeviceConfig {
             path_on_host: dummy_path_1,
             is_root_device: true,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("1"),
             rate_limiter: None,
@@ -329,6 +358,7 @@ mod tests {
         let dummy_block_device_2 = BlockDeviceConfig {
             path_on_host: dummy_path_2,
             is_root_device: false,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("2"),
             rate_limiter: None,
@@ -339,6 +369,7 @@ mod tests {
         let dummy_block_device_3 = BlockDeviceConfig {
             path_on_host: dummy_path_3,
             is_root_device: false,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("3"),
             rate_limiter: None,
@@ -361,7 +392,8 @@ mod tests {
         assert_eq!(block_devices_configs.config_list.len(), 3);
 
         let mut block_dev_iter = block_devices_configs.config_list.iter();
-        // The root device should be first in the list no matter of the order in which the devices were added
+        // The root device should be first in the list no matter of the order in
+        // which the devices were added.
         assert_eq!(block_dev_iter.next().unwrap(), &root_block_device);
         assert_eq!(block_dev_iter.next().unwrap(), &dummy_block_device_2);
         assert_eq!(block_dev_iter.next().unwrap(), &dummy_block_device_3);
@@ -371,6 +403,7 @@ mod tests {
     fn test_from_drive_description() {
         let dd = DriveDescription {
             is_root_device: true,
+            partuuid: None,
             path_on_host: String::from("/foo/bar"),
             drive_id: String::from("foo"),
             state: DeviceState::Attached,
@@ -392,6 +425,7 @@ mod tests {
         let root_block_device = BlockDeviceConfig {
             path_on_host: dummy_path_1,
             is_root_device: true,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("1"),
             rate_limiter: None,
@@ -402,6 +436,7 @@ mod tests {
         let mut dummy_block_device_2 = BlockDeviceConfig {
             path_on_host: dummy_path_2.clone(),
             is_root_device: false,
+            partuuid: None,
             is_read_only: false,
             drive_id: String::from("2"),
             rate_limiter: None,

vmm/src/lib.rs

@@ -445,7 +445,7 @@ impl Vmm {
     }
 
     /// Only call this function as part of the API.
-    /// If the drive_id does not exit, a new Block Device Config is added to the list.
+    /// If the drive_id does not exist, a new Block Device Config is added to the list.
     /// Else, if the VM is running, the block device will be updated.
     /// Updating before the VM has started is not allowed.
     pub fn put_block_device(
@@ -527,23 +527,22 @@ impl Vmm {
         Ok(())
     }
 
-    /// Attach all block devices from the BlockDevicesConfig
-    /// If there is no root block device, no other devices are attached.The root device should be
-    /// the first to be attached as a way to make sure it ends up on /dev/vda
-    /// This function is to be called only from boot_source
+    /// Attach all block devices from the BlockDevicesConfig.
+    /// If there is no root block device, no other devices are attached.
     fn attach_block_devices(&mut self, device_manager: &mut MMIODeviceManager) -> Result<()> {
         // If there's no root device, do not attach any other devices
         let block_dev = &self.block_device_configs;
-        let kernel_config = match self.kernel_config.as_mut() {
-            Some(x) => x,
-            None => return Err(Error::MissingKernelConfig),
-        };
+        // We rely on check_health function for making sure kernel_config is not None.
+        let kernel_config = self.kernel_config.as_mut().unwrap();
 
         if block_dev.has_root_block_device() {
-            // this is a simple solution to add a block as a root device; should be improved
-            kernel_config.cmdline.insert_str(" root=/dev/vda")?;
-            if block_dev.has_read_only_root() {
-                kernel_config.cmdline.insert_str(" ro")?;
+            // If no PARTUUID was specified for the root device, try with the /dev/vda.
+            if !block_dev.has_partuuid_root() {
+                kernel_config.cmdline.insert_str(" root=/dev/vda")?;
+
+                if block_dev.has_read_only_root() {
+                    kernel_config.cmdline.insert_str(" ro")?;
+                }
             }
 
             let epoll_context = &mut self.epoll_context;
@@ -554,6 +553,17 @@ impl Vmm {
                     .write(!drive_config.is_read_only)
                     .open(&drive_config.path_on_host)
                     .map_err(Error::RootDiskImage)?;
+
+                if drive_config.is_root_device && drive_config.partuuid.is_some() {
+                    kernel_config.cmdline.insert_str(format!(
+                        " root=PARTUUID={}",
+                        drive_config.get_partuuid().unwrap()
+                    ))?;
+                    if drive_config.is_read_only {
+                        kernel_config.cmdline.insert_str(" ro")?;
+                    }
+                }
+
                 let epoll_config = epoll_context.allocate_virtio_block_tokens();
 
                 let rate_limiter = rate_limiter_description_into_implementation(
@@ -630,10 +640,8 @@ impl Vmm {
     }
 
     fn attach_net_devices(&mut self, device_manager: &mut MMIODeviceManager) -> Result<()> {
-        let kernel_config = match self.kernel_config.as_mut() {
-            Some(x) => x,
-            None => return Err(Error::MissingKernelConfig),
-        };
+        // We rely on check_health function for making sure kernel_config is not None.
+        let kernel_config = self.kernel_config.as_mut().unwrap();
 
         for cfg in self.network_interface_configs.iter_mut() {
             let epoll_config = self.epoll_context.allocate_virtio_net_tokens();
@@ -1079,7 +1087,7 @@ impl Vmm {
                 Ok(_) => AsyncOutcome::Ok(0),
                 Err(e) => {
                     let _ = self.stop();
-                    AsyncOutcome::Error(format!("cannot boot kernel: {:?}", e))
+                    AsyncOutcome::Error(format!("Cannot start microvm: {:?}", e))
                 }
             },
         };
@@ -1364,6 +1372,7 @@ mod tests {
             drive_id: String::from("root"),
             path_on_host: f.path().to_path_buf(),
             is_root_device: true,
+            partuuid: None,
             is_read_only: false,
             rate_limiter: None,
         };
@@ -1382,6 +1391,7 @@ mod tests {
             drive_id: String::from("root"),
             path_on_host: f.path().to_path_buf(),
             is_root_device: true,
+            partuuid: None,
             is_read_only: true,
             rate_limiter: None,
         };