Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cp --sparse=auto not creating sparse-files. #6130

Closed
Closed
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
fd5140f
Fix clone doing standard copy on sparse=auto
AnirbanHalder654322 Mar 25, 2024
2f29570
bug fix
AnirbanHalder654322 Mar 25, 2024
08a44bb
Making code more idiomatic
AnirbanHalder654322 Mar 25, 2024
15686cd
Added handling for destination fifo files
AnirbanHalder654322 Mar 26, 2024
1c9ed4e
Trying a fix
AnirbanHalder654322 Mar 26, 2024
9b00609
Trying fix
AnirbanHalder654322 Mar 26, 2024
2c52070
Empty
AnirbanHalder654322 Mar 26, 2024
1b9135c
Switching to old behaviour for non-sparse_files
AnirbanHalder654322 Mar 26, 2024
aef3dcc
Removed a bug
AnirbanHalder654322 Mar 26, 2024
ad9a907
Removed uncessary unwrap and added comments
AnirbanHalder654322 Mar 27, 2024
0fe9c55
Merge branch 'main' into fix_clone_behaviour
AnirbanHalder654322 Mar 27, 2024
2e80b71
Added os flags
AnirbanHalder654322 Mar 27, 2024
bd09b7a
Merge branch 'fix_clone_behaviour' of https://github.com/AnirbanHalde…
AnirbanHalder654322 Mar 27, 2024
0b7ddc8
Merge branch 'main' into fix_clone_behaviour
AnirbanHalder654322 Mar 27, 2024
cee93a4
Added tests
AnirbanHalder654322 Mar 27, 2024
3c869ca
Changed tests so they don't use du
AnirbanHalder654322 Mar 28, 2024
4137db0
Fix clone doing standard copy on sparse=auto
AnirbanHalder654322 Mar 25, 2024
bc20543
bug fix
AnirbanHalder654322 Mar 25, 2024
c1b2479
Making code more idiomatic
AnirbanHalder654322 Mar 25, 2024
6b7f79f
Added handling for destination fifo files
AnirbanHalder654322 Mar 26, 2024
dddd35f
Trying a fix
AnirbanHalder654322 Mar 26, 2024
0dc5772
Trying fix
AnirbanHalder654322 Mar 26, 2024
66de26c
Empty
AnirbanHalder654322 Mar 26, 2024
c8762be
Switching to old behaviour for non-sparse_files
AnirbanHalder654322 Mar 26, 2024
c5a2df3
Removed a bug
AnirbanHalder654322 Mar 26, 2024
7fc74fa
Removed uncessary unwrap and added comments
AnirbanHalder654322 Mar 27, 2024
2e03c4c
Added os flags
AnirbanHalder654322 Mar 27, 2024
34e3a2d
Added tests
AnirbanHalder654322 Mar 27, 2024
a82de20
Changed tests so they don't use du
AnirbanHalder654322 Mar 28, 2024
66552df
Fixed a bug where files without any data and disk allocation wouldn't…
AnirbanHalder654322 Mar 31, 2024
dd402b4
Merge branch 'uutils:main' into fix_clone_behaviour
AnirbanHalder654322 Mar 31, 2024
6a227b7
Fixing conflict
AnirbanHalder654322 Mar 31, 2024
4a2228c
Merge branch 'fix_clone_behaviour' of https://github.com/AnirbanHalde…
AnirbanHalder654322 Mar 31, 2024
b9bf4c1
Merge branch 'uutils:main' into fix_clone_behaviour
AnirbanHalder654322 Mar 31, 2024
211002e
simplified logic and added suggestions
AnirbanHalder654322 Mar 31, 2024
65213d4
Lint change
AnirbanHalder654322 Mar 31, 2024
7a14d33
Apply suggestions from code review
AnirbanHalder654322 Apr 1, 2024
d48b98f
Added testing for virtual files
AnirbanHalder654322 Apr 1, 2024
89382ef
Merge branch 'fix_clone_behaviour' of https://github.com/AnirbanHalde…
AnirbanHalder654322 Apr 1, 2024
d219243
simplified logic and added suggestions
AnirbanHalder654322 Mar 31, 2024
9503a26
Added testing for virtual files
AnirbanHalder654322 Apr 1, 2024
450c6ea
Merge conf
AnirbanHalder654322 Apr 1, 2024
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
80 changes: 74 additions & 6 deletions src/uu/cp/src/platform/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore ficlone reflink ftruncate pwrite fiemap
use std::fs::{File, OpenOptions};
use std::fs::{self, File, OpenOptions};
use std::io::Read;
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::fs::{FileTypeExt, OpenOptionsExt};
use std::os::unix::io::AsRawFd;
use std::os::unix::prelude::MetadataExt;
use std::path::Path;

