From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 6396D1FF09C for ; Mon, 21 Sep 2026 11:57:23 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2E51221548; Mon, 21 Sep 2026 11:57:21 +0200 (CEST) Message-ID: Date: Mon, 21 Sep 2026 11:57:15 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-cluster 03/10] rust: ffi: add C ABI staticlib for pmxcfs To: pve-devel@lists.proxmox.com References: <20260918144152.575163-1-h.laimer@proxmox.com> <20260918144152.575163-4-h.laimer@proxmox.com> Content-Language: en-US, de-AT From: Robert Obkircher In-Reply-To: <20260918144152.575163-4-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1789984635352 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.533 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 4VIJZRXZ4CMNSJ2C4UZ5IM7DN5V5UQQW X-Message-ID-Hash: 4VIJZRXZ4CMNSJ2C4UZ5IM7DN5V5UQQW X-MailFrom: r.obkircher@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 18.09.26 16:42, Hannes Laimer wrote: > pmxcfs is a C daemon, so the notification server needs a small C > facing surface it can link statically. Three functions cover start, > event emission and shutdown, and a log callback taking a syslog > priority routes messages into the daemon's own logging. The emit call > takes the memdb version of the mutation along, which becomes the > sequence number clients see and resume from. > > Rust aborts the process when a panic reaches the C caller. That would > take down /etc/pve because of a bug in an optional feature, so every > entry point catches a panic at that boundary, and one only disables > the notifier for the rest of the daemon's life. It also drops every > connection, so clients notice, reconnect and log the outage rather > than waiting on a socket that will never speak again. > > Signed-off-by: Hannes Laimer > --- > src/rust/Cargo.toml | 1 + > src/rust/pmxcfs-ffi/Cargo.toml | 17 ++ > src/rust/pmxcfs-ffi/include/pmxcfs-notify.h | 29 ++ > src/rust/pmxcfs-ffi/src/lib.rs | 309 ++++++++++++++++++++ > 4 files changed, 356 insertions(+) > create mode 100644 src/rust/pmxcfs-ffi/Cargo.toml > create mode 100644 src/rust/pmxcfs-ffi/include/pmxcfs-notify.h > create mode 100644 src/rust/pmxcfs-ffi/src/lib.rs > > diff --git a/src/rust/Cargo.toml b/src/rust/Cargo.toml > index 6de0ab8..9324685 100644 > --- a/src/rust/Cargo.toml > +++ b/src/rust/Cargo.toml > @@ -1,5 +1,6 @@ > [workspace] > members = [ > + "pmxcfs-ffi", > "pmxcfs-notify", > ] > resolver = "3" > diff --git a/src/rust/pmxcfs-ffi/Cargo.toml b/src/rust/pmxcfs-ffi/Cargo.toml > new file mode 100644 > index 0000000..8264cde > --- /dev/null > +++ b/src/rust/pmxcfs-ffi/Cargo.toml > @@ -0,0 +1,17 @@ > +[package] > +name = "pmxcfs-ffi" > +version = "0.1.0" > +description = "C ABI bridge exposing pmxcfs-notify to the pmxcfs daemon" > +authors.workspace = true > +edition.workspace = true > +license.workspace = true > +homepage.workspace = true > +rust-version.workspace = true > + > +[lib] > +crate-type = ["staticlib"] > + > +[dependencies] > +libc.workspace = true > +log.workspace = true > +pmxcfs-notify.workspace = true > diff --git a/src/rust/pmxcfs-ffi/include/pmxcfs-notify.h b/src/rust/pmxcfs-ffi/include/pmxcfs-notify.h > new file mode 100644 > index 0000000..811213d > --- /dev/null > +++ b/src/rust/pmxcfs-ffi/include/pmxcfs-notify.h > @@ -0,0 +1,29 @@ > +#ifndef PMXCFS_NOTIFY_H > +#define PMXCFS_NOTIFY_H > + > +#include > +#include > + > +enum pmxcfs_notify_type { > + PMXCFS_NOTIFY_CREATE = 0, > + PMXCFS_NOTIFY_WRITE = 1, > + PMXCFS_NOTIFY_MTIME = 2, > + PMXCFS_NOTIFY_RENAME = 3, > + PMXCFS_NOTIFY_DELETE = 4, > + PMXCFS_NOTIFY_MKDIR = 5, > + PMXCFS_NOTIFY_RESYNC = 6, > +}; > + > +/* priority is a syslog(3) level */ > +typedef void (*pmxcfs_notify_log_fn)(int priority, const char *msg); > + > +/* seq is the version of the last mutation so far, a client resuming at it is current */ > +int pmxcfs_notify_init( > + const char *socket_path, gid_t gid, pmxcfs_notify_log_fn log_cb, uint64_t seq > +); > +void pmxcfs_notify_emit( > + enum pmxcfs_notify_type type, uint64_t seq, const char *path, const char *to > +); > +void pmxcfs_notify_shutdown(void); > + > +#endif /* PMXCFS_NOTIFY_H */ > diff --git a/src/rust/pmxcfs-ffi/src/lib.rs b/src/rust/pmxcfs-ffi/src/lib.rs > new file mode 100644 > index 0000000..f8a9629 > --- /dev/null > +++ b/src/rust/pmxcfs-ffi/src/lib.rs > @@ -0,0 +1,309 @@ > +//! C ABI for the pmxcfs daemon, declared in include/pmxcfs-notify.h. > +//! > +//! Rust aborts the process when a panic reaches an `extern "C"` frame, > +//! which must never happen to pmxcfs. Every entry point therefore runs > +//! under catch_unwind and a panic switches the notifier off for the rest > +//! of the daemon's life instead of taking it down. > + > +use std::ffi::{CStr, CString, OsStr, c_char, c_int}; > +use std::os::unix::ffi::OsStrExt; > +use std::panic::{AssertUnwindSafe, catch_unwind}; > +use std::path::PathBuf; > +use std::sync::atomic::{AtomicBool, Ordering}; > +use std::sync::{Mutex, MutexGuard, OnceLock}; > + > +use log::{Level, LevelFilter, Log, Metadata, Record}; > +use pmxcfs_notify::{Config, EventKind, Server}; > + > +pub type LogFn = unsafe extern "C" fn(c_int, *const c_char); > + > +const TYPE_CREATE: c_int = 0; > +const TYPE_WRITE: c_int = 1; > +const TYPE_MTIME: c_int = 2; > +const TYPE_RENAME: c_int = 3; > +const TYPE_DELETE: c_int = 4; > +const TYPE_MKDIR: c_int = 5; > +const TYPE_RESYNC: c_int = 6; > +const RING_SIZE: usize = 8192; > + > +static SERVER: Mutex> = Mutex::new(None); > +static DISABLED: AtomicBool = AtomicBool::new(false); > +static LOG_CB: Mutex> = Mutex::new(None); > +static LOGGER: CLogger = CLogger; > +static LOGGER_INSTALLED: OnceLock<()> = OnceLock::new(); > + > +struct CLogger; > + > +impl Log for CLogger { > + fn enabled(&self, _: &Metadata) -> bool { > + true > + } > + > + fn log(&self, record: &Record) { > + let Some(callback) = *lock(&LOG_CB) else { > + return; > + }; > + let priority = match record.level() { > + Level::Error => libc::LOG_ERR, > + Level::Warn => libc::LOG_WARNING, > + Level::Info => libc::LOG_INFO, > + Level::Debug | Level::Trace => libc::LOG_DEBUG, > + }; > + let msg = record.args().to_string().replace('\0', " "); > + let Ok(msg) = CString::new(msg) else { > + return; > + }; > + unsafe { callback(priority, msg.as_ptr()) }; > + } > + > + fn flush(&self) {} > +} > + > +fn lock(mutex: &Mutex) -> MutexGuard<'_, T> { > + mutex > + .lock() > + .unwrap_or_else(|poisoned| poisoned.into_inner()) > +} > + > +fn guard(what: &str, fallback: R, f: impl FnOnce() -> R) -> R { > + if DISABLED.load(Ordering::SeqCst) { > + return fallback; > + } > + match catch_unwind(AssertUnwindSafe(f)) { > + Ok(result) => result, > + Err(payload) => { > + DISABLED.store(true, Ordering::SeqCst); > + let reason = payload > + .downcast_ref::<&str>() > + .map(|s| s.to_string()) > + .or_else(|| payload.downcast_ref::().cloned()) to_string and cloned are theoretically allowed to panic on allocation failure. I think you could just map the &String to &str instead. (not a full review, I just quickly skimmed this patch) > [..]