agent/device: Make DevIndex local to update_spec_devices()

The DevIndex data structure keeps track of devices in the OCI
specification.  We used to carry it around to quite a lot of
functions, but it's now used only within update_spec_devices().  That
means we can simplify things a bit by just open coding the maps we
need, rather than declaring a special type.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
David Gibson 2021-11-04 20:46:10 +11:00
parent 084538d334
commit c855a312f0

View File

@ -52,15 +52,6 @@ pub const DRIVER_VFIO_GK_TYPE: &str = "vfio-gk";
// container as a VFIO device node // container as a VFIO device node
pub const DRIVER_VFIO_TYPE: &str = "vfio"; pub const DRIVER_VFIO_TYPE: &str = "vfio";
#[derive(Debug)]
struct DevIndexEntry {
idx: usize,
residx: Vec<usize>,
}
#[derive(Debug)]
struct DevIndex(HashMap<String, DevIndexEntry>);
#[instrument] #[instrument]
pub fn online_device(path: &str) -> Result<()> { pub fn online_device(path: &str) -> Result<()> {
fs::write(path, "1")?; fs::write(path, "1")?;
@ -509,11 +500,27 @@ impl<T: Into<DevUpdate>> From<T> for SpecUpdate {
// 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_devices( fn update_spec_devices(spec: &mut Spec, updates: HashMap<&str, DevUpdate>) -> Result<()> {
spec: &mut Spec, let linux = spec
devidx: &DevIndex, .linux
updates: HashMap<&str, DevUpdate>, .as_mut()
) -> Result<()> { .ok_or_else(|| anyhow!("Spec didn't contain linux field"))?;
let mut devidx = HashMap::<String, (usize, Vec<usize>)>::new();
for (i, d) in linux.devices.iter().enumerate() {
let mut residx = Vec::new();
if let Some(linuxres) = linux.resources.as_ref() {
for (j, r) in linuxres.devices.iter().enumerate() {
if r.r#type == d.r#type && r.major == Some(d.major) && r.minor == Some(d.minor) {
residx.push(j);
}
}
}
devidx.insert(d.path.clone(), (i, residx));
}
for (container_path, update) in updates { 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.
@ -521,11 +528,6 @@ fn update_spec_devices(
return Err(anyhow!("Container path cannot be empty for device")); 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!( info!(
sl!(), sl!(),
"update_spec_devices() considering device"; "update_spec_devices() considering device";
@ -534,8 +536,8 @@ fn update_spec_devices(
"guest_minor" => update.num.guest_minor, "guest_minor" => update.num.guest_minor,
); );
if let Some(idxdata) = devidx.0.get(container_path) { if let Some(idxdata) = devidx.get(container_path) {
let dev = &mut linux.devices[idxdata.idx]; let dev = &mut linux.devices[idxdata.0];
let host_major = dev.major; let host_major = dev.major;
let host_minor = dev.minor; let host_minor = dev.minor;
@ -558,7 +560,7 @@ fn update_spec_devices(
// Resources must be updated since they are used to identify // Resources must be updated since they are used to identify
// the device in the devices cgroup. // the device in the devices cgroup.
for ridx in &idxdata.residx { for ridx in &idxdata.1 {
// unwrap is safe, because residx would be empty if there // unwrap is safe, because residx would be empty if there
// were no resources // were no resources
let res = &mut linux.resources.as_mut().unwrap().devices[*ridx]; let res = &mut linux.resources.as_mut().unwrap().devices[*ridx];
@ -711,31 +713,6 @@ async fn vfio_device_handler(device: &Device, sandbox: &Arc<Mutex<Sandbox>>) ->
}) })
} }
impl DevIndex {
fn new(spec: &Spec) -> DevIndex {
let mut map = HashMap::new();
if let Some(linux) = spec.linux.as_ref() {
for (i, d) in linux.devices.iter().enumerate() {
let mut residx = Vec::new();
if let Some(linuxres) = linux.resources.as_ref() {
for (j, r) in linuxres.devices.iter().enumerate() {
if r.r#type == d.r#type
&& r.major == Some(d.major)
&& r.minor == Some(d.minor)
{
residx.push(j);
}
}
}
map.insert(d.path.clone(), DevIndexEntry { idx: i, residx });
}
}
DevIndex(map)
}
}
#[instrument] #[instrument]
pub async fn add_devices( pub async fn add_devices(
devices: &[Device], devices: &[Device],
@ -759,8 +736,7 @@ pub async fn add_devices(
} }
} }
let devidx = DevIndex::new(spec); update_spec_devices(spec, dev_updates)
update_spec_devices(spec, &devidx, dev_updates)
} }
#[instrument] #[instrument]
@ -864,10 +840,8 @@ mod tests {
// container_path empty // container_path empty
let container_path = ""; let container_path = "";
let vm_path = "/dev/null"; let vm_path = "/dev/null";
let devidx = DevIndex::new(&spec);
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( HashMap::from_iter(vec![(
container_path, container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
@ -877,10 +851,8 @@ mod tests {
// linux is empty // linux is empty
let container_path = "/dev/null"; let container_path = "/dev/null";
let devidx = DevIndex::new(&spec);
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( HashMap::from_iter(vec![(
container_path, container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
@ -891,10 +863,8 @@ mod tests {
spec.linux = Some(Linux::default()); spec.linux = Some(Linux::default());
// linux.devices is empty // linux.devices is empty
let devidx = DevIndex::new(&spec);
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( HashMap::from_iter(vec![(
container_path, container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
@ -910,10 +880,8 @@ mod tests {
}]; }];
// guest and host path are not the same // guest and host path are not the same
let devidx = DevIndex::new(&spec);
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( HashMap::from_iter(vec![(
container_path, container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
@ -930,10 +898,8 @@ mod tests {
spec.linux.as_mut().unwrap().devices[0].path = container_path.to_string(); spec.linux.as_mut().unwrap().devices[0].path = container_path.to_string();
// spec.linux.resources is empty // spec.linux.resources is empty
let devidx = DevIndex::new(&spec);
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( HashMap::from_iter(vec![(
container_path, container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
@ -958,10 +924,8 @@ mod tests {
..oci::LinuxResources::default() ..oci::LinuxResources::default()
}); });
let devidx = DevIndex::new(&spec);
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( HashMap::from_iter(vec![(
container_path, container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
@ -1020,7 +984,6 @@ mod tests {
}), }),
..Spec::default() ..Spec::default()
}; };
let devidx = DevIndex::new(&spec);
let container_path_a = "/dev/a"; let container_path_a = "/dev/a";
let vm_path_a = "/dev/zero"; let vm_path_a = "/dev/zero";
@ -1056,7 +1019,7 @@ mod tests {
DevNumUpdate::from_vm_path(vm_path_b).unwrap().into(), DevNumUpdate::from_vm_path(vm_path_b).unwrap().into(),
), ),
]); ]);
let res = update_spec_devices(&mut spec, &devidx, updates); let res = update_spec_devices(&mut spec, updates);
assert!(res.is_ok()); assert!(res.is_ok());
let specdevices = &spec.linux.as_ref().unwrap().devices; let specdevices = &spec.linux.as_ref().unwrap().devices;
@ -1120,7 +1083,6 @@ mod tests {
}), }),
..Spec::default() ..Spec::default()
}; };
let devidx = DevIndex::new(&spec);
let container_path = "/dev/char"; let container_path = "/dev/char";
let vm_path = "/dev/null"; let vm_path = "/dev/null";
@ -1133,7 +1095,6 @@ mod tests {
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( HashMap::from_iter(vec![(
container_path, container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(), DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
@ -1172,14 +1133,12 @@ mod tests {
}), }),
..Spec::default() ..Spec::default()
}; };
let devidx = DevIndex::new(&spec);
let vm_path = "/dev/null"; let vm_path = "/dev/null";
let final_path = "/dev/new"; let final_path = "/dev/new";
let res = update_spec_devices( let res = update_spec_devices(
&mut spec, &mut spec,
&devidx,
HashMap::from_iter(vec![( 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(),