mirror of
https://github.com/kata-containers/kata-containers.git
synced 2025-08-16 07:05:14 +00:00
agent/device: Change update_spec_device to handle multiple devices at once
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:
parent
d6a3ebc496
commit
084538d334
@ -503,18 +503,18 @@ 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<()> {
|
||||||
|
for (container_path, update) in updates {
|
||||||
// If no container_path is provided, we won't be able to match and
|
// If no container_path is provided, we won't be able to match and
|
||||||
// update the device in the OCI spec device list. This is an error.
|
// update the device in the OCI spec device list. This is an error.
|
||||||
if container_path.is_empty() {
|
if container_path.is_empty() {
|
||||||
@ -528,7 +528,7 @@ fn update_spec_device(
|
|||||||
|
|
||||||
info!(
|
info!(
|
||||||
sl!(),
|
sl!(),
|
||||||
"update_spec_device() considering device";
|
"update_spec_devices() considering device";
|
||||||
"container_path" => container_path,
|
"container_path" => container_path,
|
||||||
"guest_major" => update.num.guest_major,
|
"guest_major" => update.num.guest_major,
|
||||||
"guest_minor" => update.num.guest_minor,
|
"guest_minor" => update.num.guest_minor,
|
||||||
@ -547,7 +547,7 @@ fn update_spec_device(
|
|||||||
|
|
||||||
info!(
|
info!(
|
||||||
sl!(),
|
sl!(),
|
||||||
"update_spec_device(): updating device";
|
"update_spec_devices(): updating device";
|
||||||
"container_path" => container_path,
|
"container_path" => container_path,
|
||||||
"host_major" => host_major,
|
"host_major" => host_major,
|
||||||
"host_minor" => host_minor,
|
"host_minor" => host_minor,
|
||||||
@ -572,13 +572,14 @@ fn update_spec_device(
|
|||||||
"guest_minor" => update.num.guest_minor,
|
"guest_minor" => update.num.guest_minor,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
Ok(())
|
|
||||||
} else {
|
} else {
|
||||||
Err(anyhow!(
|
return Err(anyhow!(
|
||||||
"Should have found a device {} in the spec",
|
"Should have found a device {} in the spec",
|
||||||
container_path
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
|
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,
|
||||||
|
HashMap::from_iter(vec![(
|
||||||
container_path,
|
container_path,
|
||||||
DevUpdate::from_vm_path(vm_path, final_path.to_string()).unwrap(),
|
DevUpdate::from_vm_path(vm_path, final_path.to_string()).unwrap(),
|
||||||
|
)]),
|
||||||
);
|
);
|
||||||
assert!(res.is_ok());
|
assert!(res.is_ok());
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user