[Qemu-devel] [PATCH v2 0/2] block/raw-posix: fix launching with failed disks

[Qemu-devel] [PATCH v2 0/2] block/raw-posix: fix launching with failed disks

[Stefan Hajnoczi]

Guests configured for multipath I/O might be started up with failed disks
attached.  QEMU should not refuse starting when a disk returns I/O errors (and
in the past this behavior was implemented correctly).

This patch series fixes a regression that prevents QEMU from opening failed
disks and adds a qemu-iotests test case to cover this use case.

Stefan Hajnoczi (2):
  block/raw-posix: fix launching with failed disks
  iotests: add O_DIRECT alignment probing test

 block/raw-posix.c          | 29 ++++++++++++++--
 tests/qemu-iotests/128     | 82 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/128.out |  5 +++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 115 insertions(+), 2 deletions(-)
 create mode 100755 tests/qemu-iotests/128
 create mode 100644 tests/qemu-iotests/128.out

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/2] block/raw-posix: fix launching with failed disks

[Stefan Hajnoczi]

Since commit c25f53b06eba1575d5d0e92a0132455c97825b83 ("raw: Probe
required direct I/O alignment") QEMU has failed to launch if image files
produce I/O errors.

Previously, QEMU would launch successfully and the guest would see the
errors when attempting I/O.

This is a regression and may prevent multipath I/O inside the guest,
where QEMU must launch and let the guest figure out by itself which
disks are online.

Tweak the alignment probing code in raw-posix.c to explicitly look for
EINVAL on Linux instead of bailing.  The kernel refuses misaligned
requests with this error code and other error codes can be ignored.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-posix.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 3263d2b..f0b4488 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -272,6 +272,31 @@ static int probe_physical_blocksize(int fd, unsigned int *blk_size)
 #endif
 }
 
+/* Check if read is allowed with given memory buffer and length.
+ *
+ * This function is used to check O_DIRECT memory buffer and request alignment.
+ */
+static bool raw_is_io_aligned(int fd, void *buf, size_t len)
+{
+    ssize_t ret = pread(fd, buf, len, 0);
+
+    if (ret >= 0) {
+        return true;
+    }
+
+#ifdef __linux__
+    /* The Linux kernel returns EINVAL for misaligned O_DIRECT reads.  Ignore
+     * other errors (e.g. real I/O error), which could happen on a failed
+     * drive, since we only care about probing alignment.
+     */
+    if (errno != EINVAL) {
+        return true;
+    }
+#endif
+
+    return false;
+}
+
 static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
@@ -307,7 +332,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
         size_t align;
         buf = qemu_memalign(MAX_BLOCKSIZE, 2 * MAX_BLOCKSIZE);
         for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
-            if (pread(fd, buf + align, MAX_BLOCKSIZE, 0) >= 0) {
+            if (raw_is_io_aligned(fd, buf + align, MAX_BLOCKSIZE)) {
                 s->buf_align = align;
                 break;
             }
@@ -319,7 +344,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
         size_t align;
         buf = qemu_memalign(s->buf_align, MAX_BLOCKSIZE);
         for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) {
-            if (pread(fd, buf, align, 0) >= 0) {
+            if (raw_is_io_aligned(fd, buf, align)) {
                 bs->request_alignment = align;
                 break;
             }
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/2] iotests: add O_DIRECT alignment probing test

[Stefan Hajnoczi]

This test case checks that image files can be opened even if I/O
produces EIO errors.  QEMU should not refuse opening failed disks since
the guest may be configured for multipath I/O where accessing failed
disks is expected.

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

diff --git a/tests/qemu-iotests/128 b/tests/qemu-iotests/128
new file mode 100755
index 0000000..249a865
--- /dev/null
+++ b/tests/qemu-iotests/128
@@ -0,0 +1,82 @@
+#!/bin/bash
+#
+# Test that opening O_DIRECT succeeds when image file I/O produces EIO
+#
+# Copyright (C) 2015 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!
+
+devname="eiodev$$"
+
+_setup_eiodev()
+{
+	# This test should either be run as root or with passwordless sudo
+	for cmd in "" "sudo -n"; do
+		echo "0 $((1024 * 1024 * 1024 / 512)) error" | \
+			$cmd dmsetup create "$devname" 2>/dev/null
+		if [ "$?" -eq 0 ]; then
+			return
+		fi
+	done
+	_notrun "root privileges required to run dmsetup"
+}
+
+_cleanup_eiodev()
+{
+	for cmd in "" "sudo -n"; do
+		$cmd dmsetup remove "$devname" 2>/dev/null
+		if [ "$?" -eq 0 ]; then
+			return
+		fi
+	done
+}
+
+_cleanup()
+{
+	_cleanup_eiodev
+}
+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
+
+_setup_eiodev
+
+TEST_IMG="/dev/mapper/$devname"
+
+echo
+echo "== reading from error device =="
+# Opening image should succeed but the read operation should fail
+$QEMU_IO --format "$IMGFMT" --nocache -c "read 0 65536" "$TEST_IMG" | _filter_qemu_io
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/128.out b/tests/qemu-iotests/128.out
new file mode 100644
index 0000000..4e43f5f
--- /dev/null
+++ b/tests/qemu-iotests/128.out
@@ -0,0 +1,5 @@
+QA output created by 128
+
+== reading from error device ==
+read failed: Input/output error
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 87eec39..71f19d4 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -121,3 +121,4 @@
 114 rw auto quick
 116 rw auto quick
 123 rw auto quick
+128 rw auto quick
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 0/2] block/raw-posix: fix launching with failed disks

[Kevin Wolf]

Am 05.03.2015 um 22:38 hat Stefan Hajnoczi geschrieben:
> Guests configured for multipath I/O might be started up with failed disks
> attached.  QEMU should not refuse starting when a disk returns I/O errors (and
> in the past this behavior was implemented correctly).
> 
> This patch series fixes a regression that prevents QEMU from opening failed
> disks and adds a qemu-iotests test case to cover this use case.
> 
> Stefan Hajnoczi (2):
>   block/raw-posix: fix launching with failed disks
>   iotests: add O_DIRECT alignment probing test

Thanks, applied to the block branch.

Kevin