From a91c1b6b51f89aa51a26cf1d2cd0267fe23f95db Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Wed, 28 Oct 2026 00:43:56 +0100 Subject: [PATCH] refactor(vm): the engine owns a LogLevel, not UI booleans RunOptions carried quiet/verbose, which are flags, not engine state. They are now a LogLevel enum (Quiet/Normal/Verbose) mapped onto libkrun's filter; the CLI maps its flags onto it. The forked child's last-gasp boot diagnostics also stop going straight to stderr: they land in the console file the parent streams, with stderr kept for the case where the console was never configured. lib.rs now states the crate contract: no UI in the library, progress through the sink, logging through the facade, typed errors out. --- SPEC.md | 14 ++++++++--- crates/ecrou-cli/src/main.rs | 11 ++++++--- crates/ecrou/src/lib.rs | 12 +++++++++- crates/ecrou/src/vm.rs | 46 +++++++++++++++++++++++------------- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/SPEC.md b/SPEC.md index 6ef5898..de53576 100644 --- a/SPEC.md +++ b/SPEC.md @@ -14,9 +14,17 @@ keep whatever crossed the workspace share, throw the machine away. The project is library-first like ecr: `crates/ecrou` is the library (package `ecrou`), `crates/ecrou-cli` is the thin front-end (binary `ecrou`) that parses flags, maps them onto the library API, and -propagates exit codes. The library never writes to the terminal: -diagnostics go through the `log` facade, long operations report through -the `Progress` sink, all output is the CLI's. +propagates exit codes. The split is enforced by the dependency edges: +the library links no clap and no rendering crate. It never writes to +the terminal: diagnostics go through the `log` facade, filtered by +`RunOptions::log_level` (an engine-owned enum, not UI booleans), long +operations report through the `Progress` sink, all output is the +CLI's. In-process failures come back as typed errors; the one +exception is the forked machine child's last-gasp diagnostics, which +land in the console file the parent streams. The CLI owns clap, the +indicatif rendering of progress events, the log sink, and exit codes. +A consumer embeds the library exactly the way the CLI does: a +`RootfsCache` to prepare a rootfs, then `run` to boot it. The first consumer is rhema's sandbox seam, which shells out to the binary the way it shells out to `docker` and `lxc` today. The library diff --git a/crates/ecrou-cli/src/main.rs b/crates/ecrou-cli/src/main.rs index fd98edb..cda7070 100644 --- a/crates/ecrou-cli/src/main.rs +++ b/crates/ecrou-cli/src/main.rs @@ -8,7 +8,7 @@ mod progress; use clap::Parser; use ecrou::vm::RunOptions; -use ecrou::{NoProgress, Progress}; +use ecrou::{LogLevel, NoProgress, Progress}; use cli::{parse_env, parse_share, Cli, Command, RunArgs}; use progress::TermProgress; @@ -28,8 +28,13 @@ fn cmd_run(args: &RunArgs, cli_verbose: bool) -> std::process::ExitCode { command: args.command.clone(), memory: args.memory.clone(), workdir: args.workdir.clone(), - quiet: args.quiet, - verbose: cli_verbose, + log_level: if args.quiet { + LogLevel::Quiet + } else if cli_verbose { + LogLevel::Verbose + } else { + LogLevel::Normal + }, no_cache: args.no_cache, ..RunOptions::default() }; diff --git a/crates/ecrou/src/lib.rs b/crates/ecrou/src/lib.rs index b0f53cd..67e6515 100644 --- a/crates/ecrou/src/lib.rs +++ b/crates/ecrou/src/lib.rs @@ -20,6 +20,16 @@ //! ([`Error::KvmUnavailable`], [`Error::ArchNotAvailable`]) versus //! ordinary failures ([`Error::Other`], which preserves the full context //! chain). +//! +//! ## Crate split +//! +//! `ecrou` is the library and carries no UI: progress flows through +//! the [`Progress`] sink, engine logging through the `log` facade +//! filtered by [`LogLevel`], and results come back as typed errors. +//! `ecrou-cli` owns everything a human sees: flags, the indicatif +//! rendering of progress events, the log sink, exit codes. A consumer +//! embeds the library exactly the way the CLI does - a [`RootfsCache`] +//! to prepare a rootfs, then [`run`] to boot it. pub mod config; pub mod distro; @@ -36,4 +46,4 @@ pub mod vm; pub use error::{Error, Result}; pub use progress::{NoProgress, Progress, Unit}; pub use rootfs::{PrepareRequest, PreparedRootfs, RootfsCache}; -pub use vm::{run, RunOptions, Share}; +pub use vm::{run, LogLevel, RunOptions, Share}; diff --git a/crates/ecrou/src/vm.rs b/crates/ecrou/src/vm.rs index 691dc7f..8bf9a4b 100644 --- a/crates/ecrou/src/vm.rs +++ b/crates/ecrou/src/vm.rs @@ -42,12 +42,25 @@ pub struct RunOptions { pub workdir: String, pub env: Vec<(String, String)>, pub hostname: String, - pub quiet: bool, - /// More library diagnostics on stderr - pub verbose: bool, + /// How much of the engine's own logging reaches stderr + pub log_level: LogLevel, pub no_cache: bool, } +/// How much of the engine's own logging reaches stderr. The engine +/// logs through the `log` facade, so the consumer's sink decides where +/// lines land; this only picks the filter. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum LogLevel { + /// Nothing from the engine; the consumer owns all output. + Quiet, + /// Warnings and errors. The default. + #[default] + Normal, + /// Engine info as well: lifecycle lines, boot chatter. + Verbose, +} + impl Default for RunOptions { fn default() -> Self { RunOptions { @@ -60,18 +73,12 @@ impl Default for RunOptions { workdir: "/".to_string(), env: Vec::new(), hostname: String::new(), - quiet: false, - verbose: false, + log_level: LogLevel::Normal, no_cache: false, } } } -/// libkrun log levels (0 off, 1 error, 2 warn, 3 info, 4 debug). -const LOG_LEVEL_WARN: u32 = 2; -const LOG_LEVEL_INFO: u32 = 3; -const LOG_LEVEL_QUIET: u32 = 0; - /// Run one machine to completion and return the payload's exit code. pub fn run(options: &RunOptions, progress: &dyn Progress) -> Result { if options.image.is_empty() { @@ -122,12 +129,10 @@ pub fn run(options: &RunOptions, progress: &dyn Progress) -> Result { ); let _ = std::fs::remove_file(&exit_file_host); - krun.set_log_level(if options.quiet { - LOG_LEVEL_QUIET - } else if options.verbose { - LOG_LEVEL_INFO - } else { - LOG_LEVEL_WARN + krun.set_log_level(match options.log_level { + LogLevel::Quiet => 0, // off + LogLevel::Normal => 2, // warn + LogLevel::Verbose => 3, // info })?; let ctx = krun.create_ctx()?; krun.set_vm_config(ctx, vcpus, ram_mib)?; @@ -165,6 +170,15 @@ pub fn run(options: &RunOptions, progress: &dyn Progress) -> Result { // Safety: after fork the child runs no threads of its own; the // prctl/exit pair has no unwinding preconditions. let bail = |msg: String, code: i32| -> ! { + // The console file is the channel the parent streams: a + // boot failure belongs in it, next to whatever the guest + // printed. stderr carries the last-gasp case where the + // console was never configured; the child is process + // teardown, not library logging. + use std::io::Write as _; + if let Ok(mut f) = std::fs::OpenOptions::new().append(true).open(&console_path) { + let _ = f.write_all(format!("ecrou: {msg}\n").as_bytes()); + } eprintln!("Error: {msg}"); unsafe { libc::_exit(code) } };