github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-06-25 23:11:57 +00:00
Code Issues Projects Releases Wiki Activity

agent/device: Change update_spec_device to handle multiple devices at once

Browse Source 
update_spec_device() adjusts the OCI spec for device differences
between the host and guest.  It is called repeatedly for each device
we need to alter.  These calls are now all in a single loop in
add_devices(), so it makes more sense to move the loop into a renamed
update_spec_devices() and process all the fixups in one call.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
David Gibson 2021-11-04 19:58:52 +11:00
parent d6a3ebc496
commit 084538d334
1 changed files with 114 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

221
src/agent/src/device.rs
View File

@ -503,82 +503,83 @@ impl<T: Into<DevUpdate>> From<T> for SpecUpdate {
} }
} }
// update_spec_device updates the device list in the OCI spec to make // update_spec_devices updates the device list in the OCI spec to make
// it include details appropriate for the VM, instead of the host. It // it include details appropriate for the VM, instead of the host. It
// is given: // is given a map of (container_path => update) where:
// container_path: the path to the device in the original OCI spec // container_path: the path to the device in the original OCI spec
// update: information on changes to make to the device // update: information on changes to make to the device
#[instrument] #[instrument]
fn update_spec_device( fn update_spec_devices(
spec: &mut Spec, spec: &mut Spec,
devidx: &DevIndex, devidx: &DevIndex,
container_path: &str, updates: HashMap<&str, DevUpdate>,
update: DevUpdate,
) -> Result<()> { ) -> Result<()> {
// If no container_path is provided, we won't be able to match and for (container_path, update) in updates {
// update the device in the OCI spec device list. This is an error. // If no container_path is provided, we won't be able to match and
if container_path.is_empty() { // update the device in the OCI spec device list. This is an error.
return Err(anyhow!("Container path cannot be empty for device")); if container_path.is_empty() {
} return Err(anyhow!("Container path cannot be empty for device"));
let linux = spec
.linux
.as_mut()
.ok_or_else(|| anyhow!("Spec didn't container linux field"))?;
info!(
sl!(),
"update_spec_device() considering device";
"container_path" => container_path,
"guest_major" => update.num.guest_major,
"guest_minor" => update.num.guest_minor,
);
if let Some(idxdata) = devidx.0.get(container_path) {
let dev = &mut linux.devices[idxdata.idx];
let host_major = dev.major;
let host_minor = dev.minor;
dev.major = update.num.guest_major;
dev.minor = update.num.guest_minor;
if let Some(final_path) = update.final_path {
dev.path = final_path;
} }
let linux = spec
.linux
.as_mut()
.ok_or_else(|| anyhow!("Spec didn't container linux field"))?;
info!( info!(
sl!(), sl!(),
"update_spec_device(): updating device"; "update_spec_devices() considering device";
"container_path" => container_path, "container_path" => container_path,
"host_major" => host_major, "guest_major" => update.num.guest_major,
"host_minor" => host_minor, "guest_minor" => update.num.guest_minor,
"updated_path" => &dev.path,
"guest_major" => dev.major,
"guest_minor" => dev.minor,
); );
// Resources must be updated since they are used to identify if let Some(idxdata) = devidx.0.get(container_path) {
// the device in the devices cgroup. let dev = &mut linux.devices[idxdata.idx];
for ridx in &idxdata.residx { let host_major = dev.major;
// unwrap is safe, because residx would be empty if there let host_minor = dev.minor;
// were no resources
let res = &mut linux.resources.as_mut().unwrap().devices[*ridx]; dev.major = update.num.guest_major;
res.major = Some(update.num.guest_major); dev.minor = update.num.guest_minor;
res.minor = Some(update.num.guest_minor); if let Some(final_path) = update.final_path {
dev.path = final_path;
}
info!( info!(
sl!(), sl!(),
"set resources for resource"; "update_spec_devices(): updating device";
"guest_major" => update.num.guest_major, "container_path" => container_path,
"guest_minor" => update.num.guest_minor, "host_major" => host_major,
"host_minor" => host_minor,
"updated_path" => &dev.path,
"guest_major" => dev.major,
"guest_minor" => dev.minor,
); );
// Resources must be updated since they are used to identify
// the device in the devices cgroup.
for ridx in &idxdata.residx {
// unwrap is safe, because residx would be empty if there
// were no resources
let res = &mut linux.resources.as_mut().unwrap().devices[*ridx];
res.major = Some(update.num.guest_major);
res.minor = Some(update.num.guest_minor);
info!(
sl!(),
"set resources for resource";
"guest_major" => update.num.guest_major,
"guest_minor" => update.num.guest_minor,
);
}
} else {
return Err(anyhow!(
"Should have found a device {} in the spec",
container_path
));
} }
Ok(())
} else {
Err(anyhow!(
"Should have found a device {} in the spec",
container_path
))
} }
Ok(())
} }
// device.Id should be the predicted device name (vda, vdb, ...) // device.Id should be the predicted device name (vda, vdb, ...)
@ -759,11 +760,7 @@ pub async fn add_devices(
} }
let devidx = DevIndex::new(spec); let devidx = DevIndex::new(spec);
for (container_path, update) in dev_updates.drain() { update_spec_devices(spec, &devidx, dev_updates)
update_spec_device(spec, &devidx, container_path, update)?;
}
Ok(())
} }
#[instrument] #[instrument]
@ -831,6 +828,7 @@ mod tests {
use super::*; use super::*;
use crate::uevent::spawn_test_watcher; use crate::uevent::spawn_test_watcher;
use oci::Linux; use oci::Linux;
use std::iter::FromIterator;
use tempfile::tempdir; use tempfile::tempdir;
#[test] #[test]
@ -855,7 +853,7 @@ mod tests {
} }
#[test] #[test]
fn test_update_spec_device() { fn test_update_spec_devices() {
let (major, minor) = (7, 2); let (major, minor) = (7, 2);
let mut spec = Spec::default(); let mut spec = Spec::default();
@ -867,22 +865,26 @@ mod tests {
let container_path = ""; let container_path = "";
let vm_path = "/dev/null"; let vm_path = "/dev/null";
let devidx = DevIndex::new(&spec); let devidx = DevIndex::new(&spec);
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
)]),
); );
assert!(res.is_err()); assert!(res.is_err());
// linux is empty // linux is empty
let container_path = "/dev/null"; let container_path = "/dev/null";
let devidx = DevIndex::new(&spec); let devidx = DevIndex::new(&spec);
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
)]),
); );
assert!(res.is_err()); assert!(res.is_err());
@ -890,11 +892,13 @@ mod tests {
// linux.devices is empty // linux.devices is empty
let devidx = DevIndex::new(&spec); let devidx = DevIndex::new(&spec);
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
)]),
); );
assert!(res.is_err()); assert!(res.is_err());
@ -907,11 +911,13 @@ mod tests {
// guest and host path are not the same // guest and host path are not the same
let devidx = DevIndex::new(&spec); let devidx = DevIndex::new(&spec);
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
)]),
); );
assert!( assert!(
res.is_err(), res.is_err(),
@ -925,11 +931,13 @@ mod tests {
// spec.linux.resources is empty // spec.linux.resources is empty
let devidx = DevIndex::new(&spec); let devidx = DevIndex::new(&spec);
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
)]),
); );
assert!(res.is_ok()); assert!(res.is_ok());
@ -951,17 +959,19 @@ mod tests {
}); });
let devidx = DevIndex::new(&spec); let devidx = DevIndex::new(&spec);
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
)]),
); );
assert!(res.is_ok()); assert!(res.is_ok());
} }
#[test] #[test]
fn test_update_spec_device_guest_host_conflict() { fn test_update_spec_devices_guest_host_conflict() {
let null_rdev = fs::metadata("/dev/null").unwrap().rdev(); let null_rdev = fs::metadata("/dev/null").unwrap().rdev();
let zero_rdev = fs::metadata("/dev/zero").unwrap().rdev(); let zero_rdev = fs::metadata("/dev/zero").unwrap().rdev();
let full_rdev = fs::metadata("/dev/full").unwrap().rdev(); let full_rdev = fs::metadata("/dev/full").unwrap().rdev();
@ -1036,24 +1046,17 @@ mod tests {
assert_eq!(Some(host_major_b), specresources.devices[1].major); assert_eq!(Some(host_major_b), specresources.devices[1].major);
assert_eq!(Some(host_minor_b), specresources.devices[1].minor); assert_eq!(Some(host_minor_b), specresources.devices[1].minor);
let update_a = DevNumUpdate::from_vm_path(vm_path_a).unwrap().into(); let updates = HashMap::from_iter(vec![
let res = update_spec_device(&mut spec, &devidx, container_path_a, update_a); (
assert!(res.is_ok()); container_path_a,
DevNumUpdate::from_vm_path(vm_path_a).unwrap().into(),
let specdevices = &spec.linux.as_ref().unwrap().devices; ),
assert_eq!(guest_major_a, specdevices[0].major); (
assert_eq!(guest_minor_a, specdevices[0].minor); container_path_b,
assert_eq!(host_major_b, specdevices[1].major); DevNumUpdate::from_vm_path(vm_path_b).unwrap().into(),
assert_eq!(host_minor_b, specdevices[1].minor); ),
]);
let specresources = spec.linux.as_ref().unwrap().resources.as_ref().unwrap(); let res = update_spec_devices(&mut spec, &devidx, updates);
assert_eq!(Some(guest_major_a), specresources.devices[0].major);
assert_eq!(Some(guest_minor_a), specresources.devices[0].minor);
assert_eq!(Some(host_major_b), specresources.devices[1].major);
assert_eq!(Some(host_minor_b), specresources.devices[1].minor);
let update_b = DevNumUpdate::from_vm_path(vm_path_b).unwrap().into();
let res = update_spec_device(&mut spec, &devidx, container_path_b, update_b);
assert!(res.is_ok()); assert!(res.is_ok());
let specdevices = &spec.linux.as_ref().unwrap().devices; let specdevices = &spec.linux.as_ref().unwrap().devices;
@ -1070,7 +1073,7 @@ mod tests {
} }
#[test] #[test]
fn test_update_spec_device_char_block_conflict() { fn test_update_spec_devices_char_block_conflict() {
let null_rdev = fs::metadata("/dev/null").unwrap().rdev(); let null_rdev = fs::metadata("/dev/null").unwrap().rdev();
let guest_major = stat::major(null_rdev) as i64; let guest_major = stat::major(null_rdev) as i64;
@ -1128,11 +1131,13 @@ mod tests {
assert_eq!(Some(host_major), specresources.devices[1].major); assert_eq!(Some(host_major), specresources.devices[1].major);
assert_eq!(Some(host_minor), specresources.devices[1].minor); assert_eq!(Some(host_minor), specresources.devices[1].minor);
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
)]),
); );
assert!(res.is_ok()); assert!(res.is_ok());
@ -1145,7 +1150,7 @@ mod tests {
} }
#[test] #[test]
fn test_update_spec_device_final_path() { fn test_update_spec_devices_final_path() {
let null_rdev = fs::metadata("/dev/null").unwrap().rdev(); let null_rdev = fs::metadata("/dev/null").unwrap().rdev();
let guest_major = stat::major(null_rdev) as i64; let guest_major = stat::major(null_rdev) as i64;
let guest_minor = stat::minor(null_rdev) as i64; let guest_minor = stat::minor(null_rdev) as i64;
@ -1172,11 +1177,13 @@ mod tests {
let vm_path = "/dev/null"; let vm_path = "/dev/null";
let final_path = "/dev/new"; let final_path = "/dev/new";
let res = update_spec_device( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx, &devidx,
container_path, HashMap::from_iter(vec![(
DevUpdate::from_vm_path(vm_path, final_path.to_string()).unwrap(), container_path,
DevUpdate::from_vm_path(vm_path, final_path.to_string()).unwrap(),
)]),
); );
assert!(res.is_ok()); assert!(res.is_ok());