From 3a01b8c20b22db50d3aa8de511ef1c60f2f2ebb4 Mon Sep 17 00:00:00 2001 From: Zvonko Kaiser Date: Wed, 30 Oct 2024 23:50:55 +0000 Subject: [PATCH 1/3] Initialize Cache with auto_refresh true according to CDI go-impl Signed-off-by: Zvonko Kaiser --- src/cache.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/cache.rs b/src/cache.rs index d07b986..7c3f04b 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -80,7 +80,16 @@ pub struct Cache { } pub fn new_cache(options: Vec>) -> Arc> { - let cache = Arc::new(Mutex::new(Cache::default())); + let cache = Arc::new(Mutex::new( + Cache { + spec_dirs: Vec::new(), + specs: HashMap::new(), + devices: HashMap::new(), + errors: HashMap::new(), + dir_errors: HashMap::new(), + auto_refresh: true, + } + )); { let mut c = cache.lock().unwrap(); From 2090a19049ada4f2badda9c8614a1489bcac64a4 Mon Sep 17 00:00:00 2001 From: Zvonko Kaiser Date: Thu, 31 Oct 2024 00:34:50 +0000 Subject: [PATCH 2/3] Renamed and adjusted with_auto_refresh according to CdiOption, in Rust we cannot use Option is a reserved keyword Signed-off-by: Zvonko Kaiser --- src/cache.rs | 44 ++++++++++++++++++++------------------------ src/default_cache.rs | 12 ++++++------ src/spec_dirs.rs | 7 +------ 3 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 7c3f04b..296a8c6 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -48,22 +48,20 @@ impl fmt::Display for ConflictError { impl Error for ConflictError {} -// CacheOption is an option to change some aspect of default CDI behavior. -pub trait CacheOption { - fn apply(&self, cache: &mut Cache); -} +// CdiOption is an option to change some aspect of default CDI behavior. +// We define the CdiOption type using a type alias, which is a Box. +// This means that CdiOption is a trait object that represents a one-time closure that takes a &mut Cache parameter. +pub type CdiOption = Box; -// WithAutoRefresh returns an option to control automatic Cache refresh. +// with_auto_refresh returns an option to control automatic Cache refresh. // By default auto-refresh is enabled, the list of Spec directories are // monitored and the Cache is automatically refreshed whenever a change // is detected. This option can be used to disable this behavior when a // manually refreshed mode is preferable. -pub struct WithAutoRefresh(pub bool); - -impl CacheOption for WithAutoRefresh { - fn apply(&self, cache: &mut Cache) { - cache.auto_refresh = self.0; - } +pub fn with_auto_refresh(auto_refresh: bool) -> CdiOption { + Box::new(move |c: &mut Cache| { + c.auto_refresh = auto_refresh; + }) } #[allow(dead_code)] @@ -79,17 +77,15 @@ pub struct Cache { //watch: Watch, } -pub fn new_cache(options: Vec>) -> Arc> { - let cache = Arc::new(Mutex::new( - Cache { - spec_dirs: Vec::new(), - specs: HashMap::new(), - devices: HashMap::new(), - errors: HashMap::new(), - dir_errors: HashMap::new(), - auto_refresh: true, - } - )); +pub fn new_cache(options: Vec) -> Arc> { + let cache = Arc::new(Mutex::new(Cache { + spec_dirs: Vec::new(), + specs: HashMap::new(), + devices: HashMap::new(), + errors: HashMap::new(), + dir_errors: HashMap::new(), + auto_refresh: true, + })); { let mut c = cache.lock().unwrap(); @@ -119,9 +115,9 @@ impl Cache { } } - pub fn configure(&mut self, options: Vec>) { + pub fn configure(&mut self, options: Vec) { for option in options { - option.apply(self); + option(self); } } diff --git a/src/default_cache.rs b/src/default_cache.rs index 2371618..a07c409 100644 --- a/src/default_cache.rs +++ b/src/default_cache.rs @@ -6,23 +6,23 @@ use std::sync::{Arc, Mutex}; use oci_spec::runtime::Spec; use once_cell::sync::OnceCell; -use crate::cache::{new_cache, Cache, CacheOption, WithAutoRefresh}; +use crate::cache::{new_cache, with_auto_refresh, Cache, CdiOption}; -fn get_or_create_default_cache(_options: Vec>) -> Arc> { +fn get_or_create_default_cache(_options: &[CdiOption]) -> Arc> { let mut cache: OnceCell>> = OnceCell::new(); cache.get_or_init(|| { - let options: Vec> = vec![Arc::new(WithAutoRefresh(true))]; + let options: Vec = vec![with_auto_refresh(true)]; new_cache(options) }); cache.take().unwrap() } pub fn get_default_cache() -> Arc> { - get_or_create_default_cache(vec![]) + get_or_create_default_cache(vec![].as_ref()) } -pub fn configure(options: Vec>) -> Result<()> { - let cache = get_or_create_default_cache(options.clone()); +pub fn configure(options: Vec) -> Result<()> { + let cache = get_or_create_default_cache(&options); let mut cache = cache.lock().unwrap(); if options.is_empty() { return Ok(()); diff --git a/src/spec_dirs.rs b/src/spec_dirs.rs index 5a956bd..427aa38 100644 --- a/src/spec_dirs.rs +++ b/src/spec_dirs.rs @@ -9,7 +9,7 @@ use lazy_static::lazy_static; use path_clean::clean; use crate::{ - cache::Cache, + cache::{Cache, CdiOption}, spec::{read_spec, Spec}, utils::is_cdi_spec, }; @@ -32,11 +32,6 @@ lazy_static! { ]; } -// CdiOption is an option to change some aspect of default CDI behavior. -// We define the CdiOption type using a type alias, which is a Box. -// This means that CdiOption is a trait object that represents a one-time closure that takes a &mut Cache parameter. -type CdiOption = Box; - #[derive(Debug)] pub struct SpecError { message: String, From d325c311be8e7ca8b9867ba60aa6622a64972705 Mon Sep 17 00:00:00 2001 From: Zvonko Kaiser Date: Thu, 31 Oct 2024 02:41:02 +0000 Subject: [PATCH 3/3] Fix iter if value is None Signed-off-by: Zvonko Kaiser --- src/cache.rs | 18 ++++++------------ src/version.rs | 2 +- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 296a8c6..8458f5f 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -78,14 +78,7 @@ pub struct Cache { } pub fn new_cache(options: Vec) -> Arc> { - let cache = Arc::new(Mutex::new(Cache { - spec_dirs: Vec::new(), - specs: HashMap::new(), - devices: HashMap::new(), - errors: HashMap::new(), - dir_errors: HashMap::new(), - auto_refresh: true, - })); + let cache = Arc::new(Mutex::new(Cache::default())); { let mut c = cache.lock().unwrap(); @@ -273,10 +266,11 @@ impl Cache { if let Some(dev) = self.devices.get(&device) { let mut spec = dev.get_spec(); if specs.insert(spec.clone()) { - match spec.edits() { - Some(ce) => edits.append(ce)?, - None => continue, - }; + // spec.edits may be none when we only have dev.edits + // allow dev.edits to be added even if spec.edits is None + if let Some(ce) = spec.edits() { + edits.append(ce)? + } } edits.append(dev.edits())?; } else { diff --git a/src/version.rs b/src/version.rs index 3dabf39..a451898 100644 --- a/src/version.rs +++ b/src/version.rs @@ -165,7 +165,7 @@ fn requires_v040(spec: &CDISpec) -> bool { spec.devices .iter() .map(|d| &d.container_edits) - .chain(std::iter::once(spec.container_edits.as_ref().unwrap())) + .chain(spec.container_edits.as_ref()) .flat_map(|edits| edits.mounts.iter().flat_map(|mounts| mounts.iter())) .any(|mount| mount.r#type.as_ref().map_or(false, |typ| !typ.is_empty())) }