Unit tests for DumpSetup

Cargo.toml

@@ -408,7 +408,7 @@ wicket-common = { path = "wicket-common" }
 wicketd-client = { path = "clients/wicketd-client" }
 zeroize = { version = "1.7.0", features = ["zeroize_derive", "std"] }
 zip = { version = "0.6.6", default-features = false, features = ["deflate","bzip2"] }
-zone = { version = "0.3", default-features = false, features = ["async"] }
+zone = { version = "0.3", default-features = false, features = ["async", "sync"] }
 
 # NOTE: The test profile inherits from the dev profile, so settings under
 # profile.dev get inherited. AVOID setting anything under profile.test: that

illumos-utils/src/coreadm.rs

@@ -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(())
     }
 }

illumos-utils/src/dumpadm.rs

@@ -1,3 +1,4 @@
+use crate::{execute, ExecutionError};
 use byteorder::{LittleEndian, ReadBytesExt};
 use camino::Utf8PathBuf;
 use std::ffi::OsString;
@@ -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}")]
@@ -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)?;
 
@@ -75,134 +79,86 @@ 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),
+pub enum DumpContentType {
+    Kernel,
+    All,
+    CurProc,
+}
 
-    #[error("dumpadm process was terminated by a signal.")]
-    TerminatedBySignal,
+impl AsRef<str> for DumpContentType {
+    fn as_ref(&self) -> &str {
+        match self {
+            DumpContentType::Kernel => "kernel",
+            DumpContentType::All => "all",
+            DumpContentType::CurProc => "curproc",
+        }
+    }
+}
 
-    #[error("dumpadm invocation exited with unexpected return code {0}")]
-    UnexpectedExitCode(i32),
+/// Invokes `dumpadm(8)` to configure the kernel to dump core into the given
+/// `dump_slice` block device in the event of a panic.
+pub struct DumpAdm {
+    cmd: Command,
+    content_type: Option<DumpContentType>,
+    dump_slice: Utf8PathBuf,
+    savecore_dir: Utf8PathBuf,
+}
 
-    #[error(
-        "Failed to create placeholder savecore directory at /tmp/crash: {0}"
-    )]
-    Mkdir(std::io::Error),
+impl DumpAdm {
+    pub fn new(dump_slice: Utf8PathBuf, savecore_dir: Utf8PathBuf) -> Self {
+        let mut cmd = Command::new(DUMPADM);
+        cmd.env_clear();
 
-    #[error("savecore failed: {0:?}")]
-    SavecoreFailure(OsString),
+        Self { cmd, content_type: None, dump_slice, savecore_dir }
+    }
 
-    #[error("Failed to execute dumpadm process: {0}")]
-    ExecDumpadm(std::io::Error),
+    pub fn content_type(&mut self, ctype: DumpContentType) {
+        self.content_type = Some(ctype);
+    }
 
-    #[error("Failed to execute savecore process: {0}")]
-    ExecSavecore(std::io::Error),
-}
+    pub fn compress(&mut self, on: bool) {
+        let arg = if on { "on" } else { "off" };
+        self.cmd.arg("-z").arg(arg);
+    }
 
-/// 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");
-
-    // Use the given block device path for dump storage:
-    cmd.arg("-d").arg(dump_slice);
-
-    // Compress crash dumps:
-    cmd.arg("-z").arg("on");
-
-    // 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");
-
-    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)
-        }
-        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))
+    pub fn execute(mut self) -> Result<(), ExecutionError> {
+        if let Some(ctype) = self.content_type {
+            self.cmd.arg("-c").arg(ctype.as_ref());
         }
-        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> {
-    let mut cmd = Command::new(SAVECORE);
-    cmd.env_clear();
-    cmd.arg("-v");
-    let out = cmd.output().map_err(DumpAdmError::ExecSavecore)?;
-    if out.status.success() {
+pub struct SaveCore;
+
+impl SaveCore {
+    /// 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 execute(&self) -> Result<Option<OsString>, ExecutionError> {
+        let mut cmd = Command::new(SAVECORE);
+        cmd.env_clear();
+        cmd.arg("-v");
+        let out = execute(&mut cmd)?;
         if out.stdout.is_empty() || out.stdout == vec![b'\n'] {
             Ok(None)
         } else {
             Ok(Some(OsString::from_vec(out.stdout)))
         }
-    } else {
-        Err(DumpAdmError::SavecoreFailure(OsString::from_vec(out.stderr)))
     }
 }