From a2468e6de24f6e618e1e6ec3979502a418f063ca Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 9 Feb 2026 21:27:56 -0800 Subject: [PATCH 1/7] Add parallel sibling branches with first-wins conflict detection Signed-off-by: Cong Wang --- src/branch.rs | 46 +++++++++++++-- src/error.rs | 3 + src/fs.rs | 97 +++++++++++++++++++++++-------- src/fs_ctl.rs | 37 +++++++++--- tests/test_ioctl.rs | 137 ++++++++++++++++++++++++++------------------ 5 files changed, 227 insertions(+), 93 deletions(-) diff --git a/src/branch.rs b/src/branch.rs index 9454e01..3bda86d 100644 --- a/src/branch.rs +++ b/src/branch.rs @@ -18,10 +18,19 @@ pub struct Branch { pub files_dir: PathBuf, pub tombstones_file: PathBuf, tombstones: RwLock>, + /// How many children have been committed (merged) into this branch. + pub commit_count: AtomicU64, + /// Parent's commit_count at the time this branch was forked. + pub parent_version_at_fork: u64, } impl Branch { - pub fn new(name: &str, parent: Option<&str>, storage_path: &Path) -> Result { + pub fn new( + name: &str, + parent: Option<&str>, + storage_path: &Path, + parent_version_at_fork: u64, + ) -> Result { let branch_dir = storage_path.join("branches").join(name); let files_dir = branch_dir.join("files"); let tombstones_file = branch_dir.join("tombstones"); @@ -39,6 +48,8 @@ impl Branch { files_dir, tombstones_file, tombstones: RwLock::new(tombstones), + commit_count: AtomicU64::new(0), + parent_version_at_fork, }) } @@ -145,7 +156,7 @@ impl BranchManager { // Always start fresh with just the "main" branch let mut branches = std::collections::HashMap::new(); - let main_branch = Branch::new("main", None, &storage_path)?; + let main_branch = Branch::new("main", None, &storage_path, 0)?; branches.insert("main".to_string(), main_branch); Ok(Self { @@ -169,11 +180,12 @@ impl BranchManager { return Err(BranchError::AlreadyExists(name.to_string())); } - if !branches.contains_key(parent) { - return Err(BranchError::ParentNotFound(parent.to_string())); - } + let parent_branch = branches + .get(parent) + .ok_or_else(|| BranchError::ParentNotFound(parent.to_string()))?; + let parent_version = parent_branch.commit_count.load(Ordering::SeqCst); - let branch = Branch::new(name, Some(parent), &self.storage_path)?; + let branch = Branch::new(name, Some(parent), &self.storage_path, parent_version)?; branches.insert(name.to_string(), branch); let elapsed = start.elapsed(); @@ -405,6 +417,20 @@ impl BranchManager { .clone() .ok_or_else(|| BranchError::NotFound(branch_name.to_string()))?; + let child_version_at_fork = branch.parent_version_at_fork; + + // First-wins conflict detection: check that the parent hasn't had + // another sibling committed since this branch was forked. + { + let parent = branches + .get(&parent_name) + .ok_or_else(|| BranchError::NotFound(parent_name.to_string()))?; + let current_parent_version = parent.commit_count.load(Ordering::SeqCst); + if current_parent_version != child_version_at_fork { + return Err(BranchError::Conflict(branch_name.to_string())); + } + } + let child_tombstones = branch.get_tombstones(); let child_files_dir = branch.files_dir.clone(); @@ -437,6 +463,11 @@ impl BranchManager { num_files += 1; })?; + // Increment parent's commit_count (first-wins bookkeeping) + if let Some(main_branch) = branches.get("main") { + main_branch.commit_count.fetch_add(1, Ordering::SeqCst); + } + // Remove branch branches.remove(branch_name); let branch_dir = self.storage_path.join("branches").join(branch_name); @@ -501,6 +532,9 @@ impl BranchManager { // Write updated tombstones to parent parent.set_tombstones(parent_tombstones)?; + // Increment parent's commit_count (first-wins bookkeeping) + parent.commit_count.fetch_add(1, Ordering::SeqCst); + // Remove child branch branches.remove(branch_name); let branch_dir = self.storage_path.join("branches").join(branch_name); diff --git a/src/error.rs b/src/error.rs index fea9af2..14405ea 100644 --- a/src/error.rs +++ b/src/error.rs @@ -23,6 +23,9 @@ pub enum BranchError { #[error("cannot commit/abort non-leaf branch '{0}'")] NotALeaf(String), + #[error("commit conflict: branch '{0}' is stale (another sibling already committed)")] + Conflict(String), + #[error("io error: {0}")] Io(#[from] std::io::Error), diff --git a/src/fs.rs b/src/fs.rs index 7b90ca8..02f3730 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -14,6 +14,7 @@ use fuser::{ use parking_lot::RwLock; use crate::branch::BranchManager; +use crate::error::BranchError; use crate::fs_path::{classify_path, PathContext}; use crate::inode::{InodeManager, ROOT_INO}; use crate::storage; @@ -24,9 +25,24 @@ use crate::storage; pub(crate) const TTL: Duration = Duration::from_secs(0); pub(crate) const BLOCK_SIZE: u32 = 512; -pub const FS_IOC_BRANCH_CREATE: u32 = 0x6200; // _IO('b', 0) -pub const FS_IOC_BRANCH_COMMIT: u32 = 0x6201; // _IO('b', 1) -pub const FS_IOC_BRANCH_ABORT: u32 = 0x6202; // _IO('b', 2) +pub const FS_IOC_BRANCH_CREATE: u32 = 0xC080_6200; // _IOWR('b', 0, [u8; 128]) +pub const FS_IOC_BRANCH_COMMIT: u32 = 0x4080_6201; // _IOW ('b', 1, [u8; 128]) +pub const FS_IOC_BRANCH_ABORT: u32 = 0x4080_6202; // _IOW ('b', 2, [u8; 128]) + +/// Parse a null-terminated branch name from an ioctl buffer. +fn parse_branch_name(in_data: &[u8]) -> Option { + if in_data.is_empty() { + return None; + } + let end = in_data.iter().position(|&b| b == 0).unwrap_or(in_data.len()); + let s = std::str::from_utf8(&in_data[..end]).ok()?; + let trimmed = s.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } +} pub(crate) const CTL_FILE: &str = ".branchfs_ctl"; pub(crate) const CTL_INO: u64 = u64::MAX - 1; @@ -1335,23 +1351,34 @@ impl Filesystem for BranchFs { _fh: u64, _flags: u32, cmd: u32, - _in_data: &[u8], + in_data: &[u8], _out_size: u32, reply: ReplyIoctl, ) { - let branch_name = self.get_branch_name(); match cmd { FS_IOC_BRANCH_CREATE => { + let parent_id = match parse_branch_name(in_data) { + Some(p) => p, + None => { + log::error!("ioctl CREATE: empty parent_id"); + reply.error(libc::EINVAL); + return; + } + }; let name = format!("branch-{}", uuid::Uuid::new_v4()); - log::info!("ioctl: CREATE branch '{}' from '{}'", name, branch_name); - match self.manager.create_branch(&name, &branch_name) { + log::info!("ioctl: CREATE branch '{}' from '{}'", name, parent_id); + match self.manager.create_branch(&name, &parent_id) { Ok(()) => { - self.switch_to_branch(&name); - log::info!("Switched to new branch '{}'", name); - // _IO encoding has no data direction, so we cannot - // return data through restricted FUSE ioctl. The - // mount is already switched to the new branch. - reply.ioctl(0, &[]) + // Update epoch so stale checks see the new branch + self.current_epoch + .store(self.manager.get_epoch(), Ordering::SeqCst); + log::info!("Created branch '{}' (no mount switch)", name); + // Return the branch name in a 128-byte null-padded buffer + let mut buf = [0u8; 128]; + let name_bytes = name.as_bytes(); + let len = name_bytes.len().min(127); + buf[..len].copy_from_slice(&name_bytes[..len]); + reply.ioctl(0, &buf) } Err(e) => { log::error!("create branch failed: {}", e); @@ -1360,13 +1387,27 @@ impl Filesystem for BranchFs { } } FS_IOC_BRANCH_COMMIT => { - log::info!("ioctl: COMMIT for branch '{}'", branch_name); - match self.manager.commit(&branch_name) { - Ok(parent) => { - self.switch_to_branch(&parent); - log::info!("Switched to branch '{}' after commit", parent); + let branch_id = match parse_branch_name(in_data) { + Some(b) => b, + None => { + log::error!("ioctl COMMIT: empty branch_id"); + reply.error(libc::EINVAL); + return; + } + }; + log::info!("ioctl: COMMIT branch '{}'", branch_id); + match self.manager.commit(&branch_id) { + Ok(_parent) => { + self.inodes.clear_prefix(&format!("/@{}", branch_id)); + self.current_epoch + .store(self.manager.get_epoch(), Ordering::SeqCst); + log::info!("Committed branch '{}' (no mount switch)", branch_id); reply.ioctl(0, &[]) } + Err(BranchError::Conflict(_)) => { + log::warn!("commit conflict for branch '{}'", branch_id); + reply.error(libc::ESTALE); + } Err(e) => { log::error!("commit failed: {}", e); reply.error(libc::EIO); @@ -1374,11 +1415,21 @@ impl Filesystem for BranchFs { } } FS_IOC_BRANCH_ABORT => { - log::info!("ioctl: ABORT for branch '{}'", branch_name); - match self.manager.abort(&branch_name) { - Ok(parent) => { - self.switch_to_branch(&parent); - log::info!("Switched to branch '{}' after abort", parent); + let branch_id = match parse_branch_name(in_data) { + Some(b) => b, + None => { + log::error!("ioctl ABORT: empty branch_id"); + reply.error(libc::EINVAL); + return; + } + }; + log::info!("ioctl: ABORT branch '{}'", branch_id); + match self.manager.abort(&branch_id) { + Ok(_parent) => { + self.inodes.clear_prefix(&format!("/@{}", branch_id)); + self.current_epoch + .store(self.manager.get_epoch(), Ordering::SeqCst); + log::info!("Aborted branch '{}' (no mount switch)", branch_id); reply.ioctl(0, &[]) } Err(e) => { diff --git a/src/fs_ctl.rs b/src/fs_ctl.rs index 1c0024e..42d99ed 100644 --- a/src/fs_ctl.rs +++ b/src/fs_ctl.rs @@ -2,6 +2,7 @@ use std::sync::atomic::Ordering; use fuser::ReplyWrite; +use crate::error::BranchError; use crate::fs::BranchFs; impl BranchFs { @@ -71,10 +72,17 @@ impl BranchFs { match result { Ok(parent) => { + // Root ctl always operates on the current mount branch, which + // gets deleted on commit/abort. We must switch to the parent + // so the mount stays valid. self.switch_to_branch(&parent); log::info!("Switched to branch '{}' after {}", parent, cmd_lower); reply.written(data.len() as u32) } + Err(BranchError::Conflict(_)) => { + log::warn!("Root ctl {} conflict for '{}'", cmd_lower, branch_name); + reply.error(libc::ESTALE); + } Err(e) => { log::error!("Control command failed: {}", e); reply.error(libc::EIO); @@ -104,15 +112,30 @@ impl BranchFs { self.inodes.clear_prefix(&format!("/@{}", branch)); self.current_epoch .store(self.manager.get_epoch(), Ordering::SeqCst); - *self.branch_name.write() = parent.clone(); - log::info!( - "Branch ctl {} succeeded for '{}', switched to '{}'", - cmd_lower, - branch, - parent - ); + // Only switch the mount if the operated branch is the current mount branch + let current = self.get_branch_name(); + if current == branch { + *self.branch_name.write() = parent.clone(); + log::info!( + "Branch ctl {} succeeded for '{}', switched to '{}'", + cmd_lower, + branch, + parent + ); + } else { + log::info!( + "Branch ctl {} succeeded for '{}' (mount stays on '{}')", + cmd_lower, + branch, + current + ); + } reply.written(data.len() as u32) } + Err(BranchError::Conflict(_)) => { + log::warn!("Branch ctl {} conflict for '{}'", cmd_lower, branch); + reply.error(libc::ESTALE); + } Err(e) => { log::error!("Branch ctl command failed: {}", e); reply.error(libc::EIO); diff --git a/tests/test_ioctl.rs b/tests/test_ioctl.rs index 9854dd7..22399bb 100644 --- a/tests/test_ioctl.rs +++ b/tests/test_ioctl.rs @@ -12,10 +12,38 @@ use std::time::Duration; use branchfs::{FS_IOC_BRANCH_ABORT, FS_IOC_BRANCH_COMMIT, FS_IOC_BRANCH_CREATE}; -/// Call an ioctl with no data argument. Returns the raw ioctl return value -/// (0 on success, -1 on failure with errno set). -unsafe fn branch_ioctl(fd: i32, cmd: u32) -> i32 { - libc::ioctl(fd, cmd as libc::c_ulong, 0 as libc::c_ulong) +/// Call a data-carrying ioctl with a 128-byte buffer. +/// `input` is the null-terminated string to send. +/// Returns (ret, output_buf) where ret is the raw ioctl return value. +unsafe fn branch_ioctl_buf(fd: i32, cmd: u32, input: &str) -> (i32, [u8; 128]) { + let mut buf = [0u8; 128]; + let bytes = input.as_bytes(); + let len = bytes.len().min(127); + buf[..len].copy_from_slice(&bytes[..len]); + let ret = libc::ioctl(fd, cmd as libc::c_ulong, buf.as_mut_ptr()); + (ret, buf) +} + +/// Helper: CREATE a branch from `parent`, returning the new branch name. +unsafe fn ioctl_create(fd: i32, parent: &str) -> Result { + let (ret, buf) = branch_ioctl_buf(fd, FS_IOC_BRANCH_CREATE, parent); + if ret < 0 { + return Err(*libc::__errno_location()); + } + let end = buf.iter().position(|&b| b == 0).unwrap_or(buf.len()); + Ok(String::from_utf8_lossy(&buf[..end]).to_string()) +} + +/// Helper: COMMIT a branch by name. +unsafe fn ioctl_commit(fd: i32, branch: &str) -> i32 { + let (ret, _) = branch_ioctl_buf(fd, FS_IOC_BRANCH_COMMIT, branch); + ret +} + +/// Helper: ABORT a branch by name. +unsafe fn ioctl_abort(fd: i32, branch: &str) -> i32 { + let (ret, _) = branch_ioctl_buf(fd, FS_IOC_BRANCH_ABORT, branch); + ret } struct TestFixture { @@ -126,13 +154,14 @@ fn test_ioctl_create_and_commit_new_file() { let ctl = fix.open_ctl(); let fd = ctl.as_raw_fd(); - // CREATE a branch - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_CREATE) }; - assert_eq!(ret, 0, "CREATE should succeed"); + // CREATE a branch from main + let branch = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); + assert!(!branch.is_empty(), "branch name should not be empty"); - // Write a new file on the branch - fs::write(fix.mnt.join("new_file.txt"), "branch content\n").unwrap(); - assert!(fix.mnt.join("new_file.txt").exists()); + // Write a new file on the branch via @branch virtual path + let branch_dir = fix.mnt.join(format!("@{}", branch)); + fs::write(branch_dir.join("new_file.txt"), "branch content\n").unwrap(); + assert!(branch_dir.join("new_file.txt").exists()); // Base should NOT have the file yet assert!( @@ -141,7 +170,7 @@ fn test_ioctl_create_and_commit_new_file() { ); // COMMIT the branch - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_COMMIT) }; + let ret = unsafe { ioctl_commit(fd, &branch) }; assert_eq!(ret, 0, "COMMIT should succeed"); // File should now be in base @@ -165,11 +194,11 @@ fn test_ioctl_modify_existing_and_commit() { let ctl = fix.open_ctl(); let fd = ctl.as_raw_fd(); - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_CREATE) }; - assert_eq!(ret, 0); + let branch = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); - // Overwrite an existing base file - fs::write(fix.mnt.join("file1.txt"), "modified\n").unwrap(); + // Overwrite an existing base file via @branch path + let branch_dir = fix.mnt.join(format!("@{}", branch)); + fs::write(branch_dir.join("file1.txt"), "modified\n").unwrap(); // Base still has the original assert_eq!( @@ -177,7 +206,7 @@ fn test_ioctl_modify_existing_and_commit() { "base content\n" ); - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_COMMIT) }; + let ret = unsafe { ioctl_commit(fd, &branch) }; assert_eq!(ret, 0); // Base should reflect the modification @@ -197,27 +226,21 @@ fn test_ioctl_create_and_abort() { let ctl = fix.open_ctl(); let fd = ctl.as_raw_fd(); - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_CREATE) }; - assert_eq!(ret, 0, "CREATE should succeed"); + let branch = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); - // Write a file and modify another - fs::write(fix.mnt.join("abort_file.txt"), "will be discarded\n").unwrap(); - fs::write(fix.mnt.join("file1.txt"), "modified\n").unwrap(); + // Write a file and modify another via @branch path + let branch_dir = fix.mnt.join(format!("@{}", branch)); + fs::write(branch_dir.join("abort_file.txt"), "will be discarded\n").unwrap(); + fs::write(branch_dir.join("file1.txt"), "modified\n").unwrap(); - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_ABORT) }; + let ret = unsafe { ioctl_abort(fd, &branch) }; assert_eq!(ret, 0, "ABORT should succeed"); - // New file should be gone + // Branch dir should be gone after abort assert!( - !fix.mnt.join("abort_file.txt").exists(), + !branch_dir.join("abort_file.txt").exists(), "new file should vanish after abort" ); - // Modification should be reverted - assert_eq!( - fs::read_to_string(fix.mnt.join("file1.txt")).unwrap(), - "base content\n", - "existing file should revert after abort" - ); // Base untouched assert!(!fix.base.join("abort_file.txt").exists()); assert_eq!( @@ -237,22 +260,23 @@ fn test_ioctl_nested_create_and_commit() { let fd = ctl.as_raw_fd(); // First branch (child of main) - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_CREATE) }; - assert_eq!(ret, 0, "first CREATE should succeed"); - fs::write(fix.mnt.join("level1.txt"), "level1\n").unwrap(); + let branch1 = unsafe { ioctl_create(fd, "main") }.expect("first CREATE should succeed"); + let b1_dir = fix.mnt.join(format!("@{}", branch1)); + fs::write(b1_dir.join("level1.txt"), "level1\n").unwrap(); // Second branch (child of first) - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_CREATE) }; - assert_eq!(ret, 0, "second CREATE should succeed"); - fs::write(fix.mnt.join("level2.txt"), "level2\n").unwrap(); + let branch2 = unsafe { ioctl_create(fd, &branch1) }.expect("second CREATE should succeed"); + let b2_dir = fix.mnt.join(format!("@{}", branch2)); + fs::write(b2_dir.join("level2.txt"), "level2\n").unwrap(); - // All files visible through the mount - assert!(fix.mnt.join("file1.txt").exists()); - assert!(fix.mnt.join("level1.txt").exists()); - assert!(fix.mnt.join("level2.txt").exists()); + // Files visible through their respective branch paths + assert!(b1_dir.join("level1.txt").exists()); + assert!(b2_dir.join("level2.txt").exists()); + // level2 branch should also see level1 (inherited from parent) + assert!(b2_dir.join("level1.txt").exists()); // COMMIT level-2 → merges into level-1 - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_COMMIT) }; + let ret = unsafe { ioctl_commit(fd, &branch2) }; assert_eq!(ret, 0, "commit level-2 should succeed"); // Neither file should be in base yet @@ -260,7 +284,7 @@ fn test_ioctl_nested_create_and_commit() { assert!(!fix.base.join("level2.txt").exists()); // COMMIT level-1 → merges into main / base - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_COMMIT) }; + let ret = unsafe { ioctl_commit(fd, &branch1) }; assert_eq!(ret, 0, "commit level-1 should succeed"); // Both files in base now @@ -278,7 +302,7 @@ fn test_ioctl_commit_on_main_fails() { let ctl = fix.open_ctl(); let fd = ctl.as_raw_fd(); - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_COMMIT) }; + let ret = unsafe { ioctl_commit(fd, "main") }; assert!(ret < 0, "COMMIT on main should fail (got {})", ret); } @@ -290,11 +314,11 @@ fn test_ioctl_abort_on_main_fails() { let ctl = fix.open_ctl(); let fd = ctl.as_raw_fd(); - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_ABORT) }; + let ret = unsafe { ioctl_abort(fd, "main") }; assert!(ret < 0, "ABORT on main should fail (got {})", ret); } -// ── Multiple CREATE + ABORT returns to previous branch ────────────── +// ── Multiple CREATE + ABORT discards child branch ─────────────────── #[test] #[ignore] @@ -305,25 +329,24 @@ fn test_ioctl_abort_returns_to_parent_branch() { let fd = ctl.as_raw_fd(); // CREATE first branch, write a file - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_CREATE) }; - assert_eq!(ret, 0); - fs::write(fix.mnt.join("parent_file.txt"), "parent\n").unwrap(); + let branch1 = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); + let b1_dir = fix.mnt.join(format!("@{}", branch1)); + fs::write(b1_dir.join("parent_file.txt"), "parent\n").unwrap(); // CREATE second (nested) branch, write another file - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_CREATE) }; - assert_eq!(ret, 0); - fs::write(fix.mnt.join("child_file.txt"), "child\n").unwrap(); + let branch2 = unsafe { ioctl_create(fd, &branch1) }.expect("CREATE should succeed"); + let b2_dir = fix.mnt.join(format!("@{}", branch2)); + fs::write(b2_dir.join("child_file.txt"), "child\n").unwrap(); - // ABORT second branch — should land back on first branch - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_ABORT) }; + // ABORT second branch — child's data is discarded + let ret = unsafe { ioctl_abort(fd, &branch2) }; assert_eq!(ret, 0); - // Child's file gone, parent's file still visible - assert!(!fix.mnt.join("child_file.txt").exists()); - assert!(fix.mnt.join("parent_file.txt").exists()); + // Parent branch still has its file + assert!(b1_dir.join("parent_file.txt").exists()); // COMMIT first branch back to main - let ret = unsafe { branch_ioctl(fd, FS_IOC_BRANCH_COMMIT) }; + let ret = unsafe { ioctl_commit(fd, &branch1) }; assert_eq!(ret, 0); assert!(fix.base.join("parent_file.txt").exists()); From 45ec2f359b7aae6b3d6d08a56172eea98f655a02 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 9 Feb 2026 21:54:22 -0800 Subject: [PATCH 2/7] Use explicit branch names in root ctl and CLI commit/abort Signed-off-by: Cong Wang --- src/fs.rs | 14 ++++++ src/fs_ctl.rs | 114 ++++++++++++++++++++++++++++++---------------- src/fs_helpers.rs | 7 ++- src/main.rs | 87 ++++++++++++++--------------------- 4 files changed, 129 insertions(+), 93 deletions(-) diff --git a/src/fs.rs b/src/fs.rs index 02f3730..debab45 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -576,6 +576,20 @@ impl Filesystem for BranchFs { _lock: Option, reply: ReplyData, ) { + // Reading root ctl file returns the current branch name + if ino == CTL_INO { + let branch = self.get_branch_name(); + let bytes = branch.as_bytes(); + let off = offset as usize; + if off >= bytes.len() { + reply.data(&[]); + } else { + let end = (off + size as usize).min(bytes.len()); + reply.data(&bytes[off..end]); + } + return; + } + let epoch = self.current_epoch.load(Ordering::SeqCst); // Fast path: reuse cached fd for the same inode+epoch (avoids diff --git a/src/fs_ctl.rs b/src/fs_ctl.rs index 42d99ed..54674d4 100644 --- a/src/fs_ctl.rs +++ b/src/fs_ctl.rs @@ -34,58 +34,94 @@ impl BranchFs { None } + /// Parse a `cmd:arg` style control command. + /// Returns `(action, target_branch)`. + fn parse_ctl_command<'a>(&self, cmd: &'a str) -> Option<(&'a str, String)> { + if let Some(pos) = cmd.find(':') { + let action = cmd[..pos].trim(); + let arg = cmd[pos + 1..].trim(); + if arg.is_empty() { + None + } else { + Some((action, arg.to_string())) + } + } else { + // Bare command — target is the current mount branch + Some((cmd.trim(), self.get_branch_name())) + } + } + /// Handle a write to the root ctl file. pub(crate) fn handle_root_ctl_write(&mut self, data: &[u8], reply: ReplyWrite) { let cmd = String::from_utf8_lossy(data).trim().to_string(); let cmd_lower = cmd.to_lowercase(); - let branch_name = self.get_branch_name(); - log::info!("Control command: '{}' for branch '{}'", cmd, branch_name); + log::info!("Control command: '{}'", cmd); - // Handle switch command: "switch:branchname" - if cmd_lower.starts_with("switch:") { - let new_branch = cmd[7..].trim(); - if new_branch.is_empty() { - log::warn!("Empty branch name in switch command"); + let (action, target) = match self.parse_ctl_command(&cmd_lower) { + Some(pair) => pair, + None => { + log::warn!("Empty argument in control command: {}", cmd); reply.error(libc::EINVAL); return; } - if !self.manager.is_branch_valid(new_branch) { - log::warn!("Branch '{}' does not exist", new_branch); - reply.error(libc::ENOENT); - return; + }; + + match action { + "switch" => { + if !self.manager.is_branch_valid(&target) { + log::warn!("Branch '{}' does not exist", target); + reply.error(libc::ENOENT); + return; + } + self.switch_to_branch(&target); + log::info!("Switched to branch '{}'", target); + reply.written(data.len() as u32); } - self.switch_to_branch(new_branch); - log::info!("Switched to branch '{}'", new_branch); - reply.written(data.len() as u32); - return; - } + "commit" | "abort" => { + let result = if action == "commit" { + self.manager.commit(&target) + } else { + self.manager.abort(&target) + }; - let result = match cmd_lower.as_str() { - "commit" => self.manager.commit(&branch_name), - "abort" => self.manager.abort(&branch_name), + match result { + Ok(parent) => { + self.inodes.clear_prefix(&format!("/@{}", target)); + // Only switch mount if the target was the current branch + let current = self.get_branch_name(); + if current == target { + self.switch_to_branch(&parent); + log::info!( + "Root ctl {} '{}', switched to '{}'", + action, + target, + parent + ); + } else { + self.current_epoch + .store(self.manager.get_epoch(), Ordering::SeqCst); + log::info!( + "Root ctl {} '{}' (mount stays on '{}')", + action, + target, + current + ); + } + reply.written(data.len() as u32) + } + Err(BranchError::Conflict(_)) => { + log::warn!("Root ctl {} conflict for '{}'", action, target); + reply.error(libc::ESTALE); + } + Err(e) => { + log::error!("Control command failed: {}", e); + reply.error(libc::EIO); + } + } + } _ => { log::warn!("Unknown control command: {}", cmd); reply.error(libc::EINVAL); - return; - } - }; - - match result { - Ok(parent) => { - // Root ctl always operates on the current mount branch, which - // gets deleted on commit/abort. We must switch to the parent - // so the mount stays valid. - self.switch_to_branch(&parent); - log::info!("Switched to branch '{}' after {}", parent, cmd_lower); - reply.written(data.len() as u32) - } - Err(BranchError::Conflict(_)) => { - log::warn!("Root ctl {} conflict for '{}'", cmd_lower, branch_name); - reply.error(libc::ESTALE); - } - Err(e) => { - log::error!("Control command failed: {}", e); - reply.error(libc::EIO); } } } diff --git a/src/fs_helpers.rs b/src/fs_helpers.rs index e5bc399..8f313aa 100644 --- a/src/fs_helpers.rs +++ b/src/fs_helpers.rs @@ -4,7 +4,7 @@ use std::time::UNIX_EPOCH; use fuser::{FileAttr, FileType}; -use crate::fs::{BranchFs, BLOCK_SIZE}; +use crate::fs::{BranchFs, BLOCK_SIZE, CTL_INO}; use crate::storage; impl BranchFs { @@ -116,9 +116,12 @@ impl BranchFs { /// Return a synthetic ctl-file FileAttr. pub(crate) fn ctl_file_attr(&self, ino: u64) -> FileAttr { + // Report a non-zero size so the kernel issues read() calls. + // The actual content length is determined by the read handler. + let size = if ino == CTL_INO { 256 } else { 0 }; FileAttr { ino, - size: 0, + size, blocks: 0, atime: UNIX_EPOCH, mtime: UNIX_EPOCH, diff --git a/src/main.rs b/src/main.rs index 3de8467..84a54fb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -52,21 +52,29 @@ enum Commands { storage: PathBuf, }, - /// Commit branch to base + /// Commit a branch (merges into parent) Commit { - /// Mount point of the branch to commit + /// Mount point mountpoint: PathBuf, + /// Branch to commit (default: current mount branch) + #[arg(long, short)] + branch: Option, + /// Storage directory #[arg(long, default_value = "/var/lib/branchfs")] storage: PathBuf, }, - /// Abort branch + /// Abort a branch (discards changes) Abort { - /// Mount point of the branch to abort + /// Mount point mountpoint: PathBuf, + /// Branch to abort (default: current mount branch) + #[arg(long, short)] + branch: Option, + /// Storage directory #[arg(long, default_value = "/var/lib/branchfs")] storage: PathBuf, @@ -100,44 +108,16 @@ fn send_request(storage: &Path, request: &Request) -> Result { .map_err(|e| anyhow::anyhow!("Failed to communicate with daemon: {}", e)) } -/// Determine the parent branch of the mount's current branch. -/// Returns "main" if the current branch is unknown or has no parent. -fn get_parent_branch(storage: &Path, mountpoint: &Path) -> String { - // Ask the daemon what branch this mount is currently on - let current = match send_request( - storage, - &Request::GetMountBranch { - mountpoint: mountpoint.to_string_lossy().to_string(), - }, - ) { - Ok(resp) if resp.ok => resp - .data - .and_then(|d| d.as_str().map(|s| s.to_string())) - .unwrap_or_else(|| "main".to_string()), - _ => return "main".to_string(), - }; - - if current == "main" { - return "main".to_string(); - } - - // Get the branch list to find the parent - let list_resp = match send_request(storage, &Request::List) { - Ok(resp) if resp.ok => resp, - _ => return "main".to_string(), - }; - - if let Some(data) = list_resp.data { - if let Some(branches) = data.as_array() { - for branch in branches { - if branch["name"].as_str() == Some(¤t) { - return branch["parent"].as_str().unwrap_or("main").to_string(); - } - } +/// Get the current mount branch by reading the FUSE ctl file. +fn get_current_branch(mountpoint: &Path) -> String { + let ctl_path = mountpoint.join(".branchfs_ctl"); + match std::fs::read_to_string(&ctl_path) { + Ok(branch) => { + let trimmed = branch.trim(); + if trimmed.is_empty() { "main".to_string() } else { trimmed.to_string() } } + Err(_) => "main".to_string(), } - - "main".to_string() } fn main() -> Result<()> { @@ -244,64 +224,67 @@ fn main() -> Result<()> { Commands::Commit { mountpoint, + branch, storage, } => { let mountpoint = mountpoint.canonicalize()?; let storage = storage.canonicalize()?; let ctl_path = mountpoint.join(".branchfs_ctl"); - // Determine parent branch before commit (FUSE handler will switch to it) - let parent = get_parent_branch(&storage, &mountpoint); + let target = branch.unwrap_or_else(|| get_current_branch(&mountpoint)); let mut file = std::fs::OpenOptions::new() .write(true) .open(&ctl_path) .map_err(|e| anyhow::anyhow!("Failed to open control file: {}", e))?; - file.write_all(b"commit") + file.write_all(format!("commit:{}", target).as_bytes()) .map_err(|e| anyhow::anyhow!("Commit failed: {}", e))?; - // Notify daemon that we've switched to the parent branch + // Notify daemon of the branch change (FUSE handler switches + // to parent only if target was the current mount branch). + let new_current = get_current_branch(&mountpoint); let _ = send_request( &storage, &Request::NotifySwitch { mountpoint: mountpoint.to_string_lossy().to_string(), - branch: parent, + branch: new_current, }, ); - println!("Committed branch at {:?}", mountpoint); + println!("Committed branch '{}' at {:?}", target, mountpoint); } Commands::Abort { mountpoint, + branch, storage, } => { let mountpoint = mountpoint.canonicalize()?; let storage = storage.canonicalize()?; let ctl_path = mountpoint.join(".branchfs_ctl"); - // Determine parent branch before abort (FUSE handler will switch to it) - let parent = get_parent_branch(&storage, &mountpoint); + let target = branch.unwrap_or_else(|| get_current_branch(&mountpoint)); let mut file = std::fs::OpenOptions::new() .write(true) .open(&ctl_path) .map_err(|e| anyhow::anyhow!("Failed to open control file: {}", e))?; - file.write_all(b"abort") + file.write_all(format!("abort:{}", target).as_bytes()) .map_err(|e| anyhow::anyhow!("Abort failed: {}", e))?; - // Notify daemon that we've switched to the parent branch + // Notify daemon of the branch change + let new_current = get_current_branch(&mountpoint); let _ = send_request( &storage, &Request::NotifySwitch { mountpoint: mountpoint.to_string_lossy().to_string(), - branch: parent, + branch: new_current, }, ); - println!("Aborted branch at {:?}", mountpoint); + println!("Aborted branch '{}' at {:?}", target, mountpoint); } Commands::List { storage } => { From 05dd9af36e66b2efc1a6a8b68566f9a8a1d6d0e2 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 9 Feb 2026 22:20:39 -0800 Subject: [PATCH 3/7] Clean up ctl file design: root ctl for switch only, per-branch ctl for commit/abort Signed-off-by: Cong Wang --- src/fs_ctl.rs | 97 +++++++--------------------------------- src/main.rs | 103 +++++++++++++++++++++++++++---------------- tests/test_helper.sh | 14 +++++- 3 files changed, 94 insertions(+), 120 deletions(-) diff --git a/src/fs_ctl.rs b/src/fs_ctl.rs index 54674d4..914d373 100644 --- a/src/fs_ctl.rs +++ b/src/fs_ctl.rs @@ -34,95 +34,32 @@ impl BranchFs { None } - /// Parse a `cmd:arg` style control command. - /// Returns `(action, target_branch)`. - fn parse_ctl_command<'a>(&self, cmd: &'a str) -> Option<(&'a str, String)> { - if let Some(pos) = cmd.find(':') { - let action = cmd[..pos].trim(); - let arg = cmd[pos + 1..].trim(); - if arg.is_empty() { - None - } else { - Some((action, arg.to_string())) - } - } else { - // Bare command — target is the current mount branch - Some((cmd.trim(), self.get_branch_name())) - } - } - /// Handle a write to the root ctl file. + /// Only supports `switch:` — commit/abort go through + /// per-branch ctl files (`/@/.branchfs_ctl`). pub(crate) fn handle_root_ctl_write(&mut self, data: &[u8], reply: ReplyWrite) { let cmd = String::from_utf8_lossy(data).trim().to_string(); let cmd_lower = cmd.to_lowercase(); - log::info!("Control command: '{}'", cmd); + log::info!("Root ctl command: '{}'", cmd); - let (action, target) = match self.parse_ctl_command(&cmd_lower) { - Some(pair) => pair, - None => { - log::warn!("Empty argument in control command: {}", cmd); + if let Some(new_branch) = cmd_lower.strip_prefix("switch:") { + let new_branch = new_branch.trim(); + if new_branch.is_empty() { + log::warn!("Empty branch name in switch command"); reply.error(libc::EINVAL); return; } - }; - - match action { - "switch" => { - if !self.manager.is_branch_valid(&target) { - log::warn!("Branch '{}' does not exist", target); - reply.error(libc::ENOENT); - return; - } - self.switch_to_branch(&target); - log::info!("Switched to branch '{}'", target); - reply.written(data.len() as u32); - } - "commit" | "abort" => { - let result = if action == "commit" { - self.manager.commit(&target) - } else { - self.manager.abort(&target) - }; - - match result { - Ok(parent) => { - self.inodes.clear_prefix(&format!("/@{}", target)); - // Only switch mount if the target was the current branch - let current = self.get_branch_name(); - if current == target { - self.switch_to_branch(&parent); - log::info!( - "Root ctl {} '{}', switched to '{}'", - action, - target, - parent - ); - } else { - self.current_epoch - .store(self.manager.get_epoch(), Ordering::SeqCst); - log::info!( - "Root ctl {} '{}' (mount stays on '{}')", - action, - target, - current - ); - } - reply.written(data.len() as u32) - } - Err(BranchError::Conflict(_)) => { - log::warn!("Root ctl {} conflict for '{}'", action, target); - reply.error(libc::ESTALE); - } - Err(e) => { - log::error!("Control command failed: {}", e); - reply.error(libc::EIO); - } - } - } - _ => { - log::warn!("Unknown control command: {}", cmd); - reply.error(libc::EINVAL); + if !self.manager.is_branch_valid(new_branch) { + log::warn!("Branch '{}' does not exist", new_branch); + reply.error(libc::ENOENT); + return; } + self.switch_to_branch(new_branch); + log::info!("Switched to branch '{}'", new_branch); + reply.written(data.len() as u32); + } else { + log::warn!("Unknown root ctl command: '{}' (use /@branch/.branchfs_ctl for commit/abort)", cmd); + reply.error(libc::EINVAL); } } diff --git a/src/main.rs b/src/main.rs index 84a54fb..5363754 100644 --- a/src/main.rs +++ b/src/main.rs @@ -52,29 +52,21 @@ enum Commands { storage: PathBuf, }, - /// Commit a branch (merges into parent) + /// Commit branch to base Commit { - /// Mount point + /// Mount point of the branch to commit mountpoint: PathBuf, - /// Branch to commit (default: current mount branch) - #[arg(long, short)] - branch: Option, - /// Storage directory #[arg(long, default_value = "/var/lib/branchfs")] storage: PathBuf, }, - /// Abort a branch (discards changes) + /// Abort branch Abort { - /// Mount point + /// Mount point of the branch to abort mountpoint: PathBuf, - /// Branch to abort (default: current mount branch) - #[arg(long, short)] - branch: Option, - /// Storage directory #[arg(long, default_value = "/var/lib/branchfs")] storage: PathBuf, @@ -108,16 +100,40 @@ fn send_request(storage: &Path, request: &Request) -> Result { .map_err(|e| anyhow::anyhow!("Failed to communicate with daemon: {}", e)) } -/// Get the current mount branch by reading the FUSE ctl file. -fn get_current_branch(mountpoint: &Path) -> String { - let ctl_path = mountpoint.join(".branchfs_ctl"); - match std::fs::read_to_string(&ctl_path) { - Ok(branch) => { - let trimmed = branch.trim(); - if trimmed.is_empty() { "main".to_string() } else { trimmed.to_string() } +/// Get the current mount branch from the daemon. +fn get_mount_branch(storage: &Path, mountpoint: &Path) -> Result { + let resp = send_request( + storage, + &Request::GetMountBranch { + mountpoint: mountpoint.to_string_lossy().to_string(), + }, + )?; + if !resp.ok { + anyhow::bail!("{}", resp.error.unwrap_or_else(|| "unknown error".into())); + } + resp.data + .and_then(|d| d.as_str().map(|s| s.to_string())) + .ok_or_else(|| anyhow::anyhow!("daemon returned no branch info")) +} + +/// Look up the parent of a branch from the daemon's branch list. +fn get_parent_of(storage: &Path, branch: &str) -> Result { + let resp = send_request(storage, &Request::List)?; + if !resp.ok { + anyhow::bail!("{}", resp.error.unwrap_or_else(|| "unknown error".into())); + } + if let Some(data) = resp.data { + if let Some(branches) = data.as_array() { + for b in branches { + if b["name"].as_str() == Some(branch) { + if let Some(parent) = b["parent"].as_str() { + return Ok(parent.to_string()); + } + } + } } - Err(_) => "main".to_string(), } + anyhow::bail!("Could not determine parent of branch '{}'", branch) } fn main() -> Result<()> { @@ -224,67 +240,76 @@ fn main() -> Result<()> { Commands::Commit { mountpoint, - branch, storage, } => { let mountpoint = mountpoint.canonicalize()?; let storage = storage.canonicalize()?; - let ctl_path = mountpoint.join(".branchfs_ctl"); - let target = branch.unwrap_or_else(|| get_current_branch(&mountpoint)); + let branch = get_mount_branch(&storage, &mountpoint)?; + if branch == "main" { + anyhow::bail!("Cannot commit main branch"); + } + // Pre-compute parent before commit (FUSE handler will switch to it) + let parent = get_parent_of(&storage, &branch)?; + + // Write to the per-branch ctl file + let ctl_path = mountpoint.join(format!("@{}", branch)).join(".branchfs_ctl"); let mut file = std::fs::OpenOptions::new() .write(true) .open(&ctl_path) - .map_err(|e| anyhow::anyhow!("Failed to open control file: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to open branch control file: {}", e))?; - file.write_all(format!("commit:{}", target).as_bytes()) + file.write_all(b"commit") .map_err(|e| anyhow::anyhow!("Commit failed: {}", e))?; - // Notify daemon of the branch change (FUSE handler switches - // to parent only if target was the current mount branch). - let new_current = get_current_branch(&mountpoint); + // Notify daemon that mount switched to parent let _ = send_request( &storage, &Request::NotifySwitch { mountpoint: mountpoint.to_string_lossy().to_string(), - branch: new_current, + branch: parent, }, ); - println!("Committed branch '{}' at {:?}", target, mountpoint); + println!("Committed branch at {:?}", mountpoint); } Commands::Abort { mountpoint, - branch, storage, } => { let mountpoint = mountpoint.canonicalize()?; let storage = storage.canonicalize()?; - let ctl_path = mountpoint.join(".branchfs_ctl"); - let target = branch.unwrap_or_else(|| get_current_branch(&mountpoint)); + let branch = get_mount_branch(&storage, &mountpoint)?; + if branch == "main" { + anyhow::bail!("Cannot abort main branch"); + } + + // Pre-compute parent before abort (FUSE handler will switch to it) + let parent = get_parent_of(&storage, &branch)?; + // Write to the per-branch ctl file + let ctl_path = mountpoint.join(format!("@{}", branch)).join(".branchfs_ctl"); let mut file = std::fs::OpenOptions::new() .write(true) .open(&ctl_path) - .map_err(|e| anyhow::anyhow!("Failed to open control file: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to open branch control file: {}", e))?; - file.write_all(format!("abort:{}", target).as_bytes()) + file.write_all(b"abort") .map_err(|e| anyhow::anyhow!("Abort failed: {}", e))?; - // Notify daemon of the branch change - let new_current = get_current_branch(&mountpoint); + // Notify daemon that mount switched to parent let _ = send_request( &storage, &Request::NotifySwitch { mountpoint: mountpoint.to_string_lossy().to_string(), - branch: new_current, + branch: parent, }, ); - println!("Aborted branch '{}' at {:?}", target, mountpoint); + println!("Aborted branch at {:?}", mountpoint); } Commands::List { storage } => { diff --git a/tests/test_helper.sh b/tests/test_helper.sh index 83537cd..c47d200 100755 --- a/tests/test_helper.sh +++ b/tests/test_helper.sh @@ -125,10 +125,22 @@ do_abort() { "$BRANCHFS" abort "$TEST_MNT" --storage "$TEST_STORAGE" } -# Switch to a branch by writing to the ctl file +# Switch to a branch by writing to the ctl file and notifying daemon do_switch() { local name="$1" echo -n "switch:${name}" > "$TEST_MNT/.branchfs_ctl" + # Notify daemon of the switch so get_mount_branch() returns correct data + local canon_mnt + canon_mnt="$(readlink -f "$TEST_MNT")" + python3 -c " +import socket, json, sys +s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +s.connect(sys.argv[1]) +msg = json.dumps({'cmd': 'notify_switch', 'mountpoint': sys.argv[2], 'branch': sys.argv[3]}) + chr(10) +s.sendall(msg.encode()) +s.recv(4096) +s.close() +" "$TEST_STORAGE/daemon.sock" "$canon_mnt" "$name" 2>/dev/null || true } # List branches From a3665dbe0413fcb407600b33cf20a5b8acebc19f Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Tue, 10 Feb 2026 08:50:57 -0800 Subject: [PATCH 4/7] Consolidate current-branch tracking into BranchManager Signed-off-by: Cong Wang --- src/branch.rs | 65 ++++++++++++++++++++++++++++++++++++++++----- src/daemon.rs | 73 ++++++++++++++++----------------------------------- src/fs.rs | 17 +++++++----- src/fs_ctl.rs | 3 ++- src/main.rs | 58 +++------------------------------------- 5 files changed, 97 insertions(+), 119 deletions(-) diff --git a/src/branch.rs b/src/branch.rs index 3bda86d..3862fb2 100644 --- a/src/branch.rs +++ b/src/branch.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fs::{self, File}; use std::io::{BufRead, BufReader, Write}; use std::path::{Path, PathBuf}; @@ -140,14 +140,16 @@ pub struct BranchManager { pub storage_path: PathBuf, pub base_path: PathBuf, pub workspace_path: PathBuf, - branches: RwLock>, + branches: RwLock>, pub epoch: AtomicU64, /// Notifiers for invalidating kernel cache on commit/abort /// Maps (branch_name, mountpoint) -> Notifier - notifiers: Mutex>>, + notifiers: Mutex>>, /// Track opened file inodes per branch for cache invalidation /// Maps branch_name -> Set of inodes - opened_inodes: Mutex>>, + opened_inodes: Mutex>>, + /// Current branch per mount — single source of truth + mount_branches: RwLock>, } impl BranchManager { @@ -155,7 +157,7 @@ impl BranchManager { fs::create_dir_all(&storage_path)?; // Always start fresh with just the "main" branch - let mut branches = std::collections::HashMap::new(); + let mut branches = HashMap::new(); let main_branch = Branch::new("main", None, &storage_path, 0)?; branches.insert("main".to_string(), main_branch); @@ -165,11 +167,60 @@ impl BranchManager { workspace_path, branches: RwLock::new(branches), epoch: AtomicU64::new(0), - notifiers: Mutex::new(std::collections::HashMap::new()), - opened_inodes: Mutex::new(std::collections::HashMap::new()), + notifiers: Mutex::new(HashMap::new()), + opened_inodes: Mutex::new(HashMap::new()), + mount_branches: RwLock::new(HashMap::new()), }) } + /// Register a mount's initial branch (called before FUSE spawn). + pub fn set_mount_branch(&self, mountpoint: &Path, branch: &str) { + self.mount_branches + .write() + .insert(mountpoint.to_path_buf(), branch.to_string()); + } + + /// Read the current branch for a mount. + pub fn get_mount_branch(&self, mountpoint: &Path) -> Option { + self.mount_branches.read().get(mountpoint).cloned() + } + + /// Atomically switch a mount's branch, re-keying the notifier map. + pub fn switch_mount_branch(&self, mountpoint: &Path, new_branch: &str) { + // Lock order: mount_branches → notifiers (never reversed) + let mut mb = self.mount_branches.write(); + let old_branch = mb + .insert(mountpoint.to_path_buf(), new_branch.to_string()); + + // Re-key notifier from (old, mount) → (new, mount) + if let Some(old) = old_branch { + let mut notifiers = self.notifiers.lock(); + if let Some(notifier) = notifiers.remove(&(old.clone(), mountpoint.to_path_buf())) { + notifiers.insert( + (new_branch.to_string(), mountpoint.to_path_buf()), + notifier, + ); + } + log::info!( + "switch_mount_branch: {:?} '{}' -> '{}'", + mountpoint, + old, + new_branch + ); + } + } + + /// Remove a mount's branch tracking and notifier (used on unmount). + pub fn unregister_mount(&self, mountpoint: &Path) { + let mut mb = self.mount_branches.write(); + let old_branch = mb.remove(mountpoint); + if let Some(old) = old_branch { + self.notifiers + .lock() + .remove(&(old, mountpoint.to_path_buf())); + } + } + pub fn create_branch(&self, name: &str, parent: &str) -> Result<()> { let start = Instant::now(); validate_branch_name(name)?; diff --git a/src/daemon.rs b/src/daemon.rs index 8c45171..5979e42 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -32,10 +32,6 @@ pub enum Request { name: String, parent: String, }, - NotifySwitch { - mountpoint: String, - branch: String, - }, GetMountBranch { mountpoint: String, }, @@ -78,10 +74,10 @@ impl Response { } } -/// Per-mount state including the FUSE session and current branch +/// Per-mount state (FUSE session handle — kept alive until unmount) pub struct MountInfo { + #[allow(dead_code)] session: BackgroundSession, - current_branch: String, } pub struct Daemon { @@ -145,7 +141,10 @@ impl Daemon { mountpoint: &Path, passthrough: bool, ) -> Result<()> { - let fs = BranchFs::new(self.manager.clone(), branch_name.to_string(), passthrough); + // Register the mount branch *before* creating BranchFs so get_branch_name() works + self.manager.set_mount_branch(mountpoint, branch_name); + + let fs = BranchFs::new(self.manager.clone(), mountpoint.to_path_buf(), passthrough); let options = vec![ MountOption::FSName("branchfs".to_string()), MountOption::DefaultPermissions, @@ -157,18 +156,21 @@ impl Daemon { mountpoint, ); - let session = - fuser::spawn_mount2(fs, mountpoint, &options).map_err(crate::error::BranchError::Io)?; + let session = match fuser::spawn_mount2(fs, mountpoint, &options) { + Ok(s) => s, + Err(e) => { + // Clean up on failure + self.manager.unregister_mount(mountpoint); + return Err(crate::error::BranchError::Io(e)); + } + }; // Get the notifier for cache invalidation and register it with the manager let notifier = Arc::new(session.notifier()); self.manager .register_notifier(branch_name, mountpoint.to_path_buf(), notifier); - let mount_info = MountInfo { - session, - current_branch: branch_name.to_string(), - }; + let mount_info = MountInfo { session }; self.mounts .lock() @@ -178,12 +180,12 @@ impl Daemon { } pub fn unmount(&self, mountpoint: &Path) -> Result<()> { - let (should_shutdown, mount_info) = { + let should_shutdown = { let mut mounts = self.mounts.lock(); - if let Some(info) = mounts.remove(mountpoint) { + if mounts.remove(mountpoint).is_some() { log::info!("Unmounted {:?}", mountpoint); // The BackgroundSession drop will handle FUSE cleanup - (mounts.is_empty(), Some(info)) + mounts.is_empty() } else { return Err(crate::error::BranchError::MountNotFound(format!( "{:?}", @@ -192,10 +194,7 @@ impl Daemon { } }; - if let Some(info) = mount_info { - self.manager - .unregister_notifier(&info.current_branch, mountpoint); - } + self.manager.unregister_mount(mountpoint); if should_shutdown { log::info!("All mounts removed, daemon will exit"); @@ -209,9 +208,8 @@ impl Daemon { let mut mounts = self.mounts.lock(); let mountpoints: Vec = mounts.keys().cloned().collect(); for mountpoint in &mountpoints { - if let Some(info) = mounts.remove(mountpoint) { - self.manager - .unregister_notifier(&info.current_branch, mountpoint); + if mounts.remove(mountpoint).is_some() { + self.manager.unregister_mount(mountpoint); // BackgroundSession dropped here → FUSE unmount log::info!("Cleaned up mount at {:?}", mountpoint); } @@ -329,35 +327,10 @@ impl Daemon { Ok(()) => Response::success(), Err(e) => Response::error(&format!("{}", e)), }, - Request::NotifySwitch { mountpoint, branch } => { - let path = PathBuf::from(&mountpoint); - let mut mounts = self.mounts.lock(); - if let Some(ref mut info) = mounts.get_mut(&path) { - // Unregister old notifier - self.manager - .unregister_notifier(&info.current_branch, &path); - // Update tracked branch - let old_branch = std::mem::replace(&mut info.current_branch, branch.clone()); - // Register notifier for new branch - let notifier = Arc::new(info.session.notifier()); - self.manager - .register_notifier(&branch, path.clone(), notifier); - log::info!( - "Mount {:?} switched from '{}' to '{}'", - path, - old_branch, - branch - ); - Response::success() - } else { - Response::error(&format!("Mount not found: {:?}", path)) - } - } Request::GetMountBranch { mountpoint } => { let path = PathBuf::from(&mountpoint); - let mounts = self.mounts.lock(); - if let Some(info) = mounts.get(&path) { - Response::success_with_data(serde_json::json!(info.current_branch)) + if let Some(branch) = self.manager.get_mount_branch(&path) { + Response::success_with_data(serde_json::json!(branch)) } else { Response::error(&format!("Mount not found: {:?}", path)) } diff --git a/src/fs.rs b/src/fs.rs index debab45..a1eb261 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::ffi::OsStr; use std::fs::File; use std::io::{Read as IoRead, Seek, SeekFrom}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -132,7 +132,7 @@ impl WriteFileCache { pub struct BranchFs { pub(crate) manager: Arc, pub(crate) inodes: InodeManager, - pub(crate) branch_name: RwLock, + pub(crate) mountpoint: PathBuf, pub(crate) current_epoch: AtomicU64, /// Per-branch ctl inode numbers: branch_name → ino pub(crate) branch_ctl_inodes: RwLock>, @@ -154,12 +154,12 @@ pub struct BranchFs { } impl BranchFs { - pub fn new(manager: Arc, branch_name: String, passthrough: bool) -> Self { + pub fn new(manager: Arc, mountpoint: PathBuf, passthrough: bool) -> Self { let current_epoch = manager.get_epoch(); Self { manager, inodes: InodeManager::new(), - branch_name: RwLock::new(branch_name), + mountpoint, current_epoch: AtomicU64::new(current_epoch), branch_ctl_inodes: RwLock::new(HashMap::new()), // Reserve a range well below CTL_INO (u64::MAX - 1) for branch ctl inodes. @@ -176,7 +176,9 @@ impl BranchFs { } pub(crate) fn get_branch_name(&self) -> String { - self.branch_name.read().clone() + self.manager + .get_mount_branch(&self.mountpoint) + .unwrap_or_else(|| "main".into()) } pub(crate) fn is_stale(&self) -> bool { @@ -185,9 +187,10 @@ impl BranchFs { || !self.manager.is_branch_valid(&branch_name) } - /// Switch to a different branch (used after commit/abort to switch to main) + /// Switch to a different branch (used after commit/abort to switch to parent) pub(crate) fn switch_to_branch(&self, new_branch: &str) { - *self.branch_name.write() = new_branch.to_string(); + self.manager + .switch_mount_branch(&self.mountpoint, new_branch); self.current_epoch .store(self.manager.get_epoch(), Ordering::SeqCst); // Clear inode cache since we're on a different branch now diff --git a/src/fs_ctl.rs b/src/fs_ctl.rs index 914d373..9f5c337 100644 --- a/src/fs_ctl.rs +++ b/src/fs_ctl.rs @@ -88,7 +88,8 @@ impl BranchFs { // Only switch the mount if the operated branch is the current mount branch let current = self.get_branch_name(); if current == branch { - *self.branch_name.write() = parent.clone(); + self.manager + .switch_mount_branch(&self.mountpoint, &parent); log::info!( "Branch ctl {} succeeded for '{}', switched to '{}'", cmd_lower, diff --git a/src/main.rs b/src/main.rs index 5363754..496154a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -116,26 +116,6 @@ fn get_mount_branch(storage: &Path, mountpoint: &Path) -> Result { .ok_or_else(|| anyhow::anyhow!("daemon returned no branch info")) } -/// Look up the parent of a branch from the daemon's branch list. -fn get_parent_of(storage: &Path, branch: &str) -> Result { - let resp = send_request(storage, &Request::List)?; - if !resp.ok { - anyhow::bail!("{}", resp.error.unwrap_or_else(|| "unknown error".into())); - } - if let Some(data) = resp.data { - if let Some(branches) = data.as_array() { - for b in branches { - if b["name"].as_str() == Some(branch) { - if let Some(parent) = b["parent"].as_str() { - return Ok(parent.to_string()); - } - } - } - } - } - anyhow::bail!("Could not determine parent of branch '{}'", branch) -} - fn main() -> Result<()> { env_logger::init(); let cli = Cli::parse(); @@ -202,7 +182,8 @@ fn main() -> Result<()> { )?; if response.ok { - // Switch to the new branch + // Switch to the new branch via ctl file + // (FUSE handler updates manager.mount_branches internally) let ctl_path = mountpoint.join(".branchfs_ctl"); let mut file = std::fs::OpenOptions::new() @@ -219,15 +200,6 @@ fn main() -> Result<()> { file.write_all(format!("switch:{}", name).as_bytes()) .map_err(|e| anyhow::anyhow!("Failed to switch to branch: {}", e))?; - // Notify daemon of the switch - let _ = send_request( - &storage, - &Request::NotifySwitch { - mountpoint: mountpoint.to_string_lossy().to_string(), - branch: name.clone(), - }, - ); - println!( "Created and switched to branch '{}' (parent: '{}')", name, parent @@ -250,10 +222,8 @@ fn main() -> Result<()> { anyhow::bail!("Cannot commit main branch"); } - // Pre-compute parent before commit (FUSE handler will switch to it) - let parent = get_parent_of(&storage, &branch)?; - // Write to the per-branch ctl file + // (FUSE handler does commit + switch_mount_branch internally) let ctl_path = mountpoint.join(format!("@{}", branch)).join(".branchfs_ctl"); let mut file = std::fs::OpenOptions::new() .write(true) @@ -263,15 +233,6 @@ fn main() -> Result<()> { file.write_all(b"commit") .map_err(|e| anyhow::anyhow!("Commit failed: {}", e))?; - // Notify daemon that mount switched to parent - let _ = send_request( - &storage, - &Request::NotifySwitch { - mountpoint: mountpoint.to_string_lossy().to_string(), - branch: parent, - }, - ); - println!("Committed branch at {:?}", mountpoint); } @@ -287,10 +248,8 @@ fn main() -> Result<()> { anyhow::bail!("Cannot abort main branch"); } - // Pre-compute parent before abort (FUSE handler will switch to it) - let parent = get_parent_of(&storage, &branch)?; - // Write to the per-branch ctl file + // (FUSE handler does abort + switch_mount_branch internally) let ctl_path = mountpoint.join(format!("@{}", branch)).join(".branchfs_ctl"); let mut file = std::fs::OpenOptions::new() .write(true) @@ -300,15 +259,6 @@ fn main() -> Result<()> { file.write_all(b"abort") .map_err(|e| anyhow::anyhow!("Abort failed: {}", e))?; - // Notify daemon that mount switched to parent - let _ = send_request( - &storage, - &Request::NotifySwitch { - mountpoint: mountpoint.to_string_lossy().to_string(), - branch: parent, - }, - ); - println!("Aborted branch at {:?}", mountpoint); } From d8cc4e971517b832c75ae9742391f1a1df7ce183 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Tue, 10 Feb 2026 12:21:42 -0800 Subject: [PATCH 5/7] Remove string parameters from branch ioctls, derive context from ctl fd inode Signed-off-by: Cong Wang --- src/fs.rs | 103 ++++++++++++------------------------------ tests/test_ioctl.rs | 108 ++++++++++++++++++++++---------------------- 2 files changed, 84 insertions(+), 127 deletions(-) diff --git a/src/fs.rs b/src/fs.rs index a1eb261..14445df 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -25,24 +25,9 @@ use crate::storage; pub(crate) const TTL: Duration = Duration::from_secs(0); pub(crate) const BLOCK_SIZE: u32 = 512; -pub const FS_IOC_BRANCH_CREATE: u32 = 0xC080_6200; // _IOWR('b', 0, [u8; 128]) -pub const FS_IOC_BRANCH_COMMIT: u32 = 0x4080_6201; // _IOW ('b', 1, [u8; 128]) -pub const FS_IOC_BRANCH_ABORT: u32 = 0x4080_6202; // _IOW ('b', 2, [u8; 128]) - -/// Parse a null-terminated branch name from an ioctl buffer. -fn parse_branch_name(in_data: &[u8]) -> Option { - if in_data.is_empty() { - return None; - } - let end = in_data.iter().position(|&b| b == 0).unwrap_or(in_data.len()); - let s = std::str::from_utf8(&in_data[..end]).ok()?; - let trimmed = s.trim(); - if trimmed.is_empty() { - None - } else { - Some(trimmed.to_string()) - } -} +pub const FS_IOC_BRANCH_CREATE: u32 = 0x8080_6200; // _IOR('b', 0, [u8; 128]) +pub const FS_IOC_BRANCH_COMMIT: u32 = 0x0000_6201; // _IO ('b', 1) +pub const FS_IOC_BRANCH_ABORT: u32 = 0x0000_6202; // _IO ('b', 2) pub(crate) const CTL_FILE: &str = ".branchfs_ctl"; pub(crate) const CTL_INO: u64 = u64::MAX - 1; @@ -416,16 +401,9 @@ impl Filesystem for BranchFs { return; } - // Looking up @child inside a branch dir (nested branch) - if let Some(child_branch) = name_str.strip_prefix('@') { - let children = self.manager.get_children(&branch); - if parent_rel == "/" && children.iter().any(|c| c == child_branch) { - let inode_path = format!("/@{}/@{}", branch, child_branch); - let ino = self.inodes.get_or_create(&inode_path, true); - reply.entry(&TTL, &self.synthetic_dir_attr(ino), 0); - } else { - reply.error(libc::ENOENT); - } + // @-prefixed names inside branch dirs are not valid (flat namespace) + if name_str.starts_with('@') { + reply.error(libc::ENOENT); return; } @@ -848,13 +826,6 @@ impl Filesystem for BranchFs { let ctl_ino = self.get_or_create_branch_ctl_ino(&branch); entries.push((ctl_ino, FileType::RegularFile, CTL_FILE.to_string())); - // Add @child virtual dirs for children of this branch - let children = self.manager.get_children(&branch); - for child in children { - let child_inode_path = format!("/@{}/@{}", branch, child); - let child_ino = self.inodes.get_or_create(&child_inode_path, true); - entries.push((child_ino, FileType::Directory, format!("@{}", child))); - } for (i, (e_ino, kind, name)) in entries.into_iter().enumerate().skip(offset as usize) @@ -1364,33 +1335,33 @@ impl Filesystem for BranchFs { fn ioctl( &mut self, _req: &Request, - _ino: u64, + ino: u64, _fh: u64, _flags: u32, cmd: u32, - in_data: &[u8], + _in_data: &[u8], _out_size: u32, reply: ReplyIoctl, ) { + // Resolve ino to the branch name this ctl fd refers to. + let branch_name = if ino == CTL_INO { + self.get_branch_name() + } else if let Some(name) = self.branch_for_ctl_ino(ino) { + name + } else { + reply.error(libc::ENOTTY); + return; + }; + match cmd { FS_IOC_BRANCH_CREATE => { - let parent_id = match parse_branch_name(in_data) { - Some(p) => p, - None => { - log::error!("ioctl CREATE: empty parent_id"); - reply.error(libc::EINVAL); - return; - } - }; let name = format!("branch-{}", uuid::Uuid::new_v4()); - log::info!("ioctl: CREATE branch '{}' from '{}'", name, parent_id); - match self.manager.create_branch(&name, &parent_id) { + log::info!("ioctl: CREATE branch '{}' from '{}'", name, branch_name); + match self.manager.create_branch(&name, &branch_name) { Ok(()) => { - // Update epoch so stale checks see the new branch self.current_epoch .store(self.manager.get_epoch(), Ordering::SeqCst); log::info!("Created branch '{}' (no mount switch)", name); - // Return the branch name in a 128-byte null-padded buffer let mut buf = [0u8; 128]; let name_bytes = name.as_bytes(); let len = name_bytes.len().min(127); @@ -1404,25 +1375,17 @@ impl Filesystem for BranchFs { } } FS_IOC_BRANCH_COMMIT => { - let branch_id = match parse_branch_name(in_data) { - Some(b) => b, - None => { - log::error!("ioctl COMMIT: empty branch_id"); - reply.error(libc::EINVAL); - return; - } - }; - log::info!("ioctl: COMMIT branch '{}'", branch_id); - match self.manager.commit(&branch_id) { + log::info!("ioctl: COMMIT branch '{}'", branch_name); + match self.manager.commit(&branch_name) { Ok(_parent) => { - self.inodes.clear_prefix(&format!("/@{}", branch_id)); + self.inodes.clear_prefix(&format!("/@{}", branch_name)); self.current_epoch .store(self.manager.get_epoch(), Ordering::SeqCst); - log::info!("Committed branch '{}' (no mount switch)", branch_id); + log::info!("Committed branch '{}' (no mount switch)", branch_name); reply.ioctl(0, &[]) } Err(BranchError::Conflict(_)) => { - log::warn!("commit conflict for branch '{}'", branch_id); + log::warn!("commit conflict for branch '{}'", branch_name); reply.error(libc::ESTALE); } Err(e) => { @@ -1432,21 +1395,13 @@ impl Filesystem for BranchFs { } } FS_IOC_BRANCH_ABORT => { - let branch_id = match parse_branch_name(in_data) { - Some(b) => b, - None => { - log::error!("ioctl ABORT: empty branch_id"); - reply.error(libc::EINVAL); - return; - } - }; - log::info!("ioctl: ABORT branch '{}'", branch_id); - match self.manager.abort(&branch_id) { + log::info!("ioctl: ABORT branch '{}'", branch_name); + match self.manager.abort(&branch_name) { Ok(_parent) => { - self.inodes.clear_prefix(&format!("/@{}", branch_id)); + self.inodes.clear_prefix(&format!("/@{}", branch_name)); self.current_epoch .store(self.manager.get_epoch(), Ordering::SeqCst); - log::info!("Aborted branch '{}' (no mount switch)", branch_id); + log::info!("Aborted branch '{}' (no mount switch)", branch_name); reply.ioctl(0, &[]) } Err(e) => { diff --git a/tests/test_ioctl.rs b/tests/test_ioctl.rs index 22399bb..2198f8b 100644 --- a/tests/test_ioctl.rs +++ b/tests/test_ioctl.rs @@ -12,21 +12,11 @@ use std::time::Duration; use branchfs::{FS_IOC_BRANCH_ABORT, FS_IOC_BRANCH_COMMIT, FS_IOC_BRANCH_CREATE}; -/// Call a data-carrying ioctl with a 128-byte buffer. -/// `input` is the null-terminated string to send. -/// Returns (ret, output_buf) where ret is the raw ioctl return value. -unsafe fn branch_ioctl_buf(fd: i32, cmd: u32, input: &str) -> (i32, [u8; 128]) { +/// Helper: CREATE a branch (parent derived from the ctl fd's inode). +/// Returns the new branch name from the 128-byte output buffer. +unsafe fn ioctl_create(fd: i32) -> Result { let mut buf = [0u8; 128]; - let bytes = input.as_bytes(); - let len = bytes.len().min(127); - buf[..len].copy_from_slice(&bytes[..len]); - let ret = libc::ioctl(fd, cmd as libc::c_ulong, buf.as_mut_ptr()); - (ret, buf) -} - -/// Helper: CREATE a branch from `parent`, returning the new branch name. -unsafe fn ioctl_create(fd: i32, parent: &str) -> Result { - let (ret, buf) = branch_ioctl_buf(fd, FS_IOC_BRANCH_CREATE, parent); + let ret = libc::ioctl(fd, FS_IOC_BRANCH_CREATE as libc::c_ulong, buf.as_mut_ptr()); if ret < 0 { return Err(*libc::__errno_location()); } @@ -34,16 +24,14 @@ unsafe fn ioctl_create(fd: i32, parent: &str) -> Result { Ok(String::from_utf8_lossy(&buf[..end]).to_string()) } -/// Helper: COMMIT a branch by name. -unsafe fn ioctl_commit(fd: i32, branch: &str) -> i32 { - let (ret, _) = branch_ioctl_buf(fd, FS_IOC_BRANCH_COMMIT, branch); - ret +/// Helper: COMMIT the branch identified by the ctl fd's inode. +unsafe fn ioctl_commit(fd: i32) -> i32 { + libc::ioctl(fd, FS_IOC_BRANCH_COMMIT as libc::c_ulong) } -/// Helper: ABORT a branch by name. -unsafe fn ioctl_abort(fd: i32, branch: &str) -> i32 { - let (ret, _) = branch_ioctl_buf(fd, FS_IOC_BRANCH_ABORT, branch); - ret +/// Helper: ABORT the branch identified by the ctl fd's inode. +unsafe fn ioctl_abort(fd: i32) -> i32 { + libc::ioctl(fd, FS_IOC_BRANCH_ABORT as libc::c_ulong) } struct TestFixture { @@ -125,7 +113,7 @@ impl TestFixture { thread::sleep(Duration::from_millis(300)); } - /// Open the control file; caller must keep the returned `File` alive. + /// Open the root control file; caller must keep the returned `File` alive. fn open_ctl(&self) -> fs::File { fs::OpenOptions::new() .read(true) @@ -133,6 +121,16 @@ impl TestFixture { .open(self.mnt.join(".branchfs_ctl")) .expect("open .branchfs_ctl") } + + /// Open a per-branch control file at `/@/.branchfs_ctl`. + fn open_branch_ctl(&self, branch: &str) -> fs::File { + let path = self.mnt.join(format!("@{}", branch)).join(".branchfs_ctl"); + fs::OpenOptions::new() + .read(true) + .write(true) + .open(&path) + .unwrap_or_else(|e| panic!("open branch ctl for '{}' at {:?}: {}", branch, path, e)) + } } impl Drop for TestFixture { @@ -152,10 +150,9 @@ fn test_ioctl_create_and_commit_new_file() { let fix = TestFixture::new("create_commit"); fix.mount(); let ctl = fix.open_ctl(); - let fd = ctl.as_raw_fd(); - // CREATE a branch from main - let branch = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); + // CREATE a branch from main (root ctl → current mount branch = main) + let branch = unsafe { ioctl_create(ctl.as_raw_fd()) }.expect("CREATE should succeed"); assert!(!branch.is_empty(), "branch name should not be empty"); // Write a new file on the branch via @branch virtual path @@ -169,8 +166,9 @@ fn test_ioctl_create_and_commit_new_file() { "new file must not appear in base before commit" ); - // COMMIT the branch - let ret = unsafe { ioctl_commit(fd, &branch) }; + // COMMIT the branch via its per-branch ctl + let bctl = fix.open_branch_ctl(&branch); + let ret = unsafe { ioctl_commit(bctl.as_raw_fd()) }; assert_eq!(ret, 0, "COMMIT should succeed"); // File should now be in base @@ -192,9 +190,8 @@ fn test_ioctl_modify_existing_and_commit() { let fix = TestFixture::new("modify_commit"); fix.mount(); let ctl = fix.open_ctl(); - let fd = ctl.as_raw_fd(); - let branch = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); + let branch = unsafe { ioctl_create(ctl.as_raw_fd()) }.expect("CREATE should succeed"); // Overwrite an existing base file via @branch path let branch_dir = fix.mnt.join(format!("@{}", branch)); @@ -206,7 +203,8 @@ fn test_ioctl_modify_existing_and_commit() { "base content\n" ); - let ret = unsafe { ioctl_commit(fd, &branch) }; + let bctl = fix.open_branch_ctl(&branch); + let ret = unsafe { ioctl_commit(bctl.as_raw_fd()) }; assert_eq!(ret, 0); // Base should reflect the modification @@ -224,16 +222,16 @@ fn test_ioctl_create_and_abort() { let fix = TestFixture::new("create_abort"); fix.mount(); let ctl = fix.open_ctl(); - let fd = ctl.as_raw_fd(); - let branch = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); + let branch = unsafe { ioctl_create(ctl.as_raw_fd()) }.expect("CREATE should succeed"); // Write a file and modify another via @branch path let branch_dir = fix.mnt.join(format!("@{}", branch)); fs::write(branch_dir.join("abort_file.txt"), "will be discarded\n").unwrap(); fs::write(branch_dir.join("file1.txt"), "modified\n").unwrap(); - let ret = unsafe { ioctl_abort(fd, &branch) }; + let bctl = fix.open_branch_ctl(&branch); + let ret = unsafe { ioctl_abort(bctl.as_raw_fd()) }; assert_eq!(ret, 0, "ABORT should succeed"); // Branch dir should be gone after abort @@ -257,15 +255,16 @@ fn test_ioctl_nested_create_and_commit() { let fix = TestFixture::new("nested"); fix.mount(); let ctl = fix.open_ctl(); - let fd = ctl.as_raw_fd(); - // First branch (child of main) - let branch1 = unsafe { ioctl_create(fd, "main") }.expect("first CREATE should succeed"); + // First branch (child of main) — CREATE via root ctl + let branch1 = unsafe { ioctl_create(ctl.as_raw_fd()) }.expect("first CREATE should succeed"); let b1_dir = fix.mnt.join(format!("@{}", branch1)); fs::write(b1_dir.join("level1.txt"), "level1\n").unwrap(); - // Second branch (child of first) - let branch2 = unsafe { ioctl_create(fd, &branch1) }.expect("second CREATE should succeed"); + // Second branch (child of first) — CREATE via branch1's ctl + let b1_ctl = fix.open_branch_ctl(&branch1); + let branch2 = + unsafe { ioctl_create(b1_ctl.as_raw_fd()) }.expect("second CREATE should succeed"); let b2_dir = fix.mnt.join(format!("@{}", branch2)); fs::write(b2_dir.join("level2.txt"), "level2\n").unwrap(); @@ -275,16 +274,17 @@ fn test_ioctl_nested_create_and_commit() { // level2 branch should also see level1 (inherited from parent) assert!(b2_dir.join("level1.txt").exists()); - // COMMIT level-2 → merges into level-1 - let ret = unsafe { ioctl_commit(fd, &branch2) }; + // COMMIT level-2 → merges into level-1 (via branch2's ctl) + let b2_ctl = fix.open_branch_ctl(&branch2); + let ret = unsafe { ioctl_commit(b2_ctl.as_raw_fd()) }; assert_eq!(ret, 0, "commit level-2 should succeed"); // Neither file should be in base yet assert!(!fix.base.join("level1.txt").exists()); assert!(!fix.base.join("level2.txt").exists()); - // COMMIT level-1 → merges into main / base - let ret = unsafe { ioctl_commit(fd, &branch1) }; + // COMMIT level-1 → merges into main / base (via branch1's ctl) + let ret = unsafe { ioctl_commit(b1_ctl.as_raw_fd()) }; assert_eq!(ret, 0, "commit level-1 should succeed"); // Both files in base now @@ -300,9 +300,9 @@ fn test_ioctl_commit_on_main_fails() { let fix = TestFixture::new("commit_main"); fix.mount(); let ctl = fix.open_ctl(); - let fd = ctl.as_raw_fd(); - let ret = unsafe { ioctl_commit(fd, "main") }; + // Root ctl resolves to "main" — commit on main should fail + let ret = unsafe { ioctl_commit(ctl.as_raw_fd()) }; assert!(ret < 0, "COMMIT on main should fail (got {})", ret); } @@ -312,9 +312,9 @@ fn test_ioctl_abort_on_main_fails() { let fix = TestFixture::new("abort_main"); fix.mount(); let ctl = fix.open_ctl(); - let fd = ctl.as_raw_fd(); - let ret = unsafe { ioctl_abort(fd, "main") }; + // Root ctl resolves to "main" — abort on main should fail + let ret = unsafe { ioctl_abort(ctl.as_raw_fd()) }; assert!(ret < 0, "ABORT on main should fail (got {})", ret); } @@ -326,27 +326,29 @@ fn test_ioctl_abort_returns_to_parent_branch() { let fix = TestFixture::new("abort_parent"); fix.mount(); let ctl = fix.open_ctl(); - let fd = ctl.as_raw_fd(); // CREATE first branch, write a file - let branch1 = unsafe { ioctl_create(fd, "main") }.expect("CREATE should succeed"); + let branch1 = unsafe { ioctl_create(ctl.as_raw_fd()) }.expect("CREATE should succeed"); let b1_dir = fix.mnt.join(format!("@{}", branch1)); fs::write(b1_dir.join("parent_file.txt"), "parent\n").unwrap(); - // CREATE second (nested) branch, write another file - let branch2 = unsafe { ioctl_create(fd, &branch1) }.expect("CREATE should succeed"); + // CREATE second (nested) branch via branch1's ctl, write another file + let b1_ctl = fix.open_branch_ctl(&branch1); + let branch2 = + unsafe { ioctl_create(b1_ctl.as_raw_fd()) }.expect("CREATE should succeed"); let b2_dir = fix.mnt.join(format!("@{}", branch2)); fs::write(b2_dir.join("child_file.txt"), "child\n").unwrap(); // ABORT second branch — child's data is discarded - let ret = unsafe { ioctl_abort(fd, &branch2) }; + let b2_ctl = fix.open_branch_ctl(&branch2); + let ret = unsafe { ioctl_abort(b2_ctl.as_raw_fd()) }; assert_eq!(ret, 0); // Parent branch still has its file assert!(b1_dir.join("parent_file.txt").exists()); // COMMIT first branch back to main - let ret = unsafe { ioctl_commit(fd, &branch1) }; + let ret = unsafe { ioctl_commit(b1_ctl.as_raw_fd()) }; assert_eq!(ret, 0); assert!(fix.base.join("parent_file.txt").exists()); From 56f25df71b9004bb8b02cde22a0190687df9f279 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Tue, 10 Feb 2026 13:03:59 -0800 Subject: [PATCH 6/7] Remove nested @parent/@child virtual dirs, use flat namespace only Signed-off-by: Cong Wang --- src/branch.rs | 9 --------- src/fs_path.rs | 5 ----- tests/test_branch_dirs.sh | 10 +++++----- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/src/branch.rs b/src/branch.rs index 3862fb2..42df28a 100644 --- a/src/branch.rs +++ b/src/branch.rs @@ -399,15 +399,6 @@ impl BranchManager { } } - /// Return the names of branches whose parent is `parent_name`. - pub fn get_children(&self, parent_name: &str) -> Vec { - self.branches - .read() - .values() - .filter(|b| b.parent.as_deref() == Some(parent_name)) - .map(|b| b.name.clone()) - .collect() - } pub fn resolve_path(&self, branch_name: &str, rel_path: &str) -> Result> { let branches = self.branches.read(); diff --git a/src/fs_path.rs b/src/fs_path.rs index ba5700e..cebc9eb 100644 --- a/src/fs_path.rs +++ b/src/fs_path.rs @@ -27,11 +27,6 @@ pub(crate) fn classify_path(path: &str) -> PathContext { let branch = &rest[..slash_pos]; let remainder = &rest[slash_pos..]; // e.g. "/.branchfs_ctl" or "/src/main.rs" - // Handle nested @child: /@parent/@child/... → recurse as /@child/... - if remainder.starts_with("/@") { - return classify_path(remainder); - } - if remainder == format!("/{}", CTL_FILE).as_str() { PathContext::BranchCtl(branch.to_string()) } else { diff --git a/tests/test_branch_dirs.sh b/tests/test_branch_dirs.sh index c777469..a1d0394 100755 --- a/tests/test_branch_dirs.sh +++ b/tests/test_branch_dirs.sh @@ -170,17 +170,17 @@ test_branch_dir_nested_child() { do_create "parent-br" "main" do_create "child-br" "parent-br" - # @child-br should appear as a top-level @branch dir + # @child-br should appear as a top-level @branch dir (flat namespace) assert "[[ -d '$TEST_MNT/@child-br' ]]" "@child-br exists at top level" - # @child-br should also appear nested under @parent-br - assert "[[ -d '$TEST_MNT/@parent-br/@child-br' ]]" "@child-br nested under @parent-br" + # Nested /@parent-br/@child-br should NOT exist (pure flat) + assert "[[ ! -d '$TEST_MNT/@parent-br/@child-br' ]]" "@child-br NOT nested under @parent-br" - # Write to child branch, verify visible both ways + # Write to child branch, verify visible via flat path echo "child content" > "$TEST_MNT/@child-br/child_file.txt" assert_file_exists "$TEST_MNT/@child-br/child_file.txt" "child_file.txt via @child-br" - # Child branch should also see parent's files + # Child branch should see parent's files (inheritance via resolve_path chain) echo "parent content" > "$TEST_MNT/@parent-br/parent_file.txt" # Switch root to main so we don't confuse things From 727a222d848b4d27d62222aeebc36267093761a1 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Tue, 10 Feb 2026 13:07:54 -0800 Subject: [PATCH 7/7] Fix cargo fmt Signed-off-by: Cong Wang --- src/branch.rs | 9 ++------- src/fs.rs | 3 +-- src/fs_ctl.rs | 8 +++++--- src/main.rs | 8 ++++++-- tests/test_ioctl.rs | 3 +-- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/branch.rs b/src/branch.rs index 42df28a..aaaf867 100644 --- a/src/branch.rs +++ b/src/branch.rs @@ -189,17 +189,13 @@ impl BranchManager { pub fn switch_mount_branch(&self, mountpoint: &Path, new_branch: &str) { // Lock order: mount_branches → notifiers (never reversed) let mut mb = self.mount_branches.write(); - let old_branch = mb - .insert(mountpoint.to_path_buf(), new_branch.to_string()); + let old_branch = mb.insert(mountpoint.to_path_buf(), new_branch.to_string()); // Re-key notifier from (old, mount) → (new, mount) if let Some(old) = old_branch { let mut notifiers = self.notifiers.lock(); if let Some(notifier) = notifiers.remove(&(old.clone(), mountpoint.to_path_buf())) { - notifiers.insert( - (new_branch.to_string(), mountpoint.to_path_buf()), - notifier, - ); + notifiers.insert((new_branch.to_string(), mountpoint.to_path_buf()), notifier); } log::info!( "switch_mount_branch: {:?} '{}' -> '{}'", @@ -399,7 +395,6 @@ impl BranchManager { } } - pub fn resolve_path(&self, branch_name: &str, rel_path: &str) -> Result> { let branches = self.branches.read(); diff --git a/src/fs.rs b/src/fs.rs index 14445df..a884d82 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -27,7 +27,7 @@ pub(crate) const BLOCK_SIZE: u32 = 512; pub const FS_IOC_BRANCH_CREATE: u32 = 0x8080_6200; // _IOR('b', 0, [u8; 128]) pub const FS_IOC_BRANCH_COMMIT: u32 = 0x0000_6201; // _IO ('b', 1) -pub const FS_IOC_BRANCH_ABORT: u32 = 0x0000_6202; // _IO ('b', 2) +pub const FS_IOC_BRANCH_ABORT: u32 = 0x0000_6202; // _IO ('b', 2) pub(crate) const CTL_FILE: &str = ".branchfs_ctl"; pub(crate) const CTL_INO: u64 = u64::MAX - 1; @@ -826,7 +826,6 @@ impl Filesystem for BranchFs { let ctl_ino = self.get_or_create_branch_ctl_ino(&branch); entries.push((ctl_ino, FileType::RegularFile, CTL_FILE.to_string())); - for (i, (e_ino, kind, name)) in entries.into_iter().enumerate().skip(offset as usize) { diff --git a/src/fs_ctl.rs b/src/fs_ctl.rs index 9f5c337..b814aee 100644 --- a/src/fs_ctl.rs +++ b/src/fs_ctl.rs @@ -58,7 +58,10 @@ impl BranchFs { log::info!("Switched to branch '{}'", new_branch); reply.written(data.len() as u32); } else { - log::warn!("Unknown root ctl command: '{}' (use /@branch/.branchfs_ctl for commit/abort)", cmd); + log::warn!( + "Unknown root ctl command: '{}' (use /@branch/.branchfs_ctl for commit/abort)", + cmd + ); reply.error(libc::EINVAL); } } @@ -88,8 +91,7 @@ impl BranchFs { // Only switch the mount if the operated branch is the current mount branch let current = self.get_branch_name(); if current == branch { - self.manager - .switch_mount_branch(&self.mountpoint, &parent); + self.manager.switch_mount_branch(&self.mountpoint, &parent); log::info!( "Branch ctl {} succeeded for '{}', switched to '{}'", cmd_lower, diff --git a/src/main.rs b/src/main.rs index 496154a..818525e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -224,7 +224,9 @@ fn main() -> Result<()> { // Write to the per-branch ctl file // (FUSE handler does commit + switch_mount_branch internally) - let ctl_path = mountpoint.join(format!("@{}", branch)).join(".branchfs_ctl"); + let ctl_path = mountpoint + .join(format!("@{}", branch)) + .join(".branchfs_ctl"); let mut file = std::fs::OpenOptions::new() .write(true) .open(&ctl_path) @@ -250,7 +252,9 @@ fn main() -> Result<()> { // Write to the per-branch ctl file // (FUSE handler does abort + switch_mount_branch internally) - let ctl_path = mountpoint.join(format!("@{}", branch)).join(".branchfs_ctl"); + let ctl_path = mountpoint + .join(format!("@{}", branch)) + .join(".branchfs_ctl"); let mut file = std::fs::OpenOptions::new() .write(true) .open(&ctl_path) diff --git a/tests/test_ioctl.rs b/tests/test_ioctl.rs index 2198f8b..e8fa120 100644 --- a/tests/test_ioctl.rs +++ b/tests/test_ioctl.rs @@ -334,8 +334,7 @@ fn test_ioctl_abort_returns_to_parent_branch() { // CREATE second (nested) branch via branch1's ctl, write another file let b1_ctl = fix.open_branch_ctl(&branch1); - let branch2 = - unsafe { ioctl_create(b1_ctl.as_raw_fd()) }.expect("CREATE should succeed"); + let branch2 = unsafe { ioctl_create(b1_ctl.as_raw_fd()) }.expect("CREATE should succeed"); let b2_dir = fix.mnt.join(format!("@{}", branch2)); fs::write(b2_dir.join("child_file.txt"), "child\n").unwrap();