use quick_error::ResultExt;
Expand All @@ -32,6 +33,9 @@

/// Use [`std::fs::copy`].
FSCopy,

/// Use sparse_copy
SparseCopy,
}

/// Use the Linux `ioctl_ficlone` API to do a copy-on-write clone.
Expand All @@ -53,6 +57,7 @@
match fallback {
CloneFallback::Error => Err(std::io::Error::last_os_error()),
CloneFallback::FSCopy => std::fs::copy(source, dest).map(|_| ()),
CloneFallback::SparseCopy => sparse_copy(source, dest),

Check warning on line 60 in src/uu/cp/src/platform/linux.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/cp/src/platform/linux.rs#L60

Added line #L60 was not covered by tests
}
}

Expand All @@ -62,8 +67,6 @@
where
P: AsRef<Path>,
{
use std::os::unix::prelude::MetadataExt;

let mut src_file = File::open(source)?;
let dst_file = File::create(dest)?;
let dst_fd = dst_file.as_raw_fd();
Expand Down Expand Up @@ -132,6 +135,48 @@
Ok(num_bytes_copied)
}

#[cfg(any(target_os = "linux", target_os = "android"))]
fn check_dest_is_fifo(dest: &Path) -> bool {
//If our destination file exists and its a fifo , we do a standard copy .
AnirbanHalder654322 marked this conversation as resolved.
Show resolved Hide resolved
let file_type = fs::metadata(dest);
match file_type {
Ok(f) => f.file_type().is_fifo(),

_ => false,
}
}

#[cfg(any(target_os = "linux", target_os = "android"))]
fn check_sparse_detection(source: &Path) -> Result<bool, std::io::Error> {
let mut src_file = File::open(source)?;
AnirbanHalder654322 marked this conversation as resolved.
Show resolved Hide resolved

let metadata = src_file.metadata()?;
let size = metadata.size();
let blocks = metadata.blocks();
let blksize = metadata.blksize();
// cp uses a crude heuristic to detect sparse_files, an estimated formula which seems to accurately
//replicate that, is used. Modifications might be required if we observe any edges cases but this
//should work for majority of the files.
//Reference:https://doc.rust-lang.org/std/os/unix/fs/trait.MetadataExt.html#tymethod.blocks

let mut buf = vec![0; blksize as usize];

// Reading the file to account for virtual files which have 0 blocks allocated in disk
//and it is unknown if it contains data or not, virtual files need to be standard copied and
//categorized as not sparse.
AnirbanHalder654322 marked this conversation as resolved.
Show resolved Hide resolved

if blocks == 0 {
let _ = src_file.read(&mut buf)?;

if buf.iter().any(|&x| x != 0x0) {
return Ok(false);
}
} else if blocks < size / 512 {
return Ok(true);
}
Ok(false)
}

