Skip to content

Commit c77b060

Browse files
committed
Undo 7ce73a4 as it broke recursive copy detection for multiple directories
Example: $ ^mkdir -p a/b/c $ cpz a a/b a/b/c/ If you copy only one of the directories it works, but not both since we pick a global inode for all copies instead of one per root task. Note that the example above is still totally broken because you're modifying the directory in parallel with the copy of itself. Separately, you can still break things by creating a symlink that points to its parent. Solving both these problems requires building the file tree in-memory which isn't worth it. Signed-off-by: Alex Saveau <[email protected]>
1 parent ebbfa8a commit c77b060

File tree

1 file changed

+37
-58
lines changed

1 file changed

+37
-58
lines changed

fuc_engine/src/ops/copy.rs

Lines changed: 37 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,18 @@ mod compat {
220220
{
221221
#[cfg_attr(feature = "tracing", tracing::instrument(level = "trace", skip(self)))]
222222
fn run(&self, (from, to): (Cow<Path>, Cow<Path>)) -> Result<(), Error> {
223+
let root_to_inode = {
224+
let to_metadata = statx(CWD, &*to, AtFlags::SYMLINK_NOFOLLOW, StatxFlags::INO)
225+
.map_io_err(|| format!("Failed to stat directory: {to:?}"))?;
226+
to_metadata.stx_ino
227+
};
228+
223229
let (tasks, _) = &*self.scheduling;
224230
tasks
225231
.send(TreeNode {
226232
from: path_buf_to_cstring(from.into_owned())?,
227233
to: path_buf_to_cstring(to.into_owned())?,
234+
root_to_inode,
228235
messages: tasks.clone(),
229236
})
230237
.map_err(|_| Error::Internal)
@@ -265,26 +272,9 @@ mod compat {
265272
let mut threads = Vec::with_capacity(available_parallelism);
266273

267274
{
268-
let mut root_to_inode = None;
269275
let mut buf = [MaybeUninit::<u8>::uninit(); 8192];
270276
let symlink_buf_cache = Cell::new(Vec::new());
271277
for node in &tasks {
272-
let root_to_inode = if let Some(root_to_inode) = root_to_inode {
273-
root_to_inode
274-
} else {
275-
let to_dir = openat(
276-
CWD,
277-
&node.to,
278-
OFlags::RDONLY | OFlags::DIRECTORY | OFlags::PATH,
279-
Mode::empty(),
280-
)
281-
.map_io_err(|| format!("Failed to open directory: {:?}", node.to))?;
282-
let to_metadata = statx(to_dir, c"", AtFlags::EMPTY_PATH, StatxFlags::INO)
283-
.map_io_err(|| format!("Failed to stat directory: {:?}", node.to))?;
284-
root_to_inode = Some(to_metadata.stx_ino);
285-
to_metadata.stx_ino
286-
};
287-
288278
let mut maybe_spawn = || {
289279
if available_parallelism > 0 && !tasks.is_empty() {
290280
#[cfg(feature = "tracing")]
@@ -297,21 +287,14 @@ mod compat {
297287
available_parallelism -= 1;
298288
threads.push(scope.spawn({
299289
let tasks = tasks.clone();
300-
move || {
301-
worker_thread::<HARD_LINK>(
302-
tasks,
303-
root_to_inode,
304-
follow_symlinks,
305-
)
306-
}
290+
move || worker_thread::<HARD_LINK>(tasks, follow_symlinks)
307291
}));
308292
}
309293
};
310294
maybe_spawn();
311295

312296
copy_dir::<HARD_LINK>(
313297
node,
314-
root_to_inode,
315298
follow_symlinks,
316299
&mut buf,
317300
&symlink_buf_cache,
@@ -330,22 +313,14 @@ mod compat {
330313
#[cfg_attr(feature = "tracing", tracing::instrument(level = "trace", skip(tasks)))]
331314
fn worker_thread<const HARD_LINK: bool>(
332315
tasks: Receiver<TreeNode>,
333-
root_to_inode: u64,
334316
follow_symlinks: bool,
335317
) -> Result<(), Error> {
336318
unshare_files()?;
337319

338320
let mut buf = [MaybeUninit::<u8>::uninit(); 8192];
339321
let symlink_buf_cache = Cell::new(Vec::new());
340322
for node in tasks {
341-
copy_dir::<HARD_LINK>(
342-
node,
343-
root_to_inode,
344-
follow_symlinks,
345-
&mut buf,
346-
&symlink_buf_cache,
347-
|| {},
348-
)?;
323+
copy_dir::<HARD_LINK>(node, follow_symlinks, &mut buf, &symlink_buf_cache, || {})?;
349324
}
350325
Ok(())
351326
}
@@ -378,8 +353,12 @@ mod compat {
378353
tracing::instrument(level = "info", skip(messages, buf, symlink_buf_cache, maybe_spawn))
379354
)]
380355
fn copy_dir<const HARD_LINK: bool>(
381-
TreeNode { from, to, messages }: TreeNode,
382-
root_to_inode: u64,
356+
TreeNode {
357+
from,
358+
to,
359+
root_to_inode,
360+
messages,
361+
}: TreeNode,
383362
follow_symlinks: bool,
384363
buf: &mut [MaybeUninit<u8>],
385364
symlink_buf_cache: &Cell<Vec<u8>>,
@@ -437,6 +416,7 @@ mod compat {
437416
.send(TreeNode {
438417
from,
439418
to,
419+
root_to_inode,
440420
messages: messages.clone(),
441421
})
442422
.map_err(|_| Error::Internal)?;
@@ -678,6 +658,7 @@ mod compat {
678658
struct TreeNode {
679659
from: CString,
680660
to: CString,
661+
root_to_inode: u64,
681662
messages: Sender<TreeNode>,
682663
}
683664

@@ -686,6 +667,7 @@ mod compat {
686667
f.debug_struct("TreeNode")
687668
.field("from", &self.from)
688669
.field("to", &self.to)
670+
.field("root_to_inode", &self.root_to_inode)
689671
.finish_non_exhaustive()
690672
}
691673
}
@@ -720,8 +702,22 @@ mod compat {
720702
impl DirectoryOp<(Cow<'_, Path>, Cow<'_, Path>)> for Impl {
721703
#[cfg_attr(feature = "tracing", tracing::instrument(level = "trace", skip(self)))]
722704
fn run(&self, (from, to): (Cow<Path>, Cow<Path>)) -> Result<(), Error> {
723-
copy_dir(&from, to, self.follow_symlinks, self.hard_link, None)
724-
.map_io_err(|| format!("Failed to copy directory: {from:?}"))
705+
#[cfg(unix)]
706+
let root_to_inode = {
707+
use std::os::unix::fs::MetadataExt;
708+
fs::metadata(&*to)
709+
.map_io_err(|| format!("Failed to get inode: {to:?}"))?
710+
.ino()
711+
};
712+
copy_dir(
713+
&from,
714+
to,
715+
self.follow_symlinks,
716+
self.hard_link,
717+
#[cfg(unix)]
718+
root_to_inode,
719+
)
720+
.map_io_err(|| format!("Failed to copy directory: {from:?}"))
725721
}
726722

727723
#[cfg_attr(feature = "tracing", tracing::instrument(level = "trace", skip(self)))]
@@ -736,17 +732,13 @@ mod compat {
736732
to: Q,
737733
follow_symlinks: bool,
738734
hard_link: bool,
739-
root_to_inode: Option<u64>,
735+
#[cfg(unix)] root_to_inode: u64,
740736
) -> Result<(), io::Error> {
741737
let to = to.as_ref();
742738
match fs::create_dir(to) {
743739
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {}
744740
r => r?,
745741
}
746-
#[cfg(unix)]
747-
let root_to_inode = Some(maybe_compute_root_to_inode(to, root_to_inode)?);
748-
#[cfg(not(unix))]
749-
let _ = root_to_inode;
750742

751743
from.as_ref()
752744
.read_dir()?
@@ -757,7 +749,7 @@ mod compat {
757749
#[cfg(unix)]
758750
{
759751
use std::os::unix::fs::DirEntryExt;
760-
if Some(dir_entry.ino()) == root_to_inode {
752+
if dir_entry.ino() == root_to_inode {
761753
return Ok(());
762754
}
763755
}
@@ -776,6 +768,7 @@ mod compat {
776768
to,
777769
follow_symlinks,
778770
hard_link,
771+
#[cfg(unix)]
779772
root_to_inode,
780773
)?;
781774
} else if file_type.is_symlink() {
@@ -801,18 +794,4 @@ mod compat {
801794
Ok(())
802795
})
803796
}
804-
805-
#[cfg(unix)]
806-
#[cfg_attr(feature = "tracing", tracing::instrument(level = "trace"))]
807-
fn maybe_compute_root_to_inode<P: AsRef<Path> + Debug>(
808-
to: P,
809-
root_to_inode: Option<u64>,
810-
) -> Result<u64, io::Error> {
811-
Ok(if let Some(ino) = root_to_inode {
812-
ino
813-
} else {
814-
use std::os::unix::fs::MetadataExt;
815-
fs::metadata(to)?.ino()
816-
})
817-
}
818797
}

0 commit comments

Comments
 (0)