Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions fuse-pipe/src/protocol/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u64> {
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<u64> {
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;
}
_ => {}
Expand Down
36 changes: 30 additions & 6 deletions fuse-pipe/src/server/remap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -536,8 +536,13 @@ impl<T: FilesystemHandler> RemapFs<T> {
///
/// 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<u64, String> =
serde_json::from_str(json).unwrap_or_default();
let table: std::collections::BTreeMap<u64, String> = 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();
Expand Down Expand Up @@ -706,12 +711,19 @@ impl<T: FilesystemHandler> FilesystemHandler for RemapFs<T> {
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
Expand All @@ -725,8 +737,20 @@ impl<T: FilesystemHandler> FilesystemHandler for RemapFs<T> {
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)
Comment on lines +744 to +748
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reopen fh_out only when it is actually stale

This block unconditionally calls try_reopen_handle for fh_out on every EBADF retry, even when EBADF was caused by fh_in and fh_out is already a valid current handle (a common mixed-handle state after snapshot restore). In that case try_reopen_handle inserts a new mapping for the valid old_fh_out, so later requests are remapped to a duplicate descriptor; when Release arrives, the remapped descriptor is closed and the original valid handle is never released, causing an FD leak that can accumulate over long-running workloads.

Useful? React with 👍 / 👎.

{
retry = retry.with_fh_out(new_fh_out);
}
}

let retry_resp = self
.inner
.handle_request_with_groups(&retry, supplementary_groups);
Expand Down
Loading