[Qemu-devel] [PATCH v2 0/2] block: fix multiwrite_merge() overlapping requests

[Qemu-devel] [PATCH v2 0/2] block: fix multiwrite_merge() overlapping requests

[Stefan Hajnoczi]

v2:
 * s/subset/superset/ in Patch 1 commit message [fam]
 * Add additional overlapping request test cases [eblake]

This is a fix for https://bugs.launchpad.net/qemu/+bug/1343827.

Patch 1 fixes the bug.  Patch 2 adds a qemu-iotests test case to prevent regressions.

Stefan Hajnoczi (2):
  block: fix overlapping multiwrite requests
  qemu-iotests: add multiwrite test cases

 block.c                    |   6 ++
 tests/qemu-iotests/100     | 134 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/100.out |  89 ++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 4 files changed, 230 insertions(+)
 create mode 100755 tests/qemu-iotests/100
 create mode 100644 tests/qemu-iotests/100.out

-- 
1.9.3

[Qemu-devel] [PATCH v2 1/2] block: fix overlapping multiwrite requests

[Stefan Hajnoczi]

When request A is a strict superset of request B:

  AAAAAAAA
    BBBB

multiwrite_merge() merges them as follows:

  AABBBB

The tail of request A should have been included:

  AABBBBAA

This patch fixes data loss but this code path is probably rare.  Since
guests cannot assume ordering between in-flight requests, few
applications submit overlapping write requests.

Reported-by: Slava Pestov <sviatoslav.pestov@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block.c b/block.c
index 8cf519b..0a3ac43 100644
--- a/block.c
+++ b/block.c
@@ -4498,6 +4498,12 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs,
             // Add the second request
             qemu_iovec_concat(qiov, reqs[i].qiov, 0, reqs[i].qiov->size);
 
+            // Add tail of first request, if necessary
+            if (qiov->size < reqs[outidx].qiov->size) {
+                qemu_iovec_concat(qiov, reqs[outidx].qiov, qiov->size,
+                                  reqs[outidx].qiov->size - qiov->size);
+            }
+
             reqs[outidx].nb_sectors = qiov->size >> 9;
             reqs[outidx].qiov = qiov;
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 2/2] qemu-iotests: add multiwrite test cases

[Stefan Hajnoczi]

This test case covers the basic bdrv_aio_multiwrite() scenarios:
1. Single request
2. Sequential requests (AABB)
3. Superset overlapping requests (AABBAA)
4. Subset overlapping requests (BBAABB)
5. Head overlapping requests (AABB)
6. Tail overlapping requests (BBAA)
7. Disjoint requests (AA BB)

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

