Skip to content

Commit f3b9fb2

Browse files
[scorpio]:fix issue #1534 where a temporary mount point was incorrectly created when the normal mount at the dicfuse path was not loaded or did not exist. (#1900)
Signed-off-by: Luxian <lux1an@qq.com>
1 parent edbd7a7 commit f3b9fb2

4 files changed

Lines changed: 390 additions & 27 deletions

File tree

scorpio/src/daemon/mod.rs

Lines changed: 150 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,64 @@ struct ScoState {
107107
manager: Arc<Mutex<ScorpioManager>>, // Shared workspace manager
108108
tasks: Arc<DashMap<String, MountStatus>>, // Thread-safe storage for async mount tasks
109109
}
110+
111+
/// Resolve a mount request to an inode and whether it should be treated as a temporary mount.
112+
///
113+
/// - If the path exists, returns its inode and `temp_mount=false`.
114+
/// - If the path doesn't exist and this is a temp mount request (buck2), creates a temp point.
115+
/// - If the path doesn't exist and this is a normal mount, returns a descriptive error.
116+
async fn resolve_mount_inode(
117+
state: &ScoState,
118+
req: &MountRequest,
119+
mono_path: &str,
120+
) -> Result<(u64, bool), String> {
121+
let temp_request = req.cl.is_none();
122+
123+
match state.fuse.get_inode(mono_path).await {
124+
Ok(inode) => Ok((inode, false)),
125+
Err(_) => {
126+
if temp_request {
127+
let inode = match state.fuse.dic.store.add_temp_point(mono_path).await {
128+
Ok(inode) => inode,
129+
Err(e) => {
130+
if e.kind() == std::io::ErrorKind::NotFound {
131+
if let Ok(crate::dicfuse::store::PathLookupStatus::ParentNotLoaded {
132+
parent_path,
133+
}) = state.fuse.dic.store.lookup_path_status(mono_path).await
134+
{
135+
return Err(format!(
136+
"Temp mount parent directory not loaded in dicfuse: {mono_path} (parent: {parent_path})"
137+
));
138+
}
139+
}
140+
return Err(format!("Failed to add temp point for {mono_path}: {e}"));
141+
}
142+
};
143+
return Ok((inode, true));
144+
}
145+
146+
let status = state
147+
.fuse
148+
.dic
149+
.store
150+
.lookup_path_status(mono_path)
151+
.await
152+
.map_err(|e| format!("Mount path lookup failed in dicfuse: {mono_path}: {e}"))?;
153+
154+
match status {
155+
crate::dicfuse::store::PathLookupStatus::Found(inode) => Ok((inode, false)),
156+
crate::dicfuse::store::PathLookupStatus::ParentNotLoaded { parent_path } => Err(
157+
format!(
158+
"Mount parent directory not loaded in dicfuse: {mono_path} (parent: {parent_path})"
159+
),
160+
),
161+
crate::dicfuse::store::PathLookupStatus::NotFound => Err(format!(
162+
"Mount path not found in dicfuse: {mono_path}"
163+
)),
164+
}
165+
}
166+
}
167+
}
110168
#[allow(unused)]
111169
pub async fn daemon_main(
112170
fuse: Arc<MegaFuse>,
@@ -230,27 +288,8 @@ async fn perform_mount_task(state: ScoState, req: MountRequest) -> Result<MountI
230288
GPath::from(req.path.clone()).to_string()
231289
};
232290

