Re: [PATCH] dd: work around buffer length restrictions with oflag=direct (O_DIRECT)

[Jim Meyering]

Jim Meyering wrote:
> Eric Sandeen mentioned that dd's O_DIRECT-exposing code
> didn't always work.  The problem is that the kernel imposes
...
> Here's the patch I expect to push soon:
>
>>From 2f4004be6131e049a1452ee68377ae1756673bd4 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Tue, 4 Aug 2009 19:54:58 +0200
> Subject: [PATCH] dd: work around buffer length restrictions with oflag=direct 
> (O_DIRECT)

In case anyone actually reviews this, I've fixed a few nits:

  - mention the fix in NEWS
  - s/oflags=/oflag=/

>From 5929322ccb1f9d27c1b07b746d37419d17a7cbf6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Tue, 4 Aug 2009 19:54:58 +0200
Subject: [PATCH] dd: work around buffer length restrictions with oflag=direct 
(O_DIRECT)

dd oflag=direct would fail to copy a file with size that is
not a multiple of 512 (destination file system specific)

* NEWS (Bug fixes): Mention it.
* src/dd.c (iwrite): Turn off O_DIRECT for any
smaller-than-obs-sized write.  Don't bother to restore it.
* tests/dd/direct: New test for the above.
* tests/Makefile.am (TESTS): Add dd/direct.
* doc/coreutils.texi (dd invocation): Mention oflag=direct
buffer size restriction.
Details in http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17586
Reported by Eric Sandeen.
---
 NEWS               |    3 +++
 doc/coreutils.texi |    4 ++++
 src/dd.c           |   10 +++++++++-
 tests/Makefile.am  |    1 +
 tests/dd/direct    |   40 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 57 insertions(+), 1 deletions(-)
 create mode 100755 tests/dd/direct

diff --git a/NEWS b/NEWS
index 0a0d4a7..c61f666 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@ GNU coreutils NEWS                                    -*- 
outline -*-

 ** Bug fixes

+  dd's oflag=direct option now works even when the size of the input
+  is not a multiple of e.g., 512 bytes.
+
   install runs faster again with SELinux enabled
   [introduced in coreutils-7.0]

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index acec76e..90a54b2 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7861,6 +7861,10 @@ dd invocation
 @opindex direct
 @cindex direct I/O
 Use direct I/O for data, avoiding the buffer cache.
+Note that the kernel may impose restrictions on read or write buffer sizes.
+For example, with an ext4 destination file system and a linux-based kernel,
+using @samp{oflag=direct} will cause writes to fail with @code{EINVAL} if the
+output buffer size is not a multiple of 512.

 @item directory
 @opindex directory
diff --git a/src/dd.c b/src/dd.c
index 9a9d22a..43ad718 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -837,6 +837,14 @@ iwrite (int fd, char const *buf, size_t size)
 {
   size_t total_written = 0;

+  if ((output_flags & O_DIRECT) && size < output_blocksize)
+    {
+      int old_flags = fcntl (STDOUT_FILENO, F_GETFL);
+      if (fcntl (STDOUT_FILENO, F_SETFL, old_flags & ~O_DIRECT) != 0)
+        error (0, errno, _("failed to turn off O_DIRECT: %s"),
+               quote (output_file));
+    }
+
   while (total_written < size)
     {
       ssize_t nwritten;
@@ -1897,7 +1905,7 @@ main (int argc, char **argv)
                  || S_ISDIR (stdout_stat.st_mode)
                  || S_TYPEISSHM (&stdout_stat))
                error (EXIT_FAILURE, ftruncate_errno,
-                      _("truncating at %"PRIuMAX" bytes in output file %s"),
+                   _("failed to truncate to %"PRIuMAX" bytes in output file 
%s"),
                       size, quote (output_file));
            }
        }
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ad19d02..6ad6133 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -298,6 +298,7 @@ TESTS =                                             \
   cp/src-base-dot                              \
   cp/symlink-slash                             \
   cp/thru-dangling                             \
+  dd/direct                                    \
   dd/misc                                      \
   dd/not-rewound                               \
   dd/reblock                                   \
diff --git a/tests/dd/direct b/tests/dd/direct
new file mode 100755
index 0000000..7e80bee
--- /dev/null
+++ b/tests/dd/direct
@@ -0,0 +1,40 @@
+#!/bin/sh
+# ensure that dd's oflag=direct works
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  dd --version
+fi
+
+. $srcdir/test-lib.sh
+
+truncate -s 8192 in || framework_failure
+dd if=in oflag=direct of=out 2> /dev/null \
+  || skip_test_ 'this file system lacks support for O_DIRECT'
+
+truncate -s 511 short || framework_failure
+truncate -s 8191 m1 || framework_failure
+truncate -s 8193 p1 || framework_failure
+
+fail=0
+for i in short m1 p1; do
+  rm -f out
+  dd if=$i oflag=direct of=out || fail=1
+done
+
+Exit $fail
-- 
1.6.4.115.g33d49