Merge pull request #9016 from naoNao89/fix/dd-oflag-direct-isolated

fix(dd): handle O_DIRECT partial block writes
This commit is contained in:
Sylvestre Ledru
2025-10-27 23:31:53 +01:00
committed by GitHub
2 changed files with 113 additions and 1 deletions
+63 -1
View File
@@ -686,6 +686,54 @@ fn is_sparse(buf: &[u8]) -> bool {
buf.iter().all(|&e| e == 0u8)
}
/// Handle O_DIRECT write errors by temporarily removing the flag and retrying.
/// This follows GNU dd behavior for partial block writes with O_DIRECT.
#[cfg(any(target_os = "linux", target_os = "android"))]
fn handle_o_direct_write(f: &mut File, buf: &[u8], original_error: io::Error) -> io::Result<usize> {
use nix::fcntl::{FcntlArg, OFlag, fcntl};
// Get current flags using nix
let oflags = match fcntl(&mut *f, FcntlArg::F_GETFL) {
Ok(flags) => OFlag::from_bits_retain(flags),
Err(_) => return Err(original_error),
};
// If O_DIRECT is set, try removing it temporarily
if oflags.contains(OFlag::O_DIRECT) {
let flags_without_direct = oflags - OFlag::O_DIRECT;
// Remove O_DIRECT flag using nix
if fcntl(&mut *f, FcntlArg::F_SETFL(flags_without_direct)).is_err() {
return Err(original_error);
}
// Retry the write without O_DIRECT
let write_result = f.write(buf);
// Restore O_DIRECT flag using nix (GNU doesn't restore it, but we'll be safer)
// Log any restoration errors without failing the operation
if let Err(os_err) = fcntl(&mut *f, FcntlArg::F_SETFL(oflags)) {
// Just log the error, don't fail the whole operation
show_error!("Failed to restore O_DIRECT flag: {}", os_err);
}
write_result
} else {
// O_DIRECT wasn't set, return original error
Err(original_error)
}
}
/// Stub for non-Linux platforms - just return the original error.
#[cfg(not(any(target_os = "linux", target_os = "android")))]
fn handle_o_direct_write(
_f: &mut File,
_buf: &[u8],
original_error: io::Error,
) -> io::Result<usize> {
Err(original_error)
}
impl Write for Dest {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match self {
@@ -697,7 +745,21 @@ impl Write for Dest {
f.seek(SeekFrom::Current(seek_amt))?;
Ok(buf.len())
}
Self::File(f, _) => f.write(buf),
Self::File(f, _) => {
// Try the write first
match f.write(buf) {
Ok(len) => Ok(len),
Err(e)
if e.kind() == io::ErrorKind::InvalidInput
&& e.raw_os_error() == Some(libc::EINVAL) =>
{
// This might be an O_DIRECT alignment issue.
// Try removing O_DIRECT temporarily and retry.
handle_o_direct_write(f, buf, e)
}
Err(e) => Err(e),
}
}
Self::Stdout(stdout) => stdout.write(buf),
#[cfg(unix)]
Self::Fifo(f) => f.write(buf),
+50
View File
@@ -1780,3 +1780,53 @@ fn test_wrong_number_err_msg() {
.fails()
.stderr_contains("dd: invalid number: '1kBb555'\n");
}
#[test]
#[cfg(any(target_os = "linux", target_os = "android"))]
fn test_oflag_direct_partial_block() {
// Test for issue #9003: dd should handle partial blocks with oflag=direct
// This reproduces the scenario where writing a partial block with O_DIRECT fails
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;
// Create input file with size that's not a multiple of block size
// This will trigger the partial block write issue
let input_file = "test_direct_input.iso";
let output_file = "test_direct_output.img";
let block_size = 8192; // 8K blocks
let input_size = block_size * 3 + 511; // 3 full blocks + 511 byte partial block
// Create test input file with known pattern
let input_data = vec![0x42; input_size]; // Use non-zero pattern for better verification
at.write_bytes(input_file, &input_data);
// Get full paths for the dd command
let input_path = at.plus(input_file);
let output_path = at.plus(output_file);
// Test with oflag=direct - should succeed with the fix
new_ucmd!()
.args(&[
format!("if={}", input_path.display()),
format!("of={}", output_path.display()),
"oflag=direct".to_string(),
format!("bs={block_size}"),
"status=none".to_string(),
])
.succeeds()
.stdout_is("")
.stderr_is("");
assert!(output_path.exists());
let output_size = output_path.metadata().unwrap().len() as usize;
assert_eq!(output_size, input_size);
// Verify content matches input
let output_content = std::fs::read(&output_path).unwrap();
assert_eq!(output_content.len(), input_size);
assert_eq!(output_content, input_data);
// Clean up
at.remove(input_file);
at.remove(output_file);
}