From 71341fe6bb489389f2a01470658690b8e03f44c7 Mon Sep 17 00:00:00 2001 From: "claude[bot]" Date: Tue, 3 Mar 2026 23:39:01 +0000 Subject: [PATCH] fix: remap fh_in and fh_out independently for CopyFileRange/RemapFileRange MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous fix set both fh_in and fh_out to the same value in with_fh(), which corrupts fh_out when only fh_in is stale (or vice versa). Split into independent fh_out()/ino_out()/with_fh_out() helpers so the handle_remap lookup and EBADF retry paths remap each handle separately. Also log a warning when restore_from_table fails to parse inode table JSON instead of silently falling back to an empty table. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- fuse-pipe/src/protocol/request.rs | 38 +++++++++++++++++++++++++++++-- fuse-pipe/src/server/remap.rs | 36 ++++++++++++++++++++++++----- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/fuse-pipe/src/protocol/request.rs b/fuse-pipe/src/protocol/request.rs index 13336d79..fd2f37ca 100644 --- a/fuse-pipe/src/protocol/request.rs +++ b/fuse-pipe/src/protocol/request.rs @@ -457,9 +457,43 @@ impl VolumeRequest { | Self::Setlk { fh, .. } | Self::Readdirplus { fh, .. } => *fh = new_fh, Self::Setattr { fh, .. } => *fh = Some(new_fh), - Self::CopyFileRange { fh_in, fh_out, .. } - | Self::RemapFileRange { fh_in, fh_out, .. } => { + Self::CopyFileRange { fh_in, .. } | Self::RemapFileRange { fh_in, .. } => { *fh_in = new_fh; + } + _ => {} + } + cloned + } + + /// Extract the output file handle (`fh_out`) for dual-handle operations. + /// + /// Returns `Some(fh_out)` for `CopyFileRange` and `RemapFileRange`, `None` otherwise. + pub fn fh_out(&self) -> Option { + match self { + Self::CopyFileRange { fh_out, .. } | Self::RemapFileRange { fh_out, .. } => { + Some(*fh_out) + } + _ => None, + } + } + + /// Extract the output inode (`ino_out`) for dual-handle operations. + pub fn ino_out(&self) -> Option { + match self { + Self::CopyFileRange { ino_out, .. } | Self::RemapFileRange { ino_out, .. } => { + Some(*ino_out) + } + _ => None, + } + } + + /// Clone this request with a different output file handle. + /// + /// Only affects `CopyFileRange` and `RemapFileRange`; no-op for other variants. + pub fn with_fh_out(&self, new_fh: u64) -> Self { + let mut cloned = self.clone(); + match &mut cloned { + Self::CopyFileRange { fh_out, .. } | Self::RemapFileRange { fh_out, .. } => { *fh_out = new_fh; } _ => {} diff --git a/fuse-pipe/src/server/remap.rs b/fuse-pipe/src/server/remap.rs index 38df421c..85bacd3b 100644 --- a/fuse-pipe/src/server/remap.rs +++ b/fuse-pipe/src/server/remap.rs @@ -9,7 +9,7 @@ //! numbers regardless of which host they run on. use dashmap::DashMap; -use tracing::{debug, error, info}; +use tracing::{debug, error, info, warn}; use crate::protocol::{VolumeRequest, VolumeResponse}; @@ -536,8 +536,13 @@ impl RemapFs { /// /// Paths that no longer exist on the host are skipped (logged as warnings). pub fn restore_from_table(inner: T, json: &str) -> Self { - let table: std::collections::BTreeMap = - serde_json::from_str(json).unwrap_or_default(); + let table: std::collections::BTreeMap = match serde_json::from_str(json) { + Ok(t) => t, + Err(e) => { + warn!("failed to parse inode table JSON, starting with empty table: {e}"); + std::collections::BTreeMap::new() + } + }; let inner_to_stable = DashMap::new(); let stable_to_inner = DashMap::new(); @@ -706,12 +711,19 @@ impl FilesystemHandler for RemapFs { return VolumeResponse::io_error(); } - // Translate stale file handles from snapshot restore + // Translate stale file handles from snapshot restore. + // Handle fh (or fh_in for dual-handle ops) and fh_out independently + // so CopyFileRange/RemapFileRange don't corrupt the other handle. if let Some(old_fh) = remapped.fh() { if let Some(new_fh) = self.handle_remap.get(&old_fh) { remapped = remapped.with_fh(*new_fh); } } + if let Some(old_fh_out) = remapped.fh_out() { + if let Some(new_fh) = self.handle_remap.get(&old_fh_out) { + remapped = remapped.with_fh_out(*new_fh); + } + } // Delegate to inner handler let response = self @@ -725,8 +737,20 @@ impl FilesystemHandler for RemapFs { if let Some(new_fh) = self.try_reopen_handle(stable_ino, old_fh, request.is_dir_handle_op()) { - // Retry with the reopened handle - let retry = remapped.with_fh(new_fh); + // Retry with the reopened handle (fh_in for dual-handle ops) + let mut retry = remapped.with_fh(new_fh); + + // For dual-handle ops, also reopen fh_out independently + if let (Some(old_fh_out), Some(stable_ino_out)) = + (request.fh_out(), request.ino_out()) + { + if let Some(new_fh_out) = + self.try_reopen_handle(stable_ino_out, old_fh_out, false) + { + retry = retry.with_fh_out(new_fh_out); + } + } + let retry_resp = self .inner .handle_request_with_groups(&retry, supplementary_groups);