Skip to content

Commit

Permalink
chore(kernel): Add even more error context.
Browse files Browse the repository at this point in the history
  • Loading branch information
@vifino
vifino committed Feb 5, 2024
1 parent 87eddd7 commit 1c8be0f
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 43 deletions.
168 changes: 128 additions & 40 deletions src/kernel/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,42 @@ impl KernelConfig {
let mut state = State::default();

// Gather ports.
for port in NvmetRoot::list_ports()? {
for port in NvmetRoot::list_ports().context("Failed to gather port list")? {
if let Ok(port_type) = port.get_type() {
let subs = port.list_subsystems()?;
let subs = port.list_subsystems().with_context(|| {
format!("Failed to gather subsystem state for port {}", port.id)
})?;
state.ports.insert(port.id, Port::new(port_type, subs));
}
}

// Gather subsystems.
for subsystem in NvmetRoot::list_subsystems()? {
for subsystem in NvmetRoot::list_subsystems().context("Failed to gather subsystem list")? {
// Gather namespaces of subsystem.
let mut namespaces = BTreeMap::<u32, Namespace>::new();
for (nsid, nvmetns) in subsystem.list_namespaces()? {
let ns = nvmetns.get_namespace()?;
let ns = nvmetns.get_namespace().with_context(|| {
format!(
"Failed to get namespace {} for subsystem {}",
nsid, subsystem.nqn
)
})?;
namespaces.insert(nsid, ns);
}

let sub = Subsystem {
model: Some(subsystem.get_model()?),
serial: Some(subsystem.get_serial()?),
allowed_hosts: subsystem.list_hosts()?,
model: Some(subsystem.get_model().with_context(|| {
format!("Failed to gather model for subsystem {}", subsystem.nqn)
})?),
serial: Some(subsystem.get_serial().with_context(|| {
format!("Failed to gather serial for subsystem {}", subsystem.nqn)
})?),
allowed_hosts: subsystem.list_hosts().with_context(|| {
format!(
"Failed to gather allowed hosts for subsystem {}",
subsystem.nqn
)
})?,
namespaces,
};
state.subsystems.insert(subsystem.nqn, sub);
Expand All @@ -48,91 +64,163 @@ impl KernelConfig {
for change in changes {
match change {
StateDelta::AddPort(id, port) => {
let p = NvmetRoot::create_port(id)?;
p.set_type(port.port_type)?;
let p = NvmetRoot::create_port(id)
.with_context(|| format!("Failed to add new port {id}"))?;
p.set_type(port.port_type)
.with_context(|| format!("Failed to set new port type for port {id}"))?;
for sub in &port.subsystems {
assert_valid_nqn(sub)?;
assert_valid_nqn(sub).with_context(|| {
format!("Failed to validate new port subsystems for port {id}")
})?;
}
p.set_subsystems(&port.subsystems)?;
p.set_subsystems(&port.subsystems).with_context(|| {
format!("Failed to set new port subsystems for port {id}")
})?;
}
StateDelta::UpdatePort(id, deltas) => {
if !NvmetRoot::has_port(id)? {
return Err(Error::NoSuchPort(id).into());
return Err(Into::<anyhow::Error>::into(Error::NoSuchPort(id)))
.with_context(|| format!("Failed to update port {id}"));
}
let p = NvmetRoot::open_port(id);
for delta in deltas {
match delta {
PortDelta::UpdatePortType(pt) => p.set_type(pt).with_context(|| {
format!("Failed to update port type of port {id}")
})?,
PortDelta::AddSubsystem(nqn) => p.enable_subsystem(&nqn)?,
PortDelta::RemoveSubsystem(nqn) => p.disable_subsystem(&nqn)?,
PortDelta::AddSubsystem(nqn) => {
p.enable_subsystem(&nqn).with_context(|| {
format!("Failed to add subsystem {nqn} to port {id}")
})?
}
PortDelta::RemoveSubsystem(nqn) => {
p.disable_subsystem(&nqn).with_context(|| {
format!("Failed to remove subsytem {nqn} from port {id}")
})?
}
}
}
}
StateDelta::RemovePort(id) => {
NvmetRoot::delete_port(id)?;
NvmetRoot::delete_port(id)
.with_context(|| format!("Failed to remove port {id}"))?;
}

StateDelta::AddSubsystem(nqn, sub) => {
if NvmetRoot::has_subsystem(&nqn)? {
return Err(Error::ExistingSubsystem(nqn).into());
return Err(Into::<anyhow::Error>::into(Error::ExistingSubsystem(
nqn.to_owned(),
)))
.with_context(|| format!("Failed to add new subsystem {nqn}"));
}
let nvmetsub = NvmetRoot::create_subsystem(&nqn)?;
let nvmetsub = NvmetRoot::create_subsystem(&nqn)
.with_context(|| format!("Failed to add new subsystem {nqn}"))?;
if let Some(model) = sub.model {
nvmetsub.set_model(&model)?;
nvmetsub.set_model(&model).with_context(|| {
format!("Failed to set model for new subsystem {nqn}")
})?;
}
if let Some(serial) = sub.serial {
nvmetsub.set_serial(&serial)?;
nvmetsub.set_serial(&serial).with_context(|| {
format!("Failed to set serial for new subsystem {nqn}")
})?;
}
nvmetsub.set_namespaces(&sub.namespaces)?;
nvmetsub.set_hosts(&sub.allowed_hosts)?;
nvmetsub.set_namespaces(&sub.namespaces).with_context(|| {
format!("Failed to add namespaces for new subsystem {nqn}")
})?;
nvmetsub.set_hosts(&sub.allowed_hosts).with_context(|| {
format!("Failed to set allowed hosts for new subsystem {nqn}")
})?;
}
StateDelta::UpdateSubsystem(nqn, deltas) => {
if !NvmetRoot::has_subsystem(&nqn)? {
return Err(Error::NoSuchSubsystem(nqn).into());
return Err(Into::<anyhow::Error>::into(Error::NoSuchSubsystem(
nqn.to_owned(),
)))
.with_context(|| format!("Failed to update existing subsystem {nqn}"));
}
let nvmetsub = NvmetRoot::open_subsystem(&nqn)?;
let nvmetsub = NvmetRoot::open_subsystem(&nqn)
.with_context(|| format!("Failed to update subsystem {nqn}"))?;
for delta in deltas {
match delta {
SubsystemDelta::UpdateModel(model) => nvmetsub.set_model(&model)?,
SubsystemDelta::UpdateSerial(serial) => nvmetsub.set_serial(&serial)?,
SubsystemDelta::AddHost(host) => nvmetsub.enable_host(&host)?,
SubsystemDelta::RemoveHost(host) => nvmetsub.disable_host(&host)?,
SubsystemDelta::UpdateModel(model) => {
nvmetsub.set_model(&model).with_context(|| {
format!("Failed to update model for subsystem {nqn}")
})?
}
SubsystemDelta::UpdateSerial(serial) => {
nvmetsub.set_serial(&serial).with_context(|| {
format!("Failed to update serial for subsystem {nqn}")
})?
}
SubsystemDelta::AddHost(host) => {
nvmetsub.enable_host(&host).with_context(|| {
format!("Failed to add allowed host to subsystem {nqn}")
})?
}
SubsystemDelta::RemoveHost(host) => {
nvmetsub.disable_host(&host).with_context(|| {
format!("Failed to remove allowed host from subsystem {nqn}")
})?
}
SubsystemDelta::AddNamespace(nsid, ns) => {
let nvmetns = nvmetsub.create_namespace(nsid)?;
nvmetns.set_namespace(&ns)?;
let nvmetns =
nvmetsub.create_namespace(nsid).with_context(|| {
format!("Failed to add namespace for subsystem {nqn}")
})?;
nvmetns.set_namespace(&ns).with_context(|| {
format!("Failed to set new namespace for subsystem {nqn}")
})?;
}
SubsystemDelta::UpdateNamespace(nsid, ns) => {
let nvmetns = nvmetsub.open_namespace(nsid)?;
nvmetns.set_namespace(&ns)?;
let nvmetns = nvmetsub.open_namespace(nsid).with_context(|| {
format!("Failed to update namespace for subsystem {nqn}")
})?;
nvmetns.set_namespace(&ns).with_context(|| {
format!("Failed to update namespace for subsystem {nqn}")
})?;
}
SubsystemDelta::RemoveNamespace(nsid) => {
nvmetsub.delete_namespace(nsid)?;
nvmetsub.delete_namespace(nsid).with_context(|| {
format!("Failed to remove namespace for subsystem {nqn}")
})?;
}
}
}
}
StateDelta::RemoveSubsystem(nqn) => {
if !NvmetRoot::has_subsystem(&nqn)? {
return Err(Error::NoSuchSubsystem(nqn).into());
return Err(Into::<anyhow::Error>::into(Error::NoSuchSubsystem(
nqn.to_owned(),
)))
.with_context(|| format!("Failed to remove existing subsystem {nqn}"));
}

// Fetch global hosts just before we remove the subsystem.
let prev_hosts = NvmetRoot::list_hosts()?;
let our_hosts = NvmetRoot::open_subsystem(&nqn)?.list_hosts()?;
let prev_hosts = NvmetRoot::list_hosts()
.with_context(|| format!("Failed to list all allowed hosts before removing existing subsystem {nqn}"))?;
let our_hosts = NvmetRoot::open_subsystem(&nqn)?
.list_hosts()
.with_context(|| format!("Failed to list subsystem hosts before removing existing subsystem {nqn}"))?;

// Before removing the subsystem, we need to remove all references to it.
for port in NvmetRoot::list_ports()? {
if port.has_subsystem(&nqn)? {
port.disable_subsystem(&nqn).with_context(|| format!("Failed to disable subsystem from all ports before removing subsystem {nqn}"))?;
for port in NvmetRoot::list_ports().with_context(|| {
format!("Failed to list ports before removing existing subsystem {nqn}")
})? {
if port.has_subsystem(&nqn).with_context(|| {
format!(
"Failed to check if port has subsystem {nqn} before removing it"
)
})? {
port.disable_subsystem(&nqn).with_context(|| format!("Failed to disable subsystem {nqn} from all ports before removing it"))?;
}
}

NvmetRoot::delete_subsystem(&nqn)?;
NvmetRoot::delete_subsystem(&nqn)
.with_context(|| format!("Failed to remove subsystem {nqn}"))?;

// Iterate over all remaining subsystems and find what host we're missing now.
let current_hosts = NvmetRoot::list_hosts()?;
let current_hosts = NvmetRoot::list_hosts().with_context(|| format!("Failed to list all allowed hosts before removing existing subsystem {nqn}"))?;
for unused_host in prev_hosts.difference(&current_hosts) {
if our_hosts.contains(unused_host) {
NvmetRoot::remove_host(unused_host).with_context(|| {
Expand Down
24 changes: 21 additions & 3 deletions src/kernel/sysfs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,15 @@ impl NvmetNamespace {
pub(super) fn set_device_path(&self, dev: &PathBuf) -> Result<()> {
let path = Path::new(dev);
// TODO: is it possible to mount a file instead? there is a mysterious "buffered_io" file..
let metadata = std::fs::metadata(path)?.file_type();
let metadata = std::fs::metadata(path)
.with_context(|| {
format!(
"Failed to get metadata for device {} in namespace {}",
dev.display(),
self.nsid
)
})?
.file_type();
if !metadata.is_block_device() {
return Err(Error::InvalidDevice(dev.display().to_string()).into());
}
Expand Down Expand Up @@ -561,7 +569,12 @@ impl NvmetNamespace {
}
pub(super) fn set_namespace(&self, ns: &Namespace) -> Result<()> {
// Always need to disable before applying changes.
self.set_enabled(false)?;
self.set_enabled(false).with_context(|| {
format!(
"Failed to disable namespace {} before applying changes",
self.nsid
)
})?;

self.set_device_path(&ns.device_path)?;
if let Some(uuid) = ns.device_uuid {
Expand All @@ -571,7 +584,12 @@ impl NvmetNamespace {
self.set_device_nguid(&nguid)?;
}

self.set_enabled(ns.enabled)?;
self.set_enabled(ns.enabled).with_context(|| {
format!(
"Failed to enable namespace {} after applying changes",
self.nsid
)
})?;

Ok(())
}
Expand Down

0 comments on commit 1c8be0f

Please sign in to comment.