Thanks for the series. I’ve started reviewing patches 1–6; sending
notes for patch 1 first, and I’ll follow up with comments on the
others once I’ve gone through them in more depth.
comments inline
On 1/6/26 3:25 PM, Kefu Chai wrote:
Initialize the Rust workspace for the pmxcfs rewrite project.
Add pmxcfs-api-types crate which provides foundational types:
- PmxcfsError: Error type with errno mapping for FUSE operations
- FuseMessage: Filesystem operation messages
- KvStoreMessage: Status synchronization messages
- ApplicationMessage: Wrapper enum for both message types
- VmType: VM type enum (Qemu, Lxc)
This is the foundation crate with no internal dependencies, only
requiring thiserror and libc. All other crates will depend on these
shared type definitions.
Signed-off-by: Kefu Chai <[email protected]>
---
src/pmxcfs-rs/Cargo.lock | 2067 +++++++++++++++++++++
Following the .gitignore pattern in our other repos, Cargo.lock is
ignored, so I’d suggest dropping it from the series.
src/pmxcfs-rs/Cargo.toml | 83 +
src/pmxcfs-rs/pmxcfs-api-types/Cargo.toml | 19 +
src/pmxcfs-rs/pmxcfs-api-types/README.md | 105 ++
src/pmxcfs-rs/pmxcfs-api-types/src/lib.rs | 152 ++
5 files changed, 2426 insertions(+)
create mode 100644 src/pmxcfs-rs/Cargo.lock
create mode 100644 src/pmxcfs-rs/Cargo.toml
create mode 100644 src/pmxcfs-rs/pmxcfs-api-types/Cargo.toml
create mode 100644 src/pmxcfs-rs/pmxcfs-api-types/README.md
create mode 100644 src/pmxcfs-rs/pmxcfs-api-types/src/lib.rs
diff --git a/src/pmxcfs-rs/Cargo.lock b/src/pmxcfs-rs/Cargo.lock
[..]
+++ b/src/pmxcfs-rs/Cargo.toml
@@ -0,0 +1,83 @@
+# Workspace root for pmxcfs Rust implementation
+[workspace]
+members = [
+ "pmxcfs-api-types", # Shared types and error definitions
+]
+resolver = "2"
+
+[workspace.package]
+version = "9.0.6"
+edition = "2024"
+authors = ["Proxmox Support Team <[email protected]>"]
+license = "AGPL-3.0"
+repository = "https://git.proxmox.com/?p=pve-cluster.git";
+rust-version = "1.85"
+
+[workspace.dependencies]
Here we already declare workspace path deps for crates that aren’t
present yet (pmxcfs-config, pmxcfs-memdb, ...). For bisectability,
could we keep this patch minimal and add those workspace
members/path deps in the patches where the crates are introduced?
+# Internal workspace dependencies
+pmxcfs-api-types = { path = "pmxcfs-api-types" }
+pmxcfs-config = { path = "pmxcfs-config" }
+pmxcfs-memdb = { path = "pmxcfs-memdb" }
+pmxcfs-dfsm = { path = "pmxcfs-dfsm" }
+pmxcfs-rrd = { path = "pmxcfs-rrd" }
+pmxcfs-status = { path = "pmxcfs-status" }
+pmxcfs-ipc = { path = "pmxcfs-ipc" }
+pmxcfs-services = { path = "pmxcfs-services" }
+pmxcfs-logger = { path = "pmxcfs-logger" }
+
+# Core async runtime
+tokio = { version = "1.35", features = ["full"] }
+tokio-util = "0.7"
+async-trait = "0.1"
+
If the goal is to centrally pin external crate versions early, maybe
limit [workspace.dependencies] here generally to the crates actually
used by pmxcfs-api-types (thiserror, libc) and extend as new crates
are added.
+# Error handling
+anyhow = "1.0"
+thiserror = "1.0"
+
+# Logging and tracing
+tracing = "0.1"
+tracing-subscriber = { version = "0.3", features = ["env-filter"] }
+
+# Serialization
+serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0"
+bincode = "1.3"
+
+# Network and cluster
+bytes = "1.5"
+sha2 = "0.10"
+bytemuck = { version = "1.14", features = ["derive"] }
+
+# System integration
+libc = "0.2"
+nix = { version = "0.27", features = ["fs", "process", "signal", "user",
"socket"] }
+users = "0.11"
+
+# Corosync/CPG bindings
+rust-corosync = "0.1"
+
+# Enum conversions
+num_enum = "0.7"
+
+# Concurrency primitives
+parking_lot = "0.12"
+
+# Utilities
+chrono = "0.4"
+futures = "0.3"
+
+# Development dependencies
+tempfile = "3.8"
+
+[workspace.lints.clippy]
+uninlined_format_args = "warn"
+
+[profile.release]
+lto = true
+codegen-units = 1
+opt-level = 3
+strip = true
+
+[profile.dev]
+opt-level = 1
+debug = true
diff --git a/src/pmxcfs-rs/pmxcfs-api-types/Cargo.toml
b/src/pmxcfs-rs/pmxcfs-api-types/Cargo.toml
new file mode 100644
index 00000000..cdce7951
--- /dev/null
+++ b/src/pmxcfs-rs/pmxcfs-api-types/Cargo.toml
@@ -0,0 +1,19 @@
+[package]
+name = "pmxcfs-api-types"
+description = "Shared types and error definitions for pmxcfs"
+
+version.workspace = true
+edition.workspace = true
+authors.workspace = true
+license.workspace = true
+repository.workspace = true
+
+[lints]
+workspace = true
+
+[dependencies]
+# Error handling
+thiserror.workspace = true
+
+# System integration
+libc.workspace = true
diff --git a/src/pmxcfs-rs/pmxcfs-api-types/README.md
b/src/pmxcfs-rs/pmxcfs-api-types/README.md
new file mode 100644
index 00000000..da8304ae
--- /dev/null
+++ b/src/pmxcfs-rs/pmxcfs-api-types/README.md
@@ -0,0 +1,105 @@
+# pmxcfs-api-types
+
+**Shared Types and Error Definitions** for pmxcfs.
+
+This crate provides common types, error definitions, and message formats used across all
pmxcfs crates. It serves as the "API contract" between different components.
+
+## Overview
+
+The crate contains:
+- **Error types**: `PmxcfsError` with errno mapping for FUSE
+- **Message types**: `FuseMessage`, `KvStoreMessage`, `ApplicationMessage`
These types and the mentioned serialization helpers aren’t part of this
diff, could you re-check both README.md (and the commit message) so they
match?
+- **Shared types**: `MemberInfo`, `NodeSyncInfo`
+- **Serialization**: C-compatible wire format helpers
+
+## Error Types
+
+### PmxcfsError
+
+Type-safe error enum with automatic errno conversion.
+
+### errno Mapping
+
+Errors automatically convert to POSIX errno values for FUSE.
+
+| Error | errno | Value |
+|-------|-------|-------|
+| `NotFound` | `ENOENT` | 2 |
+| `PermissionDenied` | `EPERM` | 1 |
+| `AlreadyExists` | `EEXIST` | 17 |
+| `NotADirectory` | `ENOTDIR` | 20 |
+| `IsADirectory` | `EISDIR` | 21 |
+| `DirectoryNotEmpty` | `ENOTEMPTY` | 39 |
+| `FileTooLarge` | `EFBIG` | 27 |
+| `ReadOnlyFilesystem` | `EROFS` | 30 |
+| `NoQuorum` | `EACCES` | 13 |
+| `Timeout` | `ETIMEDOUT` | 110 |
+
+## Message Types
+
+### FuseMessage
+
+Filesystem operations broadcast through the cluster (via DFSM). Uses
C-compatible wire format compatible with `dcdb.c`.
+
+### KvStoreMessage
+
+Status and metrics synchronization (via kvstore DFSM). Uses C-compatible wire
format.
+
+### ApplicationMessage
+
+Wrapper for either FuseMessage or KvStoreMessage, used by DFSM to handle both
filesystem and status messages with type safety.
+
+## Shared Types
+
+### MemberInfo
+
+Cluster member information.
+
+### NodeSyncInfo
+
+DFSM synchronization state.
+
+## C to Rust Mapping
+
+### Error Handling
+
+**C Version (cfs-utils.h):**
+- Return codes: `0` = success, negative = error
+- errno-based error reporting
+- Manual error checking everywhere
+
+**Rust Version:**
+- `Result<T, PmxcfsError>` type
+
+### Message Types
+
+**C Version (dcdb.h):**
+
+**Rust Version:**
+- Type-safe enums
+
+## Key Differences from C Implementation
+
+All message types have `serialize()` and `deserialize()` methods that produce
byte-for-byte compatible formats with the C implementation.
+
+## Known Issues / TODOs
+
+### Missing Features
+- None identified
+
+### Compatibility
+- **Wire format**: 100% compatible with C implementation
+- **errno values**: Match POSIX standards
+- **Message types**: All C message types covered
+
+## References
+
+### C Implementation
+- `src/pmxcfs/cfs-utils.h` - Utility types and error codes
+- `src/pmxcfs/dcdb.h` - FUSE message types
+- `src/pmxcfs/status.h` - KvStore message types
+
+### Related Crates
+- **pmxcfs-dfsm**: Uses ApplicationMessage for cluster sync
+- **pmxcfs-memdb**: Uses PmxcfsError for database operations
+- **pmxcfs**: Uses FuseMessage for FUSE operations
diff --git a/src/pmxcfs-rs/pmxcfs-api-types/src/lib.rs
b/src/pmxcfs-rs/pmxcfs-api-types/src/lib.rs
new file mode 100644
index 00000000..ae0e5eb0
--- /dev/null
+++ b/src/pmxcfs-rs/pmxcfs-api-types/src/lib.rs
@@ -0,0 +1,152 @@
+use thiserror::Error;
+
+/// Error types for pmxcfs operations
+#[derive(Error, Debug)]
+pub enum PmxcfsError {
nit: the error related parts could be added into a dedicated error.rs
module
+ #[error("I/O error: {0}")]
+ Io(#[from] std::io::Error),
+
+ #[error("Database error: {0}")]
+ Database(String),
+
+ #[error("FUSE error: {0}")]
+ Fuse(String),
+
+ #[error("Cluster error: {0}")]
+ Cluster(String),
+
+ #[error("Corosync error: {0}")]
+ Corosync(String),
+
+ #[error("Configuration error: {0}")]
+ Configuration(String),
+
+ #[error("System error: {0}")]
+ System(String),
+
+ #[error("IPC error: {0}")]
+ Ipc(String),
+
+ #[error("Permission denied")]
+ PermissionDenied,
+
+ #[error("Not found: {0}")]
+ NotFound(String),
+
+ #[error("Already exists: {0}")]
+ AlreadyExists(String),
+
+ #[error("Invalid argument: {0}")]
+ InvalidArgument(String),
+
+ #[error("Not a directory: {0}")]
+ NotADirectory(String),
+
+ #[error("Is a directory: {0}")]
+ IsADirectory(String),
+
+ #[error("Directory not empty: {0}")]
+ DirectoryNotEmpty(String),
+
+ #[error("No quorum")]
+ NoQuorum,
+
+ #[error("Read-only filesystem")]
+ ReadOnlyFilesystem,
+
+ #[error("File too large")]
+ FileTooLarge,
+
+ #[error("Lock error: {0}")]
+ Lock(String),
+
+ #[error("Timeout")]
+ Timeout,
+
+ #[error("Invalid path: {0}")]
+ InvalidPath(String),
+}
+
+impl PmxcfsError {
+ /// Convert error to errno value for FUSE operations
+ pub fn to_errno(&self) -> i32 {
+ match self {
+ PmxcfsError::NotFound(_) => libc::ENOENT,
+ PmxcfsError::PermissionDenied => libc::EPERM,
+ PmxcfsError::AlreadyExists(_) => libc::EEXIST,
+ PmxcfsError::NotADirectory(_) => libc::ENOTDIR,
+ PmxcfsError::IsADirectory(_) => libc::EISDIR,
+ PmxcfsError::DirectoryNotEmpty(_) => libc::ENOTEMPTY,
+ PmxcfsError::InvalidArgument(_) => libc::EINVAL,
+ PmxcfsError::FileTooLarge => libc::EFBIG,
+ PmxcfsError::ReadOnlyFilesystem => libc::EROFS,
+ PmxcfsError::NoQuorum => libc::EACCES,
+ PmxcfsError::Timeout => libc::ETIMEDOUT,
+ PmxcfsError::Io(e) => match e.raw_os_error() {
+ Some(errno) => errno,
+ None => libc::EIO,
+ },
+ _ => libc::EIO,
Please check with C implementation, but:
"PermissionDenied" should likely map to EACCES rather than EPERM. In
FUSE/POSIX, EACCES is the standard return for file permission blocks,
whereas EPERM is usually for administrative restrictions
(like ownership)
"InvalidPath" maps better to EINVAL. EIO suggests a hardware/disk
failure, whereas InvalidPath implies an argument issue
Also, "Lock" should explicitly be mapped.
EBUSY (resource busy / lock contention)
or EDEADLK (deadlock) / EAGAIN depending on semantics
In general, can we minimize the number of errors falling into the
generic EIO branch?
+ }
+ }
+}
+
+/// Result type for pmxcfs operations
+pub type Result<T> = std::result::Result<T, PmxcfsError>;
+
+/// VM/CT types
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
If this is used in wire contexts please add #[repr(u8)] to ensure a
stable ABI.
+pub enum VmType {
+ Qemu = 1,
+ Lxc = 3,
There’s a gap between values 1 -> 3: is 2 reserved?
If so, maybe add a short comment.
+}
+
+impl VmType {
+ /// Returns the directory name where config files are stored
+ pub fn config_dir(&self) -> &'static str {
+ match self {
+ VmType::Qemu => "qemu-server",
+ VmType::Lxc => "lxc",
+ }
+ }
+}
+
+impl std::fmt::Display for VmType {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ match self {
+ VmType::Qemu => write!(f, "qemu"),
+ VmType::Lxc => write!(f, "lxc"),
+ }
+ }
+}
+
+/// VM/CT entry for vmlist
+#[derive(Debug, Clone)]
+pub struct VmEntry {
+ pub vmid: u32,
+ pub vmtype: VmType,
+ pub node: String,
+ /// Per-VM version counter (increments when this VM's config changes)
+ pub version: u32,
+}
+
+/// Information about a cluster member
+///
+/// This is a shared type used by both cluster and DFSM modules
+#[derive(Debug, Clone)]
+pub struct MemberInfo {
+ pub node_id: u32,
+ pub pid: u32,
+ pub joined_at: u64,
+}
+
+/// Node synchronization info for DFSM state sync
+///
+/// Used during DFSM synchronization to track which nodes have provided state
+#[derive(Debug, Clone)]
+pub struct NodeSyncInfo {
+ pub nodeid: u32,
We have "nodeid" here but "node_id" in MemberInfo, this should be
aligned.
+ pub pid: u32,
+ pub state: Option<Vec<u8>>,
+ pub synced: bool,
+}
_______________________________________________
pve-devel mailing list
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
