[Qemu-devel] [PATCH v2 0/3] Fix recent qemu-iotests issues

[Qemu-devel] [PATCH v2 0/3] Fix recent qemu-iotests issues

[Eric Blake]

I introduced a couple of bugs in my recent qemu-io enhancements;
time to fix them back up now that the broken patches are already
part of mainline.

in v2:
- patch 1: retitle, fix id typo, also plug a coverity leak [mreitz]
- patch 3: drop dead comment [mreitz]

001/3:[down] 'qemu-io: Fix recent UI updates'
002/3:[----] [--] 'qemu-iotests: Simplify 109 with unaligned qemu-img compare'
003/3:[0003] [FC] 'qemu-iotests: Fix regression in 136 on aio_read invalid'

Eric Blake (3):
  qemu-io: Fix recent UI updates
  qemu-iotests: Simplify 109 with unaligned qemu-img compare
  qemu-iotests: Fix regression in 136 on aio_read invalid

 qemu-io-cmds.c             | 23 ++++++++++++++++++-----
 tests/qemu-iotests/109     |  2 --
 tests/qemu-iotests/109.out |  4 ----
 tests/qemu-iotests/136     | 15 ++++-----------
 4 files changed, 22 insertions(+), 22 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH v2 1/3] qemu-io: Fix recent UI updates

[Eric Blake]

Commit 770e0e0e [*] tried to add 'writev -f', but didn't tweak
the getopt() call to actually let it work.  Likewise, commit
c2e001c missed implementing 'aio_write -u -z'.  The latter commit
also introduced a leak of ctx.

[*] does it sound "ech0e" in here? :)

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

---
v2: retitle, fix commit id typo, also plug Coverity leak [mreitz]
---
 qemu-io-cmds.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 4a00bc6..1c0b692 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1150,7 +1150,7 @@ static int writev_f(BlockBackend *blk, int argc, char **argv)
     int pattern = 0xcd;
     QEMUIOVector qiov;