diff --git a/tests/qemu-iotests/100 b/tests/qemu-iotests/100
new file mode 100755
index 0000000..9124aba
--- /dev/null
+++ b/tests/qemu-iotests/100
@@ -0,0 +1,134 @@
+#!/bin/bash
+#
+# Test simple read/write using plain bdrv_read/bdrv_write
+#
+# 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 generic
+_supported_proto generic
+_supported_os Linux
+
+
+size=128M
+
+echo
+echo "== Single request =="
+_make_test_img $size
+$QEMU_IO -c "multiwrite 0 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== verify pattern =="
+$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== Sequential requests =="
+_make_test_img $size
+$QEMU_IO -c "multiwrite 0 4k ; 4k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== verify pattern =="
+$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0xce 4k 4k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 8k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== Superset overlapping requests =="
+_make_test_img $size
+$QEMU_IO -c "multiwrite 0 4k ; 1k 2k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== verify pattern =="
+# Order of overlapping in-flight requests is not guaranteed so we cannot verify
+# [1k, 3k) since it could have either pattern 0xcd or 0xce.
+$QEMU_IO -c "read -P 0xcd 0 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0xcd 3k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== Subset overlapping requests =="
+_make_test_img $size
+$QEMU_IO -c "multiwrite 1k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== verify pattern =="
+# Order of overlapping in-flight requests is not guaranteed so we cannot verify
+# [1k, 3k) since it could have either pattern 0xcd or 0xce.
+$QEMU_IO -c "read -P 0xce 0 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0xce 3k 1k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== Head overlapping requests =="
+_make_test_img $size
+$QEMU_IO -c "multiwrite 0k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== verify pattern =="
+# Order of overlapping in-flight requests is not guaranteed so we cannot verify
+# [0k, 2k) since it could have either pattern 0xcd or 0xce.
+$QEMU_IO -c "read -P 0xce 2k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== Tail overlapping requests =="
+_make_test_img $size
+$QEMU_IO -c "multiwrite 2k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== verify pattern =="
+# Order of overlapping in-flight requests is not guaranteed so we cannot verify
+# [2k, 4k) since it could have either pattern 0xcd or 0xce.
+$QEMU_IO -c "read -P 0xce 0k 2k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== Disjoint requests =="
+_make_test_img $size
+$QEMU_IO -c "multiwrite 0 4k ; 64k 4k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "== verify pattern =="
+$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 4k 60k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0xce 64k 4k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 68k 4k" "$TEST_IMG" | _filter_qemu_io
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/100.out b/tests/qemu-iotests/100.out
new file mode 100644
index 0000000..2d6e9f0
--- /dev/null
+++ b/tests/qemu-iotests/100.out
@@ -0,0 +1,89 @@
+QA output created by 100
+
+== Single request ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Sequential requests ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 8192/8192 bytes at offset 0
+8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Superset overlapping requests ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 6144/6144 bytes at offset 0
+6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+read 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 3072
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Subset overlapping requests ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 6144/6144 bytes at offset 1024
+6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+read 1024/1024 bytes at offset 0
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 3072
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Head overlapping requests ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 6144/6144 bytes at offset 0
+6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+read 2048/2048 bytes at offset 2048
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Tail overlapping requests ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 6144/6144 bytes at offset 2048
+6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+read 2048/2048 bytes at offset 0
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Disjoint requests ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+wrote 8192/8192 bytes at offset 0
+8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 61440/61440 bytes at offset 4096
+60 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 65536
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 69632
+4 KiB, 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..8e2215e 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
+100 rw auto quick
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 2/2] qemu-iotests: add multiwrite test cases

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2154 bytes --]

On 07/30/2014 02:53 AM, Stefan Hajnoczi wrote:
> This test case covers the basic bdrv_aio_multiwrite() scenarios:
> 1. Single request
> 2. Sequential requests (AABB)
> 3. Superset overlapping requests (AABBAA)
> 4. Subset overlapping requests (BBAABB)
> 5. Head overlapping requests (AABB)
> 6. Tail overlapping requests (BBAA)
> 7. Disjoint requests (AA BB)
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/qemu-iotests/100     | 134 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/100.out |  89 ++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 224 insertions(+)
>  create mode 100755 tests/qemu-iotests/100
>  create mode 100644 tests/qemu-iotests/100.out
> 

> +echo "== Superset overlapping requests =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 0 4k ; 1k 2k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +# Order of overlapping in-flight requests is not guaranteed so we cannot verify
> +# [1k, 3k) since it could have either pattern 0xcd or 0xce.
> +$QEMU_IO -c "read -P 0xcd 0 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0xcd 3k 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io

I agree that we can't guarantee whether the first or the second write
requested ended up in [1k, 3k).  But can we enhance 'read' to allow one
or two new modes, so that we can do a test that no 0 bytes remain in the
region (all bytes were written), and/or do a test that all bytes in the
region have the same (unknown) value (we don't know whether A or B
finished first, but all bytes written came from the same job that
finished last)?  If we had such a verification mode, you could use it
here and in all the remaining tests where you skipped verification
because of not knowing which byte would be present.

