[Qemu-devel] [PATCH 0/2] raw-posix: fix O_DIRECT short reads

[Qemu-devel] [PATCH 0/2] raw-posix: fix O_DIRECT short reads

[Stefan Hajnoczi]

This series fixes qemu-iotests ./check -nocache -vmdk 059.  It also adds a
dedicated test case for O_DIRECT short reads.

Stefan Hajnoczi (2):
  raw-posix: fix O_DIRECT short reads
  qemu-iotests: add test case 101 for short file I/O

 block/raw-posix.c          |  9 +++++++
 tests/qemu-iotests/101     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/101.out | 10 ++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 78 insertions(+)
 create mode 100755 tests/qemu-iotests/101
 create mode 100644 tests/qemu-iotests/101.out

-- 
1.9.3

[Qemu-devel] [PATCH 1/2] raw-posix: fix O_DIRECT short reads

[Stefan Hajnoczi]

The following O_DIRECT read from a <512 byte file fails:

  $ truncate -s 320 test.img
  $ qemu-io -n -c 'read -P 0 0 512' test.img
  qemu-io: can't open device test.img: Could not read image for determining its format: Invalid argument

Note that qemu-io completes successfully without the -n (O_DIRECT)
option.

This patch fixes qemu-iotests ./check -nocache -vmdk 059.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-posix.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 1194eb0..a48ce91 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -747,6 +747,15 @@ static ssize_t handle_aiocb_rw_linear(RawPosixAIOData *aiocb, char *buf)
         }
         if (len == -1 && errno == EINTR) {
             continue;
+        } else if (len == -1 && errno == EINVAL &&
+                   (aiocb->bs->open_flags & BDRV_O_NOCACHE) &&
+                   !(aiocb->aio_type & QEMU_AIO_WRITE) &&
+                   offset > 0) {
+            /* O_DIRECT pread() may fail with EINVAL when offset is unaligned
+             * after a short read.  Assume that O_DIRECT short reads only occur
+             * at EOF.  Therefore this is a short read, not an I/O error.
+             */
+            break;
         } else if (len == -1) {
             offset = -errno;
             break;
-- 
1.9.3

[Qemu-devel] [PATCH 2/2] qemu-iotests: add test case 101 for short file I/O

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/101     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/101.out | 10 ++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 69 insertions(+)
 create mode 100755 tests/qemu-iotests/101
 create mode 100644 tests/qemu-iotests/101.out

diff --git a/tests/qemu-iotests/101 b/tests/qemu-iotests/101
new file mode 100755
index 0000000..70fbf25
--- /dev/null
+++ b/tests/qemu-iotests/101
@@ -0,0 +1,58 @@
+#!/bin/bash
+#
+# Test short file I/O
+#
+# Copyright (C) 2014 Red Hat, 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 2 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/>.
+#
+
+# creator
+owner=stefanha@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+
+echo
+echo "== creating short image file =="
+dd if=/dev/zero of="$TEST_IMG" bs=1 count=320
+
+echo
+echo "== reading bytes beyond EOF gives zeroes =="
+$QEMU_IO -c "read -P 0 0 512" "$TEST_IMG" | _filter_qemu_io
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/101.out b/tests/qemu-iotests/101.out
new file mode 100644
index 0000000..9a996e8
--- /dev/null
+++ b/tests/qemu-iotests/101.out
@@ -0,0 +1,10 @@
+QA output created by 101
+
+== creating short image file ==
+320+0 records in
+320+0 records out
+
+== reading bytes beyond EOF gives zeroes ==
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 6e67f61..e25e992 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -100,3 +100,4 @@
 091 rw auto quick
 092 rw auto quick
 095 rw auto quick
+101 rw auto quick
-- 
1.9.3

Re: [Qemu-devel] [PATCH 1/2] raw-posix: fix O_DIRECT short reads

[Benoît Canet]

The Thursday 21 Aug 2014 à 13:44:07 (+0100), Stefan Hajnoczi wrote :
> The following O_DIRECT read from a <512 byte file fails:
> 
>   $ truncate -s 320 test.img
>   $ qemu-io -n -c 'read -P 0 0 512' test.img
>   qemu-io: can't open device test.img: Could not read image for determining its format: Invalid argument
> 
> Note that qemu-io completes successfully without the -n (O_DIRECT)
> option.
> 
> This patch fixes qemu-iotests ./check -nocache -vmdk 059.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Reported-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/raw-posix.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 1194eb0..a48ce91 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -747,6 +747,15 @@ static ssize_t handle_aiocb_rw_linear(RawPosixAIOData *aiocb, char *buf)
>          }
>          if (len == -1 && errno == EINTR) {
>              continue;
> +        } else if (len == -1 && errno == EINVAL &&
> +                   (aiocb->bs->open_flags & BDRV_O_NOCACHE) &&
> +                   !(aiocb->aio_type & QEMU_AIO_WRITE) &&
> +                   offset > 0) {
> +            /* O_DIRECT pread() may fail with EINVAL when offset is unaligned
> +             * after a short read.  Assume that O_DIRECT short reads only occur
> +             * at EOF.  Therefore this is a short read, not an I/O error.
> +             */
> +            break;
>          } else if (len == -1) {
>              offset = -errno;
>              break;
> -- 
> 1.9.3
> 
> 
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

Re: [Qemu-devel] [PATCH 2/2] qemu-iotests: add test case 101 for short file I/O

[Benoît Canet]

The Thursday 21 Aug 2014 à 13:44:08 (+0100), Stefan Hajnoczi wrote :
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/qemu-iotests/101     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/101.out | 10 ++++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 69 insertions(+)
>  create mode 100755 tests/qemu-iotests/101
>  create mode 100644 tests/qemu-iotests/101.out
> 
> diff --git a/tests/qemu-iotests/101 b/tests/qemu-iotests/101
> new file mode 100755
> index 0000000..70fbf25
> --- /dev/null
> +++ b/tests/qemu-iotests/101
> @@ -0,0 +1,58 @@
> +#!/bin/bash
> +#
> +# Test short file I/O
> +#
> +# Copyright (C) 2014 Red Hat, 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 2 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/>.
> +#
> +
> +# creator
> +owner=stefanha@redhat.com
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +
> +_cleanup()
> +{
> +	_cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt raw
> +_supported_proto file
> +_supported_os Linux
> +
> +
> +echo
> +echo "== creating short image file =="
> +dd if=/dev/zero of="$TEST_IMG" bs=1 count=320
> +
> +echo
> +echo "== reading bytes beyond EOF gives zeroes =="

We could pad the file with 0xFF to make sure the read really get only zeroes.

Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>

> +$QEMU_IO -c "read -P 0 0 512" "$TEST_IMG" | _filter_qemu_io
> +
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/101.out b/tests/qemu-iotests/101.out
> new file mode 100644
> index 0000000..9a996e8
> --- /dev/null
> +++ b/tests/qemu-iotests/101.out
> @@ -0,0 +1,10 @@
> +QA output created by 101
> +
> +== creating short image file ==
> +320+0 records in
> +320+0 records out
> +
> +== reading bytes beyond EOF gives zeroes ==
> +read 512/512 bytes at offset 0
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +*** done
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 6e67f61..e25e992 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -100,3 +100,4 @@
>  091 rw auto quick
>  092 rw auto quick
>  095 rw auto quick
> +101 rw auto quick
> -- 
> 1.9.3
> 
> 

Re: [Qemu-devel] [PATCH 0/2] raw-posix: fix O_DIRECT short reads

[Kevin Wolf]

Am 21.08.2014 um 14:44 hat Stefan Hajnoczi geschrieben:
> This series fixes qemu-iotests ./check -nocache -vmdk 059.  It also adds a
> dedicated test case for O_DIRECT short reads.
> 
> Stefan Hajnoczi (2):
>   raw-posix: fix O_DIRECT short reads
>   qemu-iotests: add test case 101 for short file I/O

Cc: qemu-stable@nongnu.org

Thanks, applied to the block branch.

Kevin