-    while ((c = getopt(argc, argv, "CqP:")) != -1) {
+    while ((c = getopt(argc, argv, "CfqP:")) != -1) {
         switch (c) {
         case 'C':
             Cflag = true;
@@ -1595,7 +1595,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
     int flags = 0;

     ctx->blk = blk;
-    while ((c = getopt(argc, argv, "CfqP:z")) != -1) {
+    while ((c = getopt(argc, argv, "CfqP:uz")) != -1) {
         switch (c) {
         case 'C':
             ctx->Cflag = true;
@@ -1638,6 +1638,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)

     if ((flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) {
         printf("-u requires -z to be specified\n");
+        g_free(ctx);
         return 0;
     }

-- 
2.5.5

[Qemu-devel] [PATCH v2 2/3] qemu-iotests: Simplify 109 with unaligned qemu-img compare

[Eric Blake]

For some time now, qemu-img compare has been able to compare
unaligned images.  So we no longer need test 109's hack of
resizing to sector boundaries before invoking compare.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>

---
Note that qemu-img compare on unaligned images is still a bit
underwhelming on message quality:

$ printf abc > file1
$ printf ab > file2
$ qemu-img compare file1 file2
Content mismatch at offset 0!
$ printf 'ab\0' > file1
$ qemu-img compare file1 file2
Images are identical.

The first message should claim that the mismatch is at offset 2
(or in sector 0), rather than at offset 0; and the second message
might be wise to mention that the sizes differ even though the
contents read identically (since we pad out 0s to the end of the
sector for both raw files).  But improving that is unrelated to
this patch.
---
 tests/qemu-iotests/109     | 2 --
 tests/qemu-iotests/109.out | 4 ----
 2 files changed, 6 deletions(-)

diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
index f980b0c..adf9889 100755
--- a/tests/qemu-iotests/109
+++ b/tests/qemu-iotests/109
@@ -104,8 +104,6 @@ for sample_img in empty.bochs iotest-dirtylog-10G-4M.vhdx parallels-v1 \
     $QEMU_IO -c 'read -P 0 0 64k' "$TEST_IMG" | _filter_qemu_io

     run_qemu "$TEST_IMG" "$TEST_IMG.src" "'format': 'raw'," "BLOCK_JOB_READY"
-    # qemu-img compare can't handle unaligned file sizes
-    $QEMU_IMG resize -f raw "$TEST_IMG.src" +0
     $QEMU_IMG compare -f raw -F raw "$TEST_IMG" "$TEST_IMG.src"
 done

diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
index 38bc073..7c797ed 100644
--- a/tests/qemu-iotests/109.out
+++ b/tests/qemu-iotests/109.out
@@ -143,7 +143,6 @@ read 65536/65536 bytes at offset 0
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror"}}
 {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
 Warning: Image size mismatch!
 Images are identical.

@@ -164,7 +163,6 @@ read 65536/65536 bytes at offset 0
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 31457280, "offset": 31457280, "speed": 0, "type": "mirror"}}
 {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 31457280, "offset": 31457280, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
 Warning: Image size mismatch!
 Images are identical.

@@ -185,7 +183,6 @@ read 65536/65536 bytes at offset 0
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 327680, "offset": 327680, "speed": 0, "type": "mirror"}}
 {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 327680, "offset": 327680, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
 Warning: Image size mismatch!
 Images are identical.

@@ -206,7 +203,6 @@ read 65536/65536 bytes at offset 0
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2048, "offset": 2048, "speed": 0, "type": "mirror"}}
 {"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2048, "offset": 2048, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
-Image resized.
 Warning: Image size mismatch!
 Images are identical.

-- 
2.5.5

[Qemu-devel] [PATCH v2 3/3] qemu-iotests: Fix regression in 136 on aio_read invalid

[Eric Blake]

Commit 093ea232 removed the ability for aio_read and aio_write
to artificially inflate the invalid statistics counters for
block devices, since it no longer flags unaligned offset or
length.  Add 'aio_read -i' and 'aio_write -i' to restore
the ability, and update test 136 to use it.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>

---
v2: drop useless comment [mreitz]
---
 qemu-io-cmds.c         | 20 ++++++++++++++++----
 tests/qemu-iotests/136 | 15 ++++-----------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 1c0b692..c7b2aec 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1476,6 +1476,7 @@ static void aio_read_help(void)
 " used to ensure all outstanding aio requests have been completed.\n"
 " -C, -- report statistics in a machine parsable format\n"
 " -P, -- use a pattern to verify read data\n"
+" -i, -- treat request as invalid, for exercising stats\n"
 " -v, -- dump buffer to standard output\n"
 " -q, -- quiet mode, do not show I/O statistics\n"
 "\n");
@@ -1488,7 +1489,7 @@ static const cmdinfo_t aio_read_cmd = {
     .cfunc      = aio_read_f,
     .argmin     = 2,
     .argmax     = -1,
-    .args       = "[-Cqv] [-P pattern] off len [len..]",
+    .args       = "[-Ciqv] [-P pattern] off len [len..]",
     .oneline    = "asynchronously reads a number of bytes",
     .help       = aio_read_help,
 };
@@ -1499,7 +1500,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
     struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);

     ctx->blk = blk;
-    while ((c = getopt(argc, argv, "CP:qv")) != -1) {
+    while ((c = getopt(argc, argv, "CP:iqv")) != -1) {
         switch (c) {
         case 'C':
             ctx->Cflag = true;
@@ -1512,6 +1513,11 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
                 return 0;
             }
             break;
+        case 'i':
+            printf("injecting invalid read request\n");
+            block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ);
+            g_free(ctx);
+            return 0;
         case 'q':
             ctx->qflag = true;
             break;
@@ -1569,6 +1575,7 @@ static void aio_write_help(void)
 " -P, -- use different pattern to fill file\n"
 " -C, -- report statistics in a machine parsable format\n"
 " -f, -- use Force Unit Access semantics\n"
+" -i, -- treat request as invalid, for exercising stats\n"
 " -q, -- quiet mode, do not show I/O statistics\n"
 " -u, -- with -z, allow unmapping\n"
 " -z, -- write zeroes using blk_aio_write_zeroes\n"
@@ -1582,7 +1589,7 @@ static const cmdinfo_t aio_write_cmd = {
     .cfunc      = aio_write_f,
     .argmin     = 2,
     .argmax     = -1,
-    .args       = "[-Cfquz] [-P pattern] off len [len..]",
+    .args       = "[-Cfiquz] [-P pattern] off len [len..]",
     .oneline    = "asynchronously writes a number of bytes",
     .help       = aio_write_help,
 };
@@ -1595,7 +1602,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
     int flags = 0;

     ctx->blk = blk;
-    while ((c = getopt(argc, argv, "CfqP:uz")) != -1) {
+    while ((c = getopt(argc, argv, "CfiqP:uz")) != -1) {
         switch (c) {
         case 'C':
             ctx->Cflag = true;
@@ -1616,6 +1623,11 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
                 return 0;
             }
             break;
+        case 'i':
+            printf("injecting invalid write request\n");
+            block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE);
+            g_free(ctx);
+            return 0;
         case 'z':
             ctx->zflag = true;
             break;
diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136
index e8c6937..6fdd268 100644
--- a/tests/qemu-iotests/136
+++ b/tests/qemu-iotests/136
@@ -226,18 +226,11 @@ sector = "%d"

         highest_offset = wr_ops * wr_size

-        # Two types of invalid operations: unaligned length and unaligned offset
-        for i in range(invalid_rd_ops / 2):
-            ops.append("aio_read 0 511")
+        for i in range(invalid_rd_ops):
+            ops.append("aio_read -i 0 512")

-        for i in range(invalid_rd_ops / 2, invalid_rd_ops):
-            ops.append("aio_read 13 512")
-
-        for i in range(invalid_wr_ops / 2):
-            ops.append("aio_write 0 511")
-
-        for i in range(invalid_wr_ops / 2, invalid_wr_ops):
-            ops.append("aio_write 13 512")
+        for i in range(invalid_wr_ops):
+            ops.append("aio_write -i 0 512")

         for i in range(failed_rd_ops):
             ops.append("aio_read %d 512" % bad_offset)
-- 
2.5.5

Re: [Qemu-devel] [PATCH v2 0/3] Fix recent qemu-iotests issues

[Max Reitz]

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

On 16.05.2016 18:43, Eric Blake wrote:
> I introduced a couple of bugs in my recent qemu-io enhancements;
> time to fix them back up now that the broken patches are already
> part of mainline.
> 
> in v2:
> - patch 1: retitle, fix id typo, also plug a coverity leak [mreitz]
> - patch 3: drop dead comment [mreitz]
> 
> 001/3:[down] 'qemu-io: Fix recent UI updates'
> 002/3:[----] [--] 'qemu-iotests: Simplify 109 with unaligned qemu-img compare'
> 003/3:[0003] [FC] 'qemu-iotests: Fix regression in 136 on aio_read invalid'
> 
> Eric Blake (3):
>   qemu-io: Fix recent UI updates
>   qemu-iotests: Simplify 109 with unaligned qemu-img compare
>   qemu-iotests: Fix regression in 136 on aio_read invalid
> 
>  qemu-io-cmds.c             | 23 ++++++++++++++++++-----
>  tests/qemu-iotests/109     |  2 --
>  tests/qemu-iotests/109.out |  4 ----
>  tests/qemu-iotests/136     | 15 ++++-----------
>  4 files changed, 22 insertions(+), 22 deletions(-)

Thanks Eric, I've applied the series to my block branch:

https://github.com/XanClic/qemu/commits/block

Max


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