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

agent/device: Obtain guest major/minor numbers when creating DevNumUpdate

Browse Source 
Currently the DevNumUpdate structure is created with a path to a
device node in the VM, which is then used by update_spec_device().
However the only piece of information that update_spec_device()
actually needs is the VM side major and minor numbers for the device.
We can determine those when we create the DevNumUpdate structure.
This means we detect errors earlier and as a bonus we don't need to
make a copy of the vm path string.

Since that change requires updating 2 of the log statements, we take the
opportunity to update all the log statements to structured style.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This commit is contained in:
David Gibson 2021-11-04 16:43:45 +11:00
parent f4982130e1
commit d6a3ebc496
1 changed files with 39 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -11,7 +11,7 @@ use std::fmt;
use std::fs; use std::fs;
use std::os::unix::ffi::OsStrExt; use std::os::unix::ffi::OsStrExt;
use std::os::unix::fs::MetadataExt; use std::os::unix::fs::MetadataExt;
use std::path::{Path, PathBuf}; use std::path::Path;
use std::str::FromStr; use std::str::FromStr;
use std::sync::Arc; use std::sync::Arc;
use tokio::sync::Mutex; use tokio::sync::Mutex;
@ -437,15 +437,26 @@ fn scan_scsi_bus(scsi_addr: &str) -> Result<()> {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
struct DevNumUpdate { struct DevNumUpdate {
// the path to the equivalent device in the VM (used to determine // the major and minor numbers for the device within the guest
// the correct major/minor numbers) guest_major: i64,
vm_path: PathBuf, guest_minor: i64,
} }
impl DevNumUpdate { impl DevNumUpdate {
fn from_vm_path<T: AsRef<Path>>(vm_path: T) -> Result<Self> { fn from_vm_path<T: AsRef<Path>>(vm_path: T) -> Result<Self> {
let vm_path = vm_path.as_ref();
if !vm_path.exists() {
return Err(anyhow!("VM device path {:?} doesn't exist", vm_path));
}
let devid = fs::metadata(vm_path)?.rdev();
let guest_major = stat::major(devid) as i64;
let guest_minor = stat::minor(devid) as i64;
Ok(DevNumUpdate { Ok(DevNumUpdate {
vm_path: vm_path.as_ref().to_owned(), guest_major,
guest_minor,
}) })
} }
} }
@ -515,24 +526,12 @@ fn update_spec_device(
.as_mut() .as_mut()
.ok_or_else(|| anyhow!("Spec didn't container linux field"))?; .ok_or_else(|| anyhow!("Spec didn't container linux field"))?;
if !update.num.vm_path.exists() {
return Err(anyhow!(
"vm_path:{} doesn't exist",
update.num.vm_path.display()
));
}
let meta = fs::metadata(&update.num.vm_path)?;
let dev_id = meta.rdev();
let major_id = stat::major(dev_id);
let minor_id = stat::minor(dev_id);
info!( info!(
sl!(), sl!(),
"update_spec_device(): vm_path={}, major: {}, minor: {}\n", "update_spec_device() considering device";
update.num.vm_path.display(), "container_path" => container_path,
major_id, "guest_major" => update.num.guest_major,
minor_id "guest_minor" => update.num.guest_minor,
); );
if let Some(idxdata) = devidx.0.get(container_path) { if let Some(idxdata) = devidx.0.get(container_path) {
@ -540,21 +539,21 @@ fn update_spec_device(
let host_major = dev.major; let host_major = dev.major;
let host_minor = dev.minor; let host_minor = dev.minor;
dev.major = major_id as i64; dev.major = update.num.guest_major;
dev.minor = minor_id as i64; dev.minor = update.num.guest_minor;
if let Some(final_path) = update.final_path { if let Some(final_path) = update.final_path {
dev.path = final_path; dev.path = final_path;
} }
info!( info!(
sl!(), sl!(),
"change the device from path: {} major: {} minor: {} to vm device path: {} major: {} minor: {}", "update_spec_device(): updating device";
container_path, "container_path" => container_path,
host_major, "host_major" => host_major,
host_minor, "host_minor" => host_minor,
dev.path, "updated_path" => &dev.path,
dev.major, "guest_major" => dev.major,
dev.minor, "guest_minor" => dev.minor,
); );
// Resources must be updated since they are used to identify // Resources must be updated since they are used to identify
@ -563,12 +562,14 @@ fn update_spec_device(
// 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];
res.major = Some(major_id as i64); res.major = Some(update.num.guest_major);
res.minor = Some(minor_id as i64); res.minor = Some(update.num.guest_minor);
info!( info!(
sl!(), sl!(),
"set resources for device major: {} minor: {}\n", major_id, minor_id "set resources for resource";
"guest_major" => update.num.guest_major,
"guest_minor" => update.num.guest_minor,
); );
} }
Ok(()) Ok(())
@ -858,9 +859,13 @@ mod tests {
let (major, minor) = (7, 2); let (major, minor) = (7, 2);
let mut spec = Spec::default(); let mut spec = Spec::default();
// vm_path empty
let update = DevNumUpdate::from_vm_path("");
assert!(update.is_err());
// container_path empty // container_path empty
let container_path = ""; let container_path = "";
let vm_path = ""; let vm_path = "/dev/null";
let devidx = DevIndex::new(&spec); let devidx = DevIndex::new(&spec);
let res = update_spec_device( let res = update_spec_device(
&mut spec, &mut spec,
@ -900,18 +905,6 @@ mod tests {
..oci::LinuxDevice::default() ..oci::LinuxDevice::default()
}]; }];
// vm_path empty
let devidx = DevIndex::new(&spec);
let res = update_spec_device(
&mut spec,
&devidx,
container_path,
DevNumUpdate::from_vm_path(vm_path).unwrap().into(),
);
assert!(res.is_err());
let vm_path = "/dev/null";
// 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_device(