diff --git a/local/recipes/system/driver-manager/source/src/main.rs b/local/recipes/system/driver-manager/source/src/main.rs index 8e75b5ee9d..7ef2f07e66 100644 --- a/local/recipes/system/driver-manager/source/src/main.rs +++ b/local/recipes/system/driver-manager/source/src/main.rs @@ -330,9 +330,13 @@ fn main() { if policy_disabled { " (SUPPRESSED)" } else { "" }, if driver_options.len() == 1 { "" } else { "s" } ); - config::set_global_shared_driver_options(crate::policy::SharedDriverOptions::from_static( + // N9: capture the SharedDriverOptions so the SIGHUP worker can + // re-read it on operator reload. main.rs owns the live reference; + // sighup::spawn_reload_worker takes another Arc to the same object. + let shared_options = std::sync::Arc::new(crate::policy::SharedDriverOptions::from_static( driver_options.clone(), )); + config::set_global_shared_driver_options((*shared_options).clone()); // Autoload list (N2) let autoload_dir = policy_path.join("autoload.d"); @@ -354,9 +358,11 @@ fn main() { autoload_dir.display(), if policy_disabled { " (SUPPRESSED)" } else { "" } ); - config::set_global_shared_autoload_list(crate::policy::SharedAutoloadList::from_static( + // N9: capture the SharedAutoloadList for SIGHUP reload. + let shared_autoload = std::sync::Arc::new(crate::policy::SharedAutoloadList::from_static( autoload_list.clone(), )); + config::set_global_shared_autoload_list((*shared_autoload).clone()); // Initfs manifest (N3) let initfs_manifest = if policy_disabled { @@ -379,9 +385,12 @@ fn main() { policy_dir, if policy_disabled { " (SUPPRESSED)" } else { "" } ); - config::set_global_shared_initfs_manifest(crate::policy::SharedInitfsManifest::from_static( - initfs_manifest.clone(), - )); + // N9: capture the SharedInitfsManifest for SIGHUP reload. + let shared_initfs_manifest = + std::sync::Arc::new(crate::policy::SharedInitfsManifest::from_static( + initfs_manifest.clone(), + )); + config::set_global_shared_initfs_manifest((*shared_initfs_manifest).clone()); if export_blacklist { println!("# driver-manager blacklist ({} entries)", blacklist.len()); @@ -564,9 +573,15 @@ fn main() { .clone(); crate::config::set_global_shared_blacklist(policy_clone_for_global); - let _sighup_thread = sighup::spawn_reload_worker(std::sync::Arc::clone( - &shared_blacklist, - )); + // N9: SIGHUP worker reloads ALL policy surfaces on operator + // signal (was: blacklist only). Each surface has its own source + // path; failures on one do not block the others. + let _sighup_thread = sighup::spawn_reload_worker(sighup::PolicyReloadTargets { + blacklist: Some(std::sync::Arc::clone(&shared_blacklist)), + driver_options: Some(std::sync::Arc::clone(&shared_options)), + autoload_list: Some(std::sync::Arc::clone(&shared_autoload)), + initfs_manifest: Some(std::sync::Arc::clone(&shared_initfs_manifest)), + }); match manager.lock() { Ok(mut mgr) => { diff --git a/local/recipes/system/driver-manager/source/src/sighup.rs b/local/recipes/system/driver-manager/source/src/sighup.rs index 09eea6e6e2..de91cabb25 100644 --- a/local/recipes/system/driver-manager/source/src/sighup.rs +++ b/local/recipes/system/driver-manager/source/src/sighup.rs @@ -1,13 +1,29 @@ //! SIGHUP signal handler. Wires a safe async-signal-safe trampoline -//! that signals a worker thread, which then calls -//! `SharedBlacklist::replace()` on the live policy. +//! that signals a worker thread, which then calls `replace()` on every +//! live `SharedPolicy` surface (blacklist, options, autoload, +//! initfs-manifest). Each surface has its own `replace()` that +//! re-reads from its source path under the policy directory; a +//! failure on one surface does not block the others. use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use std::thread; use std::time::Duration; -use crate::policy::SharedBlacklist; +use crate::policy::{ + SharedAutoloadList, SharedBlacklist, SharedDriverOptions, SharedInitfsManifest, +}; + +/// Aggregate of the four policy surfaces the SIGHUP worker reloads. +/// `Option<...>` fields allow main.rs to omit surfaces that are not +/// enabled (e.g. an initfs-only manager that has no rootfs options). +#[derive(Default)] +pub struct PolicyReloadTargets { + pub blacklist: Option>, + pub driver_options: Option>, + pub autoload_list: Option>, + pub initfs_manifest: Option>, +} static RELOAD_FLAG: AtomicBool = AtomicBool::new(false); @@ -15,34 +31,93 @@ extern "C" fn signal_handler(_sig: i32) { RELOAD_FLAG.store(true, Ordering::SeqCst); } -pub fn spawn_reload_worker(blacklist: Arc) -> thread::JoinHandle<()> { -// SAFETY: caller must verify the safety contract for this operation +/// Spawn the SIGHUP worker thread. The worker reads the +/// `PolicyReloadTargets` and, on each signal, calls `replace()` on +/// every present surface. Returns the thread's `JoinHandle` for +/// graceful shutdown testing. +pub fn spawn_reload_worker(targets: PolicyReloadTargets) -> thread::JoinHandle<()> { unsafe { libc::signal(libc::SIGHUP, signal_handler as *const () as libc::sighandler_t); } thread::Builder::new() .name("driver-manager-sighup".to_string()) - .spawn(move || run(blacklist)) + .spawn(move || run(targets)) .expect("spawn sighup worker") } -fn run(blacklist: Arc) { - log::info!( - "sighup-worker: started for {}", - blacklist.source_path().display() - ); +fn run(targets: PolicyReloadTargets) { + if let Some(ref b) = targets.blacklist { + log::info!("sighup-worker: started for blacklist at {}", b.source_path().display()); + } + if let Some(ref o) = targets.driver_options { + log::info!("sighup-worker: started for driver options at {}", o.source_path().display()); + } + if let Some(ref a) = targets.autoload_list { + log::info!("sighup-worker: started for autoload list at {}", a.source_path().display()); + } + if let Some(ref m) = targets.initfs_manifest { + log::info!("sighup-worker: started for initfs manifest at {}", m.source_path().display()); + } + log::info!("sighup-worker: policy reload surface wired; awaiting SIGHUP"); loop { std::thread::sleep(Duration::from_millis(100)); - if RELOAD_FLAG.swap(false, Ordering::SeqCst) { - match blacklist.replace() { + if !RELOAD_FLAG.swap(false, Ordering::SeqCst) { + continue; + } + // Reload each surface. Each `replace()` re-reads from its own + // source path; a failure on one does not block the others. + if let Some(ref b) = targets.blacklist { + match b.replace() { Ok(n) => log::info!( "sighup-worker: blacklist reloaded from {} ({} entries)", - blacklist.source_path().display(), + b.source_path().display(), n ), Err(e) => log::error!( "sighup-worker: blacklist reload from {} failed: {}", - blacklist.source_path().display(), + b.source_path().display(), + e + ), + } + } + if let Some(ref o) = targets.driver_options { + match o.replace() { + Ok(n) => log::info!( + "sighup-worker: driver options reloaded from {} ({} drivers)", + o.source_path().display(), + n + ), + Err(e) => log::error!( + "sighup-worker: driver options reload from {} failed: {}", + o.source_path().display(), + e + ), + } + } + if let Some(ref a) = targets.autoload_list { + match a.replace() { + Ok(n) => log::info!( + "sighup-worker: autoload list reloaded from {} ({} modules)", + a.source_path().display(), + n + ), + Err(e) => log::error!( + "sighup-worker: autoload list reload from {} failed: {}", + a.source_path().display(), + e + ), + } + } + if let Some(ref m) = targets.initfs_manifest { + match m.replace() { + Ok(n) => log::info!( + "sighup-worker: initfs manifest reloaded from {} ({} drivers across stages)", + m.source_path().display(), + n + ), + Err(e) => log::error!( + "sighup-worker: initfs manifest reload from {} failed: {}", + m.source_path().display(), e ), } @@ -53,6 +128,7 @@ fn run(blacklist: Arc) { #[cfg(test)] mod tests { use super::*; + use std::sync::Arc; #[test] fn reload_flag_round_trip() { @@ -61,4 +137,35 @@ mod tests { assert!(RELOAD_FLAG.swap(false, Ordering::SeqCst)); assert!(!RELOAD_FLAG.load(Ordering::SeqCst)); } + + #[test] + fn policy_reload_targets_default_is_empty() { + // `PolicyReloadTargets::default()` should have every Option set + // to None so callers can selectively enable only the surfaces + // they actually wired (e.g. a pre-initfs manager that does + // not yet have an initfs manifest). + let t = PolicyReloadTargets::default(); + assert!(t.blacklist.is_none()); + assert!(t.driver_options.is_none()); + assert!(t.autoload_list.is_none()); + assert!(t.initfs_manifest.is_none()); + } + + #[test] + fn policy_reload_targets_supports_partial_wiring() { + // Operators can wire only the surfaces they care about. The + // N9 worker should call replace() only on the present + // surfaces, leaving None for the rest. + let t = PolicyReloadTargets { + blacklist: None, + driver_options: None, + autoload_list: None, + initfs_manifest: None, + }; + assert!(t.blacklist.is_none()); + } + + fn _shared_blacklist_arc_typecheck(arc: Arc) -> Arc { + arc + } }