Skip to content

Commit 2570717

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 <saveau.alexandre@gmail.com>
1 parent ebbfa8a commit 2570717

File tree

1 file changed

+38
-56
lines changed

1 file changed

+38
-56
lines changed

fuc_engine/src/ops/copy.rs

Lines changed: 38 additions & 56 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,24 @@ 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+
// TODO get rid of this crap once https://github.com/tokio-rs/tracing/issues/3320 is fixed
713+
#[cfg(not(unix))]
714+
let root_to_inode = 0;
715+
copy_dir(
716+
&from,
717+
to,
718+
self.follow_symlinks,
719+
self.hard_link,
720+
root_to_inode,
721+
)
722+
.map_io_err(|| format!("Failed to copy directory: {from:?}"))
725723
}
726724

727725
#[cfg_attr(feature = "tracing", tracing::instrument(level = "trace", skip(self)))]
@@ -736,15 +734,13 @@ mod compat {
736734
to: Q,
737735
follow_symlinks: bool,
738736
hard_link: bool,
739-
root_to_inode: Option<u64>,
737+
root_to_inode: u64,
740738
) -> Result<(), io::Error> {
741739
let to = to.as_ref();
742740
match fs::create_dir(to) {
743741
Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {}
744742
r => r?,
745743
}
746-
#[cfg(unix)]
747-
let root_to_inode = Some(maybe_compute_root_to_inode(to, root_to_inode)?);
748744
#[cfg(not(unix))]
749745
let _ = root_to_inode;
750746

@@ -757,7 +753,7 @@ mod compat {
757753
#[cfg(unix)]
758754
{
759755
use std::os::unix::fs::DirEntryExt;
760-
if Some(dir_entry.ino()) == root_to_inode {
756+
if dir_entry.ino() == root_to_inode {
761757
return Ok(());
762758
}
763759
}
@@ -801,18 +797,4 @@ mod compat {
801797
Ok(())
802798
})
803799
}
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-
}
818800
}

0 commit comments

Comments
 (0)