From fafb5cd3ef128f36894c9c54d34181dfe5ff2a7d Mon Sep 17 00:00:00 2001 From: Michael Ilyin Date: Sun, 29 Oct 2023 00:50:43 +0200 Subject: [PATCH] compare version improved --- Cargo.lock | 1 + plugins/zenoh-backend-traits/src/config.rs | 8 +- plugins/zenoh-backend-traits/src/lib.rs | 10 +- plugins/zenoh-plugin-trait/Cargo.toml | 1 + plugins/zenoh-plugin-trait/src/lib.rs | 15 ++- plugins/zenoh-plugin-trait/src/loading.rs | 30 +++--- plugins/zenoh-plugin-trait/src/vtable.rs | 102 ++++++++++++++++++--- zenoh/src/net/runtime/adminspace.rs | 20 ++-- zenoh/src/net/runtime/mod.rs | 10 +- zenoh/src/plugins/sealed.rs | 8 +- 10 files changed, 154 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ff6e7cfd4..0fe53d1dc3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4936,6 +4936,7 @@ dependencies = [ name = "zenoh-plugin-trait" version = "0.11.0-dev" dependencies = [ + "const_format", "libloading", "log", "serde_json", diff --git a/plugins/zenoh-backend-traits/src/config.rs b/plugins/zenoh-backend-traits/src/config.rs index 2672a2f346..d705adb0ed 100644 --- a/plugins/zenoh-backend-traits/src/config.rs +++ b/plugins/zenoh-backend-traits/src/config.rs @@ -68,12 +68,12 @@ pub struct ReplicaConfig { pub delta: Duration, } -const VOLUME_CONFIG_VERSION: &str = "1"; - impl CompatibilityVersion for VolumeConfig { - fn version() -> &'static str { + fn version() -> u64 { + 1 + } + fn features() -> &'static str { concat_enabled_features!( - VOLUME_CONFIG_VERSION, "auth_pubkey", "auth_usrpwd", "complete_n", diff --git a/plugins/zenoh-backend-traits/src/lib.rs b/plugins/zenoh-backend-traits/src/lib.rs index 325e5a6981..cec49188e3 100644 --- a/plugins/zenoh-backend-traits/src/lib.rs +++ b/plugins/zenoh-backend-traits/src/lib.rs @@ -132,13 +132,13 @@ //! ``` use async_trait::async_trait; -use zenoh_plugin_trait::{CompatibilityVersion, concat_enabled_features}; use std::sync::Arc; use zenoh::prelude::{KeyExpr, OwnedKeyExpr, Sample, Selector}; use zenoh::queryable::ReplyBuilder; use zenoh::time::Timestamp; use zenoh::value::Value; pub use zenoh::Result as ZResult; +use zenoh_plugin_trait::{concat_enabled_features, CompatibilityVersion}; pub mod config; use config::{StorageConfig, VolumeConfig}; @@ -217,12 +217,12 @@ pub trait Volume: Send + Sync { pub type VolumePlugin = Box; -const VOLUME_PLUGIN_VERSION: &str = "1"; - impl CompatibilityVersion for VolumePlugin { - fn version() -> &'static str { + fn version() -> u64 { + 1 + } + fn features() -> &'static str { concat_enabled_features!( - VOLUME_PLUGIN_VERSION, "auth_pubkey", "auth_usrpwd", "complete_n", diff --git a/plugins/zenoh-plugin-trait/Cargo.toml b/plugins/zenoh-plugin-trait/Cargo.toml index 1df89f538a..2a45c7f6cf 100644 --- a/plugins/zenoh-plugin-trait/Cargo.toml +++ b/plugins/zenoh-plugin-trait/Cargo.toml @@ -37,3 +37,4 @@ serde_json = { workspace = true } zenoh-macros = { workspace = true } zenoh-result = { workspace = true } zenoh-util = { workspace = true } +const_format = { workspace = true } \ No newline at end of file diff --git a/plugins/zenoh-plugin-trait/src/lib.rs b/plugins/zenoh-plugin-trait/src/lib.rs index 47b9d153bb..ec1625f49b 100644 --- a/plugins/zenoh-plugin-trait/src/lib.rs +++ b/plugins/zenoh-plugin-trait/src/lib.rs @@ -24,22 +24,29 @@ pub mod vtable; use zenoh_result::ZResult; pub mod prelude { - pub use crate::{loading::*, vtable::*, CompatibilityVersion, Plugin, concat_enabled_features}; + pub use crate::{concat_enabled_features, loading::*, vtable::*, CompatibilityVersion, Plugin}; } #[macro_export] macro_rules! concat_enabled_features { - ($version:ident, $($feature:literal),*) => { + ($($feature:literal),*) => { { use const_format::concatcp; - const_format::concatcp!($version $(, + const_format::concatcp!("" $(, if cfg!(feature = $feature) { concatcp!(" ", $feature) } else { "" } )*) } }; } + pub trait CompatibilityVersion { - fn version() -> &'static str; + /// The version of the structure implementing this trait. After any channge in the structure or it's dependencies + /// whcich may affect the ABI, this version should be incremented. + fn version() -> u64; + /// The features enabled when the structure implementing this trait was compiled. + /// Different features between the plugin and the host may cuase ABI incompatibility even if the structure version is the same. + /// Use `concat_enabled_features!` to generate this string. + fn features() -> &'static str; } pub trait Plugin: Sized + 'static { diff --git a/plugins/zenoh-plugin-trait/src/loading.rs b/plugins/zenoh-plugin-trait/src/loading.rs index 45fe71d8d5..0e01e1acc5 100644 --- a/plugins/zenoh-plugin-trait/src/loading.rs +++ b/plugins/zenoh-plugin-trait/src/loading.rs @@ -224,14 +224,16 @@ impl, T1: AsRef>( &mut self, name: T, backend_name: T1, - ) -> ZResult<&mut PluginRecord> { + ) -> ZResult<(bool, &mut PluginRecord)> { let name = name.as_ref(); - if self.get_plugin_index(name).is_some() { - bail!("Plugin `{}` already loaded", name); + if let Some(index) = self.get_plugin_index(name) { + return Ok((false, &mut self.plugins[index])); } let backend_name = backend_name.as_ref(); let (lib, p) = match &mut self.loader { @@ -243,17 +245,19 @@ impl bail!("After loading `{:?}`: {}", &p, e), }; self.plugins.push(PluginRecord::new(plugin)); - Ok(self.plugins.last_mut().unwrap()) + Ok((true, self.plugins.last_mut().unwrap())) } /// Tries to load a plugin from the list of path to plugin (absolute or relative to the current working directory) + /// Returns a tuple of (retval, plugin_record) + /// where `retval`` is true if the plugin was successfully loaded, false if pluginw with this name it was already loaded pub fn load_plugin_by_paths, P: AsRef + std::fmt::Debug>( &mut self, name: T, paths: &[P], - ) -> ZResult<&mut PluginRecord> { + ) -> ZResult<(bool, &mut PluginRecord)> { let name = name.as_ref(); - if self.get_plugin_index(name).is_some() { - bail!("Plugin `{}` already loaded", name); + if let Some(index) = self.get_plugin_index(name) { + return Ok((false, &mut self.plugins[index])); } for path in paths { let path = path.as_ref(); @@ -261,7 +265,7 @@ impl { let plugin = Self::load_plugin(name, lib, p)?; self.plugins.push(PluginRecord::new(plugin)); - return Ok(self.plugins.last_mut().unwrap()); + return Ok((true, self.plugins.last_mut().unwrap())); } Err(e) => log::warn!("Plugin '{}' load fail at {}: {}", &name, path, e), } @@ -356,9 +360,11 @@ impl DynamicPlugin { fn new(name: String, lib: Library, path: PathBuf) -> ZResult { + log::debug!("Loading plugin {}", &path.to_str().unwrap(),); let get_plugin_loader_version = unsafe { lib.get:: PluginLoaderVersion>(b"get_plugin_loader_version")? }; let plugin_loader_version = get_plugin_loader_version(); + log::debug!("Plugin loader version: {}", &plugin_loader_version); if plugin_loader_version != PLUGIN_LOADER_VERSION { bail!( "Plugin loader version mismatch: host = {}, plugin = {}", @@ -369,15 +375,17 @@ impl let get_compatibility = unsafe { lib.get:: Compatibility>(b"get_compatibility")? }; let plugin_compatibility_record = get_compatibility(); let host_compatibility_record = Compatibility::new::(); + log::debug!( + "Plugin compativilty record: {:?}", + &plugin_compatibility_record + ); if !plugin_compatibility_record.are_compatible(&host_compatibility_record) { bail!( - "Plugin compatibility mismatch:\nhost = {:?}\nplugin = {:?}\n", + "Plugin compatibility mismatch:\n\nHost:\n{}\nPlugin:\n{}\n", host_compatibility_record, plugin_compatibility_record ); } - - // TODO: check loader version and compatibility let load_plugin = unsafe { lib.get:: PluginVTable>(b"load_plugin")? }; let vtable = load_plugin(); diff --git a/plugins/zenoh-plugin-trait/src/vtable.rs b/plugins/zenoh-plugin-trait/src/vtable.rs index 59caf399b7..8c03283b2a 100644 --- a/plugins/zenoh-plugin-trait/src/vtable.rs +++ b/plugins/zenoh-plugin-trait/src/vtable.rs @@ -1,3 +1,5 @@ +use std::fmt::Display; + // // Copyright (c) 2023 ZettaScale Technology // @@ -25,8 +27,56 @@ pub struct PluginVTable { pub start: StartFn, } impl CompatibilityVersion for PluginVTable { - fn version() -> &'static str{ - "1" + fn version() -> u64 { + 1 + } + fn features() -> &'static str { + concat_enabled_features!( + "auth_pubkey", + "auth_usrpwd", + "complete_n", + "shared-memory", + "stats", + "transport_multilink", + "transport_quic", + "transport_serial", + "transport_unixpipe", + "transport_tcp", + "transport_tls", + "transport_udp", + "transport_unixsock-stream", + "transport_ws", + "unstable", + "default" + ) + } +} + +#[repr(C)] +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct StructVersion { + pub version: u64, + pub name: &'static str, + pub features: &'static str, +} + +impl StructVersion { + pub fn new() -> Self { + Self { + version: T::version(), + name: std::any::type_name::(), + features: T::features(), + } + } +} + +impl Display for StructVersion { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + " version: {}\n type: {}\n features: {}\n", + self.version, self.name, self.features + ) } } @@ -34,21 +84,22 @@ impl CompatibilityVersion for PluginVTable() -> Self { + let rust_version = RustVersion::new(); + let vtable_version = StructVersion::new::>(); + let start_args_version = StructVersion::new::(); + let running_plugin_version = StructVersion::new::(); Self { - rust_version: RustVersion::new(), - vtable_version: PluginVTable::::version(), - start_args_version: (std::any::type_name::(), StartArgs::version()), - running_plugin_version: ( - std::any::type_name::(), - RunningPlugin::version(), - ), + rust_version, + vtable_version, + start_args_version, + running_plugin_version, } } pub fn are_compatible(&self, other: &Self) -> bool { @@ -59,6 +110,19 @@ impl Compatibility { } } +impl Display for Compatibility { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}\nVTable:{}StartArgs:{}RunningPlugin:{}", + self.rust_version, + self.vtable_version, + self.start_args_version, + self.running_plugin_version + ) + } +} + #[repr(C)] #[derive(Debug, PartialEq, Eq, Clone)] pub struct RustVersion { @@ -69,6 +133,20 @@ pub struct RustVersion { commit: &'static str, } +impl Display for RustVersion { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Rust {}.{}.{}{} commit {}", + self.major, + self.minor, + self.patch, + if self.stable { "" } else { "-nightly" }, + self.commit + ) + } +} + const RELEASE_AND_COMMIT: (&str, &str) = zenoh_macros::rustc_version_release!(); impl RustVersion { pub fn new() -> Self { diff --git a/zenoh/src/net/runtime/adminspace.rs b/zenoh/src/net/runtime/adminspace.rs index 4fff3c764e..de6cef745f 100644 --- a/zenoh/src/net/runtime/adminspace.rs +++ b/zenoh/src/net/runtime/adminspace.rs @@ -200,12 +200,20 @@ impl AdminSpace { plugins_mgr.load_plugin_by_backend_name(name, &plugin.name) }; match rec { - Ok(rec) => { - log::info!( - "Loaded plugin `{}` from {}", - rec.name(), - rec.path() - ); + Ok((loaded, rec)) => { + if loaded { + log::info!( + "Loaded plugin `{}` from {}", + rec.name(), + rec.path() + ); + } else { + log::warn!( + "Plugin `{}` was already loaded from {}", + rec.name(), + rec.path() + ); + } match rec.start(&admin.context.runtime) { Ok((true, rec)) => { active_plugins diff --git a/zenoh/src/net/runtime/mod.rs b/zenoh/src/net/runtime/mod.rs index f3ca4a78db..c320f538d2 100644 --- a/zenoh/src/net/runtime/mod.rs +++ b/zenoh/src/net/runtime/mod.rs @@ -38,7 +38,7 @@ use stop_token::future::FutureExt; use stop_token::{StopSource, TimedOutError}; use uhlc::{HLCBuilder, HLC}; use zenoh_link::{EndPoint, Link}; -use zenoh_plugin_trait::{CompatibilityVersion, concat_enabled_features}; +use zenoh_plugin_trait::{concat_enabled_features, CompatibilityVersion}; use zenoh_protocol::core::{whatami::WhatAmIMatcher, Locator, WhatAmI, ZenohId}; use zenoh_protocol::network::{NetworkBody, NetworkMessage}; use zenoh_result::{bail, ZResult}; @@ -66,12 +66,12 @@ pub struct Runtime { state: Arc, } -const RUNTIME_VERSION: &str = "1"; - impl CompatibilityVersion for Runtime { - fn version() -> &'static str { + fn version() -> u64 { + 1 + } + fn features() -> &'static str { concat_enabled_features!( - RUNTIME_VERSION, "auth_pubkey", "auth_usrpwd", "complete_n", diff --git a/zenoh/src/plugins/sealed.rs b/zenoh/src/plugins/sealed.rs index 5b2d48374b..b68dd30253 100644 --- a/zenoh/src/plugins/sealed.rs +++ b/zenoh/src/plugins/sealed.rs @@ -31,12 +31,12 @@ pub type StartArgs = Runtime; /// A zenoh plugin, when started, must return this type. pub type RunningPlugin = Box; -const RUNNING_PLUGIN_VERSION: &str = "1"; - impl CompatibilityVersion for RunningPlugin { - fn version() -> &'static str { + fn version() -> u64 { + 1 + } + fn features() -> &'static str { concat_enabled_features!( - RUNNING_PLUGIN_VERSION, "auth_pubkey", "auth_usrpwd", "complete_n",