/// Copies `source` to `dest` using copy-on-write if possible.
///
/// The `source_is_fifo` flag must be set to `true` if and only if
Expand Down Expand Up @@ -159,19 +204,42 @@
copy_debug.reflink = OffloadReflinkDebug::No;
sparse_copy(source, dest)
}
(ReflinkMode::Never, _) => {
(ReflinkMode::Never, SparseMode::Never) => {
copy_debug.sparse_detection = SparseDebug::No;
copy_debug.reflink = OffloadReflinkDebug::No;
std::fs::copy(source, dest).map(|_| ())
}
(ReflinkMode::Never, SparseMode::Auto) => {
copy_debug.sparse_detection = SparseDebug::No;
copy_debug.reflink = OffloadReflinkDebug::No;
match check_sparse_detection(source) {
Ok(true) => sparse_copy(source, dest),
Ok(false) => std::fs::copy(source, dest).map(|_| ()),
Err(e) => Err(e),

Check warning on line 218 in src/uu/cp/src/platform/linux.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/cp/src/platform/linux.rs#L218

Added line #L218 was not covered by tests
}
BenWiederhake marked this conversation as resolved.
Show resolved Hide resolved
}
(ReflinkMode::Auto, SparseMode::Always) => {
copy_debug.offload = OffloadReflinkDebug::Avoided;
copy_debug.sparse_detection = SparseDebug::Zeros;
copy_debug.reflink = OffloadReflinkDebug::Unsupported;
sparse_copy(source, dest)
}

(ReflinkMode::Auto, _) => {
(ReflinkMode::Auto, SparseMode::Auto) => {
copy_debug.sparse_detection = SparseDebug::No;
copy_debug.reflink = OffloadReflinkDebug::Unsupported;
if source_is_fifo {
copy_fifo_contents(source, dest).map(|_| ())
} else {
match (check_sparse_detection(source), check_dest_is_fifo(dest)) {
(Ok(true), false) => clone(source, dest, CloneFallback::SparseCopy),
(Ok(true), true) => clone(source, dest, CloneFallback::FSCopy),

Check warning on line 236 in src/uu/cp/src/platform/linux.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/cp/src/platform/linux.rs#L235-L236

Added lines #L235 - L236 were not covered by tests
(Ok(false), _) => clone(source, dest, CloneFallback::FSCopy),
(Err(e), _) => Err(e),
BenWiederhake marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
(ReflinkMode::Auto, SparseMode::Never) => {
copy_debug.sparse_detection = SparseDebug::No;
copy_debug.reflink = OffloadReflinkDebug::Unsupported;
if source_is_fifo {
Expand Down
59 changes: 59 additions & 0 deletions tests/by-util/test_cp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3218,6 +3218,65 @@ fn test_copy_contents_fifo() {
assert_eq!(at.read("outfile"), "foo");
}

#[cfg(any(target_os = "linux", target_os = "android"))]
#[test]
fn test_sparse_auto_for_sparse_files() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;
at.touch("a");
at.write("a", "hello");
at.truncate("a", "-s10000");
ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds();

let src_size = std::fs::metadata(at.plus("a"))
.expect("Metadata of source file cannot be read")
.size();

let src_blocks = std::fs::metadata(at.plus("a"))
.expect("Metadata of source file cannot be read")
.blocks();

let dest_size = std::fs::metadata(at.plus("b"))
.expect("Metadata of copied file cannot be read")
.size();
let dest_blocks = std::fs::metadata(at.plus("b"))
.expect("Metadata of copied file cannot be read")
.blocks();

assert_eq!(src_size, dest_size);
assert_eq!(src_blocks, dest_blocks);
}

#[cfg(any(target_os = "linux", target_os = "android"))]
#[test]
fn test_sparse_auto_for_non_sparse_files() {
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;
let a_bytes = [1 as u8; 10000];
at.touch("a");
at.write("a", "hello");
at.append_bytes("a", &a_bytes);
ts.ucmd().arg("--sparse=auto").arg("a").arg("b").succeeds();

let src_size = std::fs::metadata(at.plus("a"))
.expect("Metadata of source file cannot be read")
.size();

let src_blocks = std::fs::metadata(at.plus("a"))
.expect("Metadata of source file cannot be read")
.blocks();

let dest_size = std::fs::metadata(at.plus("b"))
.expect("Metadata of copied file cannot be read")
.size();
let dest_blocks = std::fs::metadata(at.plus("b"))
.expect("Metadata of copied file cannot be read")
.blocks();

assert_eq!(src_size, dest_size);
assert_eq!(src_blocks, dest_blocks);
}

#[cfg(target_os = "linux")]
#[test]
fn test_reflink_never_sparse_always() {
Expand Down
Loading