233-
// Determine if this is a temporary mount for buck2 workflow
234-
let mut temp_mount = false;
235-
236-
// Try to get existing inode, or create temporary point if path doesn't exist
237-
let inode = match state.fuse.get_inode(&mono_path).await {
238-
Ok(a) => a,
239-
Err(_) => {
240-
// If there is no CL (change list) specified, this is considered a "temporary mount"
241-
// (e.g., for buck2 workflows or ephemeral mounts). For CL mounts, we do not set
242-
// temp_mount, as those are expected to be persistent or managed differently.
243-
// The temp_mount flag is used later to determine cleanup and lifecycle behavior.
244-
temp_mount = req.cl.is_none();
245-
state
246-
.fuse
247-
.dic
248-
.store
249-
.add_temp_point(&mono_path)
250-
.await
251-
.map_err(|e| format!("Failed to add temp point: {e}"))?
252-
}
253-
};
291+
// Resolve inode and determine temp mount behavior
292+
let (inode, temp_mount) = resolve_mount_inode(&state, &req, &mono_path).await?;
254293

255294
// Check if the target is already mounted to prevent conflicts
256295
if state.fuse.is_mount(inode).await {
@@ -467,3 +506,93 @@ async fn update_config_handler(
467506
config: config_info,
468507
})
469508
}
509+
510+
#[cfg(test)]
511+
mod tests {
512+
use tempfile::tempdir;
513+
514+
use super::*;
515+
use crate::dicfuse::Dicfuse;
516+
517+
async fn make_state() -> ScoState {
518+
let tmp = tempdir().unwrap();
519+
let dic = Dicfuse::new_with_store_path(tmp.path().to_str().unwrap()).await;
520+
let mut fuse = MegaFuse::new().await;
521+
fuse.dic = Arc::new(dic);
522+
523+
ScoState {
524+
fuse: Arc::new(fuse),
525+
manager: Arc::new(Mutex::new(ScorpioManager { works: vec![] })),
526+
tasks: Arc::new(DashMap::new()),
527+
}
528+
}
529+
530+
#[tokio::test]
531+
async fn test_resolve_mount_inode_found_path() {
532+
let state = make_state().await;
533+
state.fuse.dic.store.insert_mock_item(1, 0, "", true).await;
534+
state
535+
.fuse
536+
.dic
537+
.store
538+
.insert_mock_item(2, 1, "repo", true)
539+
.await;
540+
541+
let req = MountRequest {
542+
path: "/repo".to_string(),
543+
cl: None,
544+
};
545+
let (inode, temp_mount) = resolve_mount_inode(&state, &req, "repo").await.unwrap();
546+
assert_eq!(inode, 2);
547+
assert!(!temp_mount);
548+
}
549+
550+
#[tokio::test]
551+
async fn test_resolve_mount_inode_temp_mount() {
552+
let state = make_state().await;
553+
state.fuse.dic.store.insert_mock_item(1, 0, "", true).await;
554+
state
555+
.fuse
556+
.dic
557+
.store
558+
.insert_mock_item(2, 1, "repo", true)
559+
.await;
560+
561+
let req = MountRequest {
562+
path: "/repo/tmp".to_string(),
563+
cl: None,
564+
};
565+
let (inode, temp_mount) = resolve_mount_inode(&state, &req, "repo/tmp").await.unwrap();
566+
assert!(temp_mount);
567+
let inode_check = state
568+
.fuse
569+
.dic
570+
.store
571+
.get_inode_from_path("repo/tmp")
572+
.await
573+
.unwrap();
574+
assert_eq!(inode, inode_check);
575+
}
576+
577+
#[tokio::test]
578+
async fn test_resolve_mount_inode_not_found_normal_mount() {
579+
let state = make_state().await;
580+
state.fuse.dic.store.insert_mock_item(1, 0, "", true).await;
581+
state
582+
.fuse
583+
.dic
584+
.store
585+
.insert_mock_item(2, 1, "repo", true)
586+
.await;
587+
588+
let req = MountRequest {
589+
path: "/repo/missing".to_string(),
590+
cl: Some("cl123".to_string()),
591+
};
592+
let err = resolve_mount_inode(&state, &req, "repo/missing_cl123")
593+
.await
594+
.unwrap_err();
595+
assert!(err.contains("Mount path not found in dicfuse"));
596+
assert!(err.contains("repo/missing_cl123"));
597+
}
598+
}

0 commit comments

Comments
 (0)