Don't load plugins/volumes when plugins_loading/enabled:false (#1269)

eclipse-zenoh · Jul 25, 2024 · 0c43c08 · 0c43c08
1 parent bc8029b
commit 0c43c08
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 29 deletions.
diff --git a/commons/zenoh-util/src/lib_loader.rs b/commons/zenoh-util/src/lib_loader.rs
@@ -20,7 +20,7 @@ use std::{
 
 use libloading::Library;
 use tracing::{debug, warn};
-use zenoh_core::zconfigurable;
+use zenoh_core::{zconfigurable, zerror};
 use zenoh_result::{bail, ZResult};
 
 zconfigurable! {
@@ -35,15 +35,13 @@ zconfigurable! {
 /// LibLoader allows search for libraries and to load them.
 #[derive(Clone, Debug)]
 pub struct LibLoader {
-    search_paths: Vec<PathBuf>,
+    search_paths: Option<Vec<PathBuf>>,
 }
 
 impl LibLoader {
     /// Return an empty `LibLoader`.
     pub fn empty() -> LibLoader {
-        LibLoader {
-            search_paths: Vec::new(),
-        }
+        LibLoader { search_paths: None }
     }
 
     /// Returns the list of search paths used by `LibLoader::default()`
@@ -83,12 +81,14 @@ impl LibLoader {
             }
         }
 
-        LibLoader { search_paths }
+        LibLoader {
+            search_paths: Some(search_paths),
+        }
     }
 
     /// Return the list of search paths used by this [LibLoader]
-    pub fn search_paths(&self) -> &[PathBuf] {
-        &self.search_paths
+    pub fn search_paths(&self) -> Option<&[PathBuf]> {
+        self.search_paths.as_deref()
     }
 
     /// Load a library from the specified path.
@@ -118,21 +118,24 @@ impl LibLoader {
     ///
     /// This function calls [libloading::Library::new()](https://docs.rs/libloading/0.7.0/libloading/struct.Library.html#method.new)
     /// which is unsafe.
-    pub unsafe fn search_and_load(&self, name: &str) -> ZResult<(Library, PathBuf)> {
+    pub unsafe fn search_and_load(&self, name: &str) -> ZResult<Option<(Library, PathBuf)>> {
         let filename = format!("{}{}{}", *LIB_PREFIX, name, *LIB_SUFFIX);
         let filename_ostr = OsString::from(&filename);
         tracing::debug!(
             "Search for library {} to load in {:?}",
             filename,
             self.search_paths
         );
-        for dir in &self.search_paths {
+        let Some(search_paths) = self.search_paths() else {
+            return Ok(None);
+        };
+        for dir in search_paths {
             match dir.read_dir() {
                 Ok(read_dir) => {
                     for entry in read_dir.flatten() {
                         if entry.file_name() == filename_ostr {
                             let path = entry.path();
-                            return Ok((Library::new(path.clone())?, path));
+                            return Ok(Some((Library::new(path.clone())?, path)));
                         }
                     }
                 }
@@ -142,7 +145,7 @@ impl LibLoader {
                 ),
             }
         }
-        bail!("Library file '{}' not found", filename)
+        Err(zerror!("Library file '{}' not found", filename).into())
     }
 
     /// Search and load all libraries with filename starting with [struct@LIB_PREFIX]+`prefix` and ending with [struct@LIB_SUFFIX].
@@ -158,17 +161,16 @@ impl LibLoader {
     pub unsafe fn load_all_with_prefix(
         &self,
         prefix: Option<&str>,
-    ) -> Vec<(Library, PathBuf, String)> {
+    ) -> Option<Vec<(Library, PathBuf, String)>> {
         let lib_prefix = format!("{}{}", *LIB_PREFIX, prefix.unwrap_or(""));
         tracing::debug!(
             "Search for libraries {}*{} to load in {:?}",
             lib_prefix,
             *LIB_SUFFIX,
             self.search_paths
         );
-
         let mut result = vec![];
-        for dir in &self.search_paths {
+        for dir in self.search_paths()? {
             match dir.read_dir() {
                 Ok(read_dir) => {
                     for entry in read_dir.flatten() {
@@ -199,7 +201,7 @@ impl LibLoader {
                 ),
             }
         }
-        result
+        Some(result)
     }
 
     pub fn _plugin_name(path: &std::path::Path) -> Option<&str> {

diff --git a/plugins/zenoh-plugin-storage-manager/src/lib.rs b/plugins/zenoh-plugin-storage-manager/src/lib.rs
@@ -229,7 +229,9 @@ impl StorageRuntimeInner {
             self.plugins_manager
                 .declare_dynamic_plugin_by_name(volume_id, backend_name, true)?
         };
-        let loaded = declared.load()?;
+        let loaded = declared
+            .load()?
+            .expect("Volumes should not loaded if if the storage-manager plugin is not loaded");
         loaded.start(config)?;
 
         Ok(())

diff --git a/plugins/zenoh-plugin-trait/src/manager.rs b/plugins/zenoh-plugin-trait/src/manager.rs
@@ -25,7 +25,7 @@ use crate::*;
 
 pub trait DeclaredPlugin<StartArgs, Instance>: PluginStatus {
     fn as_status(&self) -> &dyn PluginStatus;
-    fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin<StartArgs, Instance>>;
+    fn load(&mut self) -> ZResult<Option<&mut dyn LoadedPlugin<StartArgs, Instance>>>;
     fn loaded(&self) -> Option<&dyn LoadedPlugin<StartArgs, Instance>>;
     fn loaded_mut(&mut self) -> Option<&mut dyn LoadedPlugin<StartArgs, Instance>>;
 }
@@ -88,7 +88,7 @@ impl<StartArgs: PluginStartArgs, Instance: PluginInstance> DeclaredPlugin<StartA
     fn as_status(&self) -> &dyn PluginStatus {
         self
     }
-    fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin<StartArgs, Instance>> {
+    fn load(&mut self) -> ZResult<Option<&mut dyn LoadedPlugin<StartArgs, Instance>>> {
         self.0.load()
     }
     fn loaded(&self) -> Option<&dyn LoadedPlugin<StartArgs, Instance>> {

diff --git a/plugins/zenoh-plugin-trait/src/manager/dynamic_plugin.rs b/plugins/zenoh-plugin-trait/src/manager/dynamic_plugin.rs
@@ -13,7 +13,7 @@
 use std::path::{Path, PathBuf};
 
 use libloading::Library;
-use zenoh_result::{bail, ZResult};
+use zenoh_result::{bail, zerror, ZResult};
 use zenoh_util::LibLoader;
 
 use crate::*;
@@ -28,19 +28,19 @@ pub enum DynamicPluginSource {
 }
 
 impl DynamicPluginSource {
-    fn load(&self) -> ZResult<(Library, PathBuf)> {
+    fn load(&self) -> ZResult<Option<(Library, PathBuf)>> {
         match self {
             DynamicPluginSource::ByName((libloader, name)) => unsafe {
                 libloader.search_and_load(name)
             },
             DynamicPluginSource::ByPaths(paths) => {
                 for path in paths {
                     match unsafe { LibLoader::load_file(path) } {
-                        Ok((l, p)) => return Ok((l, p)),
+                        Ok((l, p)) => return Ok(Some((l, p))),
                         Err(e) => tracing::debug!("Attempt to load {} failed: {}", path, e),
                     }
                 }
-                bail!("Plugin not found in {:?}", &paths)
+                Err(zerror!("Plugin not found in {:?}", &paths).into())
             }
         }
     }
@@ -179,16 +179,22 @@ impl<StartArgs: PluginStartArgs, Instance: PluginInstance> DeclaredPlugin<StartA
     fn as_status(&self) -> &dyn PluginStatus {
         self
     }
-    fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin<StartArgs, Instance>> {
+    fn load(&mut self) -> ZResult<Option<&mut dyn LoadedPlugin<StartArgs, Instance>>> {
         if self.starter.is_none() {
-            let (lib, path) = self.source.load().add_error(&mut self.report)?;
+            let Some((lib, path)) = self.source.load().add_error(&mut self.report)? else {
+                tracing::warn!(
+                    "Plugin `{}` will not be loaded as plugin loading is disabled",
+                    self.name
+                );
+                return Ok(None);
+            };
             let starter = DynamicPluginStarter::new(lib, path).add_error(&mut self.report)?;
             tracing::debug!("Plugin {} loaded from {}", self.name, starter.path());
             self.starter = Some(starter);
         } else {
             tracing::warn!("Plugin `{}` already loaded", self.name);
         }
-        Ok(self)
+        Ok(Some(self))
     }
     fn loaded(&self) -> Option<&dyn LoadedPlugin<StartArgs, Instance>> {
         if self.starter.is_some() {

diff --git a/plugins/zenoh-plugin-trait/src/manager/static_plugin.rs b/plugins/zenoh-plugin-trait/src/manager/static_plugin.rs
@@ -84,8 +84,8 @@ where
     fn as_status(&self) -> &dyn PluginStatus {
         self
     }
-    fn load(&mut self) -> ZResult<&mut dyn LoadedPlugin<StartArgs, Instance>> {
-        Ok(self)
+    fn load(&mut self) -> ZResult<Option<&mut dyn LoadedPlugin<StartArgs, Instance>>> {
+        Ok(Some(self))
     }
     fn loaded(&self) -> Option<&dyn LoadedPlugin<StartArgs, Instance>> {
         Some(self)

diff --git a/zenoh/src/net/runtime/adminspace.rs b/zenoh/src/net/runtime/adminspace.rs
@@ -131,7 +131,16 @@ impl AdminSpace {
             );
             loaded
         } else {
-            declared.load()?
+            match declared.load()? {
+                Some(loaded) => loaded,
+                None => {
+                    tracing::warn!(
+                        "Plugin `{}` will not be loaded as plugin loading is disabled",
+                        config.name
+                    );
+                    return Ok(());
+                }
+            }
         };
 
         if let Some(started) = loaded.started_mut() {