But I would also be okay with enhancing 'read' as a followup patch, so:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 1/2] block: fix overlapping multiwrite requests

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On 07/30/2014 02:53 AM, Stefan Hajnoczi wrote:
> When request A is a strict superset of request B:
> 
>   AAAAAAAA
>     BBBB
> 
> multiwrite_merge() merges them as follows:
> 
>   AABBBB
> 
> The tail of request A should have been included:
> 
>   AABBBBAA
> 
> This patch fixes data loss but this code path is probably rare.  Since
> guests cannot assume ordering between in-flight requests, few
> applications submit overlapping write requests.
> 
> Reported-by: Slava Pestov <sviatoslav.pestov@gmail.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

Technically a bug fix, so okay for 2.1 if desired; but it is awfully
late and as you said it is fairly unlikely for guests to be relying on
overlapping multiwrite, so I'm okay with the decision to delay it to 2.2
and just have stable 2.1.1/downstream backports.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 1/2] block: fix overlapping multiwrite requests

[Benoît Canet]

The Wednesday 30 Jul 2014 à 09:53:30 (+0100), Stefan Hajnoczi wrote :
> When request A is a strict superset of request B:
> 
>   AAAAAAAA
>     BBBB
> 
> multiwrite_merge() merges them as follows:
> 
>   AABBBB
> 
> The tail of request A should have been included:
> 
>   AABBBBAA
> 
> This patch fixes data loss but this code path is probably rare.  Since
> guests cannot assume ordering between in-flight requests, few
> applications submit overlapping write requests.
> 
> Reported-by: Slava Pestov <sviatoslav.pestov@gmail.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 8cf519b..0a3ac43 100644
> --- a/block.c
> +++ b/block.c
> @@ -4498,6 +4498,12 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs,
>              // Add the second request
>              qemu_iovec_concat(qiov, reqs[i].qiov, 0, reqs[i].qiov->size);
>  
> +            // Add tail of first request, if necessary
> +            if (qiov->size < reqs[outidx].qiov->size) {
> +                qemu_iovec_concat(qiov, reqs[outidx].qiov, qiov->size,
> +                                  reqs[outidx].qiov->size - qiov->size);
> +            }
> +
>              reqs[outidx].nb_sectors = qiov->size >> 9;
>              reqs[outidx].qiov = qiov;
>  
> -- 
> 1.9.3
> 
> 

Seems to make sense.

Reviewed-by: Benoit Canet <benoit@irqsave.net>

Re: [Qemu-devel] [PATCH v2 2/2] qemu-iotests: add multiwrite test cases

[Benoît Canet]

