Skip to content

Commit

Permalink
Unit tests for DumpSetup behavior
Browse files Browse the repository at this point in the history 
verifies decision-making in different combinations of M.2/U.2 dataset
and dump slice availability and occupancy & tests log/core-archiving.
  • Loading branch information
lif committed Aug 11, 2023
1 parent f816500 commit acfa80c
Show file tree
Hide file tree
Showing 5 changed files with 805 additions and 302 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

99 changes: 47 additions & 52 deletions illumos-utils/src/coreadm.rs
View file
Original file line number Diff line number Diff line change
@@ -1,62 +1,57 @@
use camino::Utf8PathBuf;
use std::ffi::OsString;
use std::os::unix::ffi::OsStringExt;
use crate::{execute, ExecutionError};
use std::process::Command;

#[derive(thiserror::Error, Debug)]
pub enum CoreAdmError {
#[error("Error obtaining or modifying coreadm configuration. core_dir: {core_dir:?}")]
Execution { core_dir: Utf8PathBuf },

#[error("Invalid invocation of coreadm: {0:?} {1:?}")]
InvalidCommand(Vec<String>, OsString),
const COREADM: &str = "/usr/bin/coreadm";

#[error("coreadm process was terminated by a signal.")]
TerminatedBySignal,
pub struct CoreAdm {
cmd: Command,
}

#[error("coreadm invocation exited with unexpected return code {0}")]
UnexpectedExitCode(i32),
pub enum CoreFileOption {
Global,
GlobalSetid,
Log,
Process,
ProcSetid,
}

#[error("Failed to execute dumpadm process: {0}")]
Exec(std::io::Error),
impl AsRef<str> for CoreFileOption {
fn as_ref(&self) -> &str {
match self {
CoreFileOption::Global => "global",
CoreFileOption::GlobalSetid => "global-setid",
CoreFileOption::Log => "log",
CoreFileOption::Process => "process",
CoreFileOption::ProcSetid => "proc-setid",
}
}
}

const COREADM: &str = "/usr/bin/coreadm";
impl CoreAdm {
pub fn new() -> Self {
let mut cmd = Command::new(COREADM);
cmd.env_clear();
Self { cmd }
}

pub fn coreadm(core_dir: &Utf8PathBuf) -> Result<(), CoreAdmError> {
let mut cmd = Command::new(COREADM);
cmd.env_clear();

// disable per-process core patterns
cmd.arg("-d").arg("process");
cmd.arg("-d").arg("proc-setid");

// use the global core pattern
cmd.arg("-e").arg("global");
cmd.arg("-e").arg("global-setid");

// set the global pattern to place all cores into core_dir,
// with filenames of "core.[zone-name].[exe-filename].[pid].[time]"
cmd.arg("-g").arg(core_dir.join("core.%z.%f.%p.%t"));

// also collect DWARF data from the exe and its library deps
cmd.arg("-G").arg("default+debug");

let out = cmd.output().map_err(CoreAdmError::Exec)?;

match out.status.code() {
Some(0) => Ok(()),
Some(1) => Err(CoreAdmError::Execution { core_dir: core_dir.clone() }),
Some(2) => {
// unwrap: every arg we've provided in this function is UTF-8
let mut args =
vec![cmd.get_program().to_str().unwrap().to_string()];
cmd.get_args()
.for_each(|arg| args.push(arg.to_str().unwrap().to_string()));
let stderr = OsString::from_vec(out.stderr);
Err(CoreAdmError::InvalidCommand(args, stderr))
}
Some(n) => Err(CoreAdmError::UnexpectedExitCode(n)),
None => Err(CoreAdmError::TerminatedBySignal),
pub fn disable(&mut self, opt: CoreFileOption) {
self.cmd.arg("-d").arg(opt.as_ref());
}

pub fn enable(&mut self, opt: CoreFileOption) {
self.cmd.arg("-e").arg(opt.as_ref());
}

pub fn global_pattern(&mut self, pat: impl AsRef<std::ffi::OsStr>) {
self.cmd.arg("-g").arg(pat);
}

pub fn global_contents(&mut self, contents: &str) {
self.cmd.arg("-G").arg(contents);
}

pub fn execute(mut self) -> Result<(), ExecutionError> {
execute(&mut self.cmd)?;
Ok(())
}
}
188 changes: 70 additions & 118 deletions illumos-utils/src/dumpadm.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::{execute, ExecutionError};
use byteorder::{LittleEndian, ReadBytesExt};
use camino::Utf8PathBuf;
use std::ffi::OsString;
Expand All @@ -6,6 +7,17 @@ use std::io::{Seek, SeekFrom};
use std::os::unix::ffi::OsStringExt;
use std::process::Command;

pub const DUMPADM: &str = "/usr/sbin/dumpadm";
pub const SAVECORE: &str = "/usr/bin/savecore";

// values from /usr/src/uts/common/sys/dumphdr.h:
pub const DUMP_OFFSET: u64 = 65536; // pad at start/end of dev

pub const DUMP_MAGIC: u32 = 0xdefec8ed; // weird hex but ok
pub const DUMP_VERSION: u32 = 10; // version of this dumphdr

pub const DF_VALID: u32 = 0x00000001; // Dump is valid (savecore clears)

#[derive(thiserror::Error, Debug)]
pub enum DumpHdrError {
#[error("I/O error while attempting to open raw disk: {0}")]
Expand Down Expand Up @@ -39,14 +51,6 @@ pub enum DumpHdrError {
pub fn dump_flag_is_valid(
dump_slice: &Utf8PathBuf,
) -> Result<bool, DumpHdrError> {
// values from /usr/src/uts/common/sys/dumphdr.h:
const DUMP_OFFSET: u64 = 65536; // pad at start/end of dev

const DUMP_MAGIC: u32 = 0xdefec8ed; // weird hex but ok
const DUMP_VERSION: u32 = 10; // version of this dumphdr

const DF_VALID: u32 = 0x00000001; // Dump is valid (savecore clears)

let mut f = File::open(dump_slice).map_err(DumpHdrError::OpenRaw)?;
f.seek(SeekFrom::Start(DUMP_OFFSET)).map_err(DumpHdrError::Seek)?;

Expand Down Expand Up @@ -75,134 +79,82 @@ pub fn dump_flag_is_valid(
Ok((flags & DF_VALID) != 0)
}

const DUMPADM: &str = "/usr/sbin/dumpadm";
const SAVECORE: &str = "/usr/bin/savecore";

#[derive(thiserror::Error, Debug)]
pub enum DumpAdmError {
#[error("Error obtaining or modifying dump configuration. dump_slice: {dump_slice}, savecore_dir: {savecore_dir:?}")]
Execution { dump_slice: Utf8PathBuf, savecore_dir: Option<Utf8PathBuf> },

#[error("Invalid invocation of dumpadm: {0:?} {1:?}")]
InvalidCommand(Vec<String>, OsString),

#[error("dumpadm process was terminated by a signal.")]
TerminatedBySignal,

#[error("dumpadm invocation exited with unexpected return code {0}")]
UnexpectedExitCode(i32),

#[error(
"Failed to create placeholder savecore directory at /tmp/crash: {0}"
)]
Mkdir(std::io::Error),

#[error("savecore failed: {0:?}")]
SavecoreFailure(OsString),
pub enum DumpContentType {
Kernel,
All,
CurProc,
}

#[error("Failed to execute dumpadm process: {0}")]
ExecDumpadm(std::io::Error),
impl AsRef<str> for DumpContentType {
fn as_ref(&self) -> &str {
match self {
DumpContentType::Kernel => "kernel",
DumpContentType::All => "all",
DumpContentType::CurProc => "curproc",
}
}
}

#[error("Failed to execute savecore process: {0}")]
ExecSavecore(std::io::Error),
pub struct DumpAdm {
cmd: Command,
content_type: Option<DumpContentType>,
dump_slice: Utf8PathBuf,
savecore_dir: Utf8PathBuf,
}

/// Invokes `dumpadm(8)` to configure the kernel to dump core into the given
/// `dump_slice` block device in the event of a panic. If a core is already
/// present in that block device, and a `savecore_dir` is provided, this
/// function also invokes `savecore(8)` to save it into that directory.
/// On success, returns Ok(Some(stdout)) if `savecore(8)` was invoked, or
/// Ok(None) if it wasn't.
pub fn dumpadm(
dump_slice: &Utf8PathBuf,
savecore_dir: Option<&Utf8PathBuf>,
) -> Result<Option<OsString>, DumpAdmError> {
let mut cmd = Command::new(DUMPADM);
cmd.env_clear();

// Include memory from the current process if there is one for the panic
// context, in addition to kernel memory:
cmd.arg("-c").arg("curproc");
/// `dump_slice` block device in the event of a panic.
impl DumpAdm {
pub fn new(dump_slice: Utf8PathBuf, savecore_dir: Utf8PathBuf) -> Self {
let mut cmd = Command::new(DUMPADM);
cmd.env_clear();

// Use the given block device path for dump storage:
cmd.arg("-d").arg(dump_slice);
Self { cmd, content_type: None, dump_slice, savecore_dir }
}

// Compress crash dumps:
cmd.arg("-z").arg("on");
pub fn content_type(&mut self, ctype: DumpContentType) {
self.content_type = Some(ctype);
}

// Do not run savecore(8) automatically on boot (irrelevant anyhow, as the
// config file being mutated by dumpadm won't survive reboots on gimlets).
// The sled-agent will invoke it manually instead.
cmd.arg("-n");
pub fn compress(&mut self, on: bool) {
let arg = if on { "on" } else { "off" };
self.cmd.arg("-z").arg(arg);
}

if let Some(savecore_dir) = savecore_dir {
// Run savecore(8) to place the existing contents of dump_slice (if
// any) into savecore_dir, and clear the presence flag.
cmd.arg("-s").arg(savecore_dir);
} else {
// if we don't have a savecore destination yet, still create and use
// a tmpfs path (rather than the default location under /var/crash,
// which is in the ramdisk pool), because dumpadm refuses to do what
// we ask otherwise.
let tmp_crash = "/tmp/crash";
std::fs::create_dir_all(tmp_crash).map_err(DumpAdmError::Mkdir)?;

cmd.arg("-s").arg(tmp_crash);
pub fn no_boot_time_savecore(&mut self) {
self.cmd.arg("-n");
}

let out = cmd.output().map_err(DumpAdmError::ExecDumpadm)?;

match out.status.code() {
Some(0) => {
// do we have a destination for the saved dump
if savecore_dir.is_some() {
// and does the dump slice have one to save off
if let Ok(true) = dump_flag_is_valid(dump_slice) {
return savecore();
}
}
Ok(None)
pub fn execute(mut self) -> Result<(), ExecutionError> {
if let Some(ctype) = self.content_type {
self.cmd.arg("-c").arg(ctype.as_ref());
}
Some(1) => Err(DumpAdmError::Execution {
dump_slice: dump_slice.clone(),
savecore_dir: savecore_dir.cloned(),
}),
Some(2) => {
// unwrap: every arg we've provided in this function is UTF-8
let mut args =
vec![cmd.get_program().to_str().unwrap().to_string()];
cmd.get_args()
.for_each(|arg| args.push(arg.to_str().unwrap().to_string()));
let stderr = OsString::from_vec(out.stderr);
Err(DumpAdmError::InvalidCommand(args, stderr))
}
Some(n) => Err(DumpAdmError::UnexpectedExitCode(n)),
None => Err(DumpAdmError::TerminatedBySignal),
self.cmd.arg("-d").arg(self.dump_slice);
self.cmd.arg("-s").arg(self.savecore_dir);

execute(&mut self.cmd)?;
Ok(())
}
}

// invokes savecore(8) according to the system-wide config set by dumpadm.
// savecore(8) creates a file in the savecore directory called `vmdump.<n>`,
// where `<n>` is the number in the neighboring plaintext file called `bounds`,
// or 0 if the file doesn't exist.
// if savecore(8) successfully copies the data from the dump slice to the
// vmdump file, it clears the "valid" flag in the dump slice's header and
// increments the number in `bounds` by 1.
// in the event that savecore(8) terminates before it finishes copying the
// dump, the incomplete dump will remain in the target directory, but the next
// invocation will overwrite it, because `bounds` wasn't created/incremented.
fn savecore() -> Result<Option<OsString>, DumpAdmError> {
/// Invokes savecore(8) according to the system-wide config set by dumpadm.
/// savecore(8) creates a file in the savecore directory called `vmdump.<n>`,
/// where `<n>` is the number in the neighboring plaintext file called `bounds`,
/// or 0 if the file doesn't exist.
/// If savecore(8) successfully copies the data from the dump slice to the
/// vmdump file, it clears the "valid" flag in the dump slice's header and
/// increments the number in `bounds` by 1.
/// In the event that savecore(8) terminates before it finishes copying the
/// dump, the incomplete dump will remain in the target directory, but the next
/// invocation will overwrite it, because `bounds` wasn't created/incremented.
pub fn savecore() -> Result<Option<OsString>, ExecutionError> {
let mut cmd = Command::new(SAVECORE);
cmd.env_clear();
cmd.arg("-v");
let out = cmd.output().map_err(DumpAdmError::ExecSavecore)?;
if out.status.success() {
if out.stdout.is_empty() || out.stdout == vec![b'\n'] {
Ok(None)
} else {
Ok(Some(OsString::from_vec(out.stdout)))
}
let out = execute(&mut cmd)?;
if out.stdout.is_empty() || out.stdout == vec![b'\n'] {
Ok(None)
} else {
Err(DumpAdmError::SavecoreFailure(OsString::from_vec(out.stderr)))
Ok(Some(OsString::from_vec(out.stdout)))
}
}
1 change: 1 addition & 0 deletions sled-agent/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ hyper.workspace = true
omicron-test-utils.workspace = true
openapi-lint.workspace = true
openapiv3.workspace = true
mockall.workspace = true
pretty_assertions.workspace = true
rcgen.workspace = true
serial_test.workspace = true
Expand Down
Loading

0 comments on commit acfa80c

Please sign in to comment.