The Wednesday 30 Jul 2014 à 09:53:31 (+0100), Stefan Hajnoczi wrote :
> This test case covers the basic bdrv_aio_multiwrite() scenarios:
> 1. Single request
> 2. Sequential requests (AABB)
> 3. Superset overlapping requests (AABBAA)
> 4. Subset overlapping requests (BBAABB)
> 5. Head overlapping requests (AABB)
> 6. Tail overlapping requests (BBAA)
> 7. Disjoint requests (AA BB)
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/qemu-iotests/100     | 134 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/100.out |  89 ++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 224 insertions(+)
>  create mode 100755 tests/qemu-iotests/100
>  create mode 100644 tests/qemu-iotests/100.out
> 
> diff --git a/tests/qemu-iotests/100 b/tests/qemu-iotests/100
> new file mode 100755
> index 0000000..9124aba
> --- /dev/null
> +++ b/tests/qemu-iotests/100
> @@ -0,0 +1,134 @@
> +#!/bin/bash
> +#
> +# Test simple read/write using plain bdrv_read/bdrv_write
> +#
> +# 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 generic
> +_supported_proto generic
> +_supported_os Linux
> +
> +
> +size=128M
> +
> +echo
> +echo "== Single request =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 0 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== Sequential requests =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 0 4k ; 4k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0xce 4k 4k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 8k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== Superset overlapping requests =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 0 4k ; 1k 2k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +# Order of overlapping in-flight requests is not guaranteed so we cannot verify
> +# [1k, 3k) since it could have either pattern 0xcd or 0xce.
> +$QEMU_IO -c "read -P 0xcd 0 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0xcd 3k 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== Subset overlapping requests =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 1k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +# Order of overlapping in-flight requests is not guaranteed so we cannot verify
> +# [1k, 3k) since it could have either pattern 0xcd or 0xce.
> +$QEMU_IO -c "read -P 0xce 0 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0xce 3k 1k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== Head overlapping requests =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 0k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +# Order of overlapping in-flight requests is not guaranteed so we cannot verify
> +# [0k, 2k) since it could have either pattern 0xcd or 0xce.
> +$QEMU_IO -c "read -P 0xce 2k 2k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== Tail overlapping requests =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 2k 2k ; 0k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +# Order of overlapping in-flight requests is not guaranteed so we cannot verify
> +# [2k, 4k) since it could have either pattern 0xcd or 0xce.
> +$QEMU_IO -c "read -P 0xce 0k 2k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 4k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== Disjoint requests =="
> +_make_test_img $size
> +$QEMU_IO -c "multiwrite 0 4k ; 64k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +echo
> +echo "== verify pattern =="
> +$QEMU_IO -c "read -P 0xcd 0 4k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 4k 60k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0xce 64k 4k" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0 68k 4k" "$TEST_IMG" | _filter_qemu_io
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/100.out b/tests/qemu-iotests/100.out
> new file mode 100644
> index 0000000..2d6e9f0
> --- /dev/null
> +++ b/tests/qemu-iotests/100.out
> @@ -0,0 +1,89 @@
> +QA output created by 100
> +
> +== Single request ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> +wrote 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== verify pattern ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 4096
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== Sequential requests ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> +wrote 8192/8192 bytes at offset 0
> +8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== verify pattern ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 4096
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 8192
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== Superset overlapping requests ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> +wrote 6144/6144 bytes at offset 0
> +6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== verify pattern ==
> +read 1024/1024 bytes at offset 0
> +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 1024/1024 bytes at offset 3072
> +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 4096
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== Subset overlapping requests ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> +wrote 6144/6144 bytes at offset 1024
> +6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== verify pattern ==
> +read 1024/1024 bytes at offset 0
> +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 1024/1024 bytes at offset 3072
> +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 4096
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== Head overlapping requests ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> +wrote 6144/6144 bytes at offset 0
> +6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== verify pattern ==
> +read 2048/2048 bytes at offset 2048
> +2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 4096
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== Tail overlapping requests ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> +wrote 6144/6144 bytes at offset 2048
> +6 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== verify pattern ==
> +read 2048/2048 bytes at offset 0
> +2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 4096
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== Disjoint requests ==
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
> +wrote 8192/8192 bytes at offset 0
> +8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +== verify pattern ==
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 61440/61440 bytes at offset 4096
> +60 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 65536
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 4096/4096 bytes at offset 69632
> +4 KiB, 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..8e2215e 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
> +100 rw auto quick
> -- 
> 1.9.3
> 
> 

Reviewed-by: Benoit Canet <benoit@irqsave.net>

Re: [Qemu-devel] [PATCH v2 0/2] block: fix multiwrite_merge() overlapping requests

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

On Wed, Jul 30, 2014 at 09:53:29AM +0100, Stefan Hajnoczi wrote:
> v2:
>  * s/subset/superset/ in Patch 1 commit message [fam]
>  * Add additional overlapping request test cases [eblake]
> 
> This is a fix for https://bugs.launchpad.net/qemu/+bug/1343827.
> 
> Patch 1 fixes the bug.  Patch 2 adds a qemu-iotests test case to prevent regressions.
> 
> Stefan Hajnoczi (2):
>   block: fix overlapping multiwrite requests
>   qemu-iotests: add multiwrite test cases
> 
>  block.c                    |   6 ++
>  tests/qemu-iotests/100     | 134 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/100.out |  89 ++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  4 files changed, 230 insertions(+)
>  create mode 100755 tests/qemu-iotests/100
>  create mode 100644 tests/qemu-iotests/100.out
> 
> -- 
> 1.9.3
> 
> 

Applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]