Skip to contentSkip to conversation

multiwrite patches for 2.2

PATCH In review Peter Lieven 4 participants 23 messages Oct 25, 2014 – Nov 27, 2014 Block layer

lore
Share & tools
lore

Peter Lieven

author
#0 lore raw

v3 cover multiwrite patches for 2.2

This adds some preparing patches for upcoming multiwrite modifications. I will leave the dangerous patches for after 2.2 release.

Due to oversized lines in the iotest output please pull from:
[email protected]:plieven/qemu.git -b multiwrite_22_v3

v2->v3: - Removed statistic output for merged read requests [Eric]
- Fixed double s-o-b line in Patch 3 [Eric]
- Fixed iotest in Patch 5 [Max]. Renamed from 108 to 109.

v1->v2: - incorporated Max's comments, but did not display the default
value for write merging (Patch 3) in the HMP since we do not do it for other commands. I would change this when the default changes.
- added an iotest for the write-merging cmdline parameter [Max]
- fixed iotest 067 output

Peter Lieven (6):
block: add accounting for merged requests
block: introduce bdrv_runtime_opts
block: add a knob to disable multiwrite_merge hw/virtio-blk: add a constant for max number of merged requests
block: add qemu-iotest for write-merge parameter
block: fix qemu-iotest reference output for test 067

 block.c                    |   49 +++++++++++++++++--
 block/accounting.c         |    7 +++
 block/qapi.c               |    2 +
 hmp.c                      |    8 +++-
 hw/block/virtio-blk.c      |    4 +-
 include/block/accounting.h |    3 ++
 include/block/block_int.h  |    1 +
 qapi/block-core.json       |   17 ++++++-
 qemu-options.hx            |    1 +
 qmp-commands.hx            |    2 +
 tests/qemu-iotests/067.out |   10 ++--
 tests/qemu-iotests/109     |  113 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/109.out |   68 ++++++++++++++++++++++++++
 tests/qemu-iotests/group   |    1 +
 14 files changed, 272 insertions(+), 14 deletions(-)
 create mode 100755 tests/qemu-iotests/109
 create mode 100644 tests/qemu-iotests/109.out
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH v3 1/6 block: add accounting for merged requests

Signed-off-by Peter Lieven <[email protected]>
---
 block.c                    |    2 ++
 block/accounting.c         |    7 +++++++
 block/qapi.c               |    1 +
 hmp.c                      |    4 +++-
 include/block/accounting.h |    3 +++
 qapi/block-core.json       |    7 ++++++-
 6 files changed, 22 insertions(+), 2 deletions(-)
block.c +2 -0
index 88f6d9b..d13b87e 100644
@@ -4477,6 +4477,8 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs,
         }
     }
 
+    block_merge_done(&bs->stats, BLOCK_ACCT_WRITE, num_reqs - outidx - 1);
+
     return outidx + 1;
 }
block/accounting.c +7 -0
index edbb1cc..f3162d1 100644
@@ -52,3 +52,10 @@ void block_acct_highest_sector(BlockAcctStats *stats, int64_t sector_num,
         stats->wr_highest_sector = sector_num + nb_sectors - 1;
     }
 }
+
+void block_merge_done(BlockAcctStats *stats, enum BlockAcctType type,
+                      int num_requests)
+{
+    assert(type < BLOCK_MAX_IOTYPE);
+    stats->merged[type] += num_requests;
+}
block/qapi.c +1 -0
index 1301144..28ea4db 100644
@@ -338,6 +338,7 @@ static BlockStats *bdrv_query_stats(const BlockDriverState *bs)
     s->stats->wr_bytes = bs->stats.nr_bytes[BLOCK_ACCT_WRITE];
     s->stats->rd_operations = bs->stats.nr_ops[BLOCK_ACCT_READ];
     s->stats->wr_operations = bs->stats.nr_ops[BLOCK_ACCT_WRITE];
+    s->stats->wr_merged = bs->stats.merged[BLOCK_ACCT_WRITE];
     s->stats->wr_highest_offset =
         bs->stats.wr_highest_sector * BDRV_SECTOR_SIZE;
     s->stats->flush_operations = bs->stats.nr_ops[BLOCK_ACCT_FLUSH];
hmp.c +3 -1
index 63d7686..5741dfd 100644
@@ -419,6 +419,7 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
                        " wr_total_time_ns=%" PRId64
                        " rd_total_time_ns=%" PRId64
                        " flush_total_time_ns=%" PRId64
+                       " wr_merged=%" PRId64
                        "\n",
                        stats->value->stats->rd_bytes,
                        stats->value->stats->wr_bytes,

@@ -427,7 +428,8 @@ void hmp_info_blockstats(Monitor *mon, const QDict *qdict)
                        stats->value->stats->flush_operations,
                        stats->value->stats->wr_total_time_ns,
                        stats->value->stats->rd_total_time_ns,
-                       stats->value->stats->flush_total_time_ns);
+                       stats->value->stats->flush_total_time_ns,
+                       stats->value->stats->wr_merged);
     }
 
     qapi_free_BlockStatsList(stats_list);
include/block/accounting.h +3 -0
index 50b42b3..07394f7 100644
@@ -39,6 +39,7 @@ typedef struct BlockAcctStats {
     uint64_t nr_bytes[BLOCK_MAX_IOTYPE];
     uint64_t nr_ops[BLOCK_MAX_IOTYPE];
     uint64_t total_time_ns[BLOCK_MAX_IOTYPE];
+    uint64_t merged[BLOCK_MAX_IOTYPE];
     uint64_t wr_highest_sector;
 } BlockAcctStats;
 

@@ -53,5 +54,7 @@ void block_acct_start(BlockAcctStats *stats, BlockAcctCookie *cookie,
 void block_acct_done(BlockAcctStats *stats, BlockAcctCookie *cookie);
 void block_acct_highest_sector(BlockAcctStats *stats, int64_t sector_num,
                                unsigned int nb_sectors);
+void block_merge_done(BlockAcctStats *stats, enum BlockAcctType type,
+                      int num_requests);
 
 #endif
qapi/block-core.json +6 -1
index 8f7089e..2095f9e 100644
@@ -392,13 +392,18 @@
 #                     growable sparse files (like qcow2) that are used on top
 #                     of a physical device.
 #
+# @wr_merged: Number of requests that have been merged into another request
+#             while performing a multiwrite_merge. (Since 2.2)
+#
+#
 # Since: 0.14.0
 ##
 { 'type': 'BlockDeviceStats',
   'data': {'rd_bytes': 'int', 'wr_bytes': 'int', 'rd_operations': 'int',
            'wr_operations': 'int', 'flush_operations': 'int',
            'flush_total_time_ns': 'int', 'wr_total_time_ns': 'int',
-           'rd_total_time_ns': 'int', 'wr_highest_offset': 'int' } }
+           'rd_total_time_ns': 'int', 'wr_highest_offset': 'int',
+           'wr_merged': 'int' } }
 
 ##
 # @BlockStats:
Signature · 1 line
1.7.9.5
Reviewed-by on v3 1/6
#2 lore raw

Re: [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests

Quoted 9 lines · Peter Lieven
Signed-off-by Peter Lieven <[email protected]>
---
  block.c                    |    2 ++
  block/accounting.c         |    7 +++++++
  block/qapi.c               |    1 +
  hmp.c                      |    4 +++-
  include/block/accounting.h |    3 +++
  qapi/block-core.json       |    7 ++++++-
  6 files changed, 22 insertions(+), 2 deletions(-)
Reviewed-by Max Reitz <[email protected]>
#3 lore raw

Re: [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests

In reply to Peter Lieven
Quoted 5 lines · Peter Lieven
block/accounting.c +0 -0
index edbb1cc..f3162d1 100644

block/accounting.c void block_acct_highest_sector(BlockAcctStats *stats, int64_t sector_num, · line 61

Quoted 10 lines
block/accounting.c +7 -0
@@ -52,3 +52,10 @@ void block_acct_highest_sector(BlockAcctStats *stats, int64_t sector_num,
         stats->wr_highest_sector = sector_num + nb_sectors - 1;
     }
 }
+
+void block_merge_done(BlockAcctStats *stats, enum BlockAcctType type,
+                      int num_requests)
+{
+    assert(type < BLOCK_MAX_IOTYPE);
+    stats->merged[type] += num_requests;
+}

Please keep the function name consistent with other functions in this
file: block_acct_merge_done().

Stefan

author
#4 lore raw

PATCH v3 2/6 block: introduce bdrv_runtime_opts

In reply to Peter Lieven

This patch (orginally by Kevin) adds a bdrv_runtime_opts QemuOptsList. The list will absorb all options that belong to the BDS (and not the BlockBackend) and will be parsed and handled in bdrv_open_common.

Signed-off-by Kevin Wolf <[email protected]>
Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 block.c |   38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)
block.c +33 -5
index d13b87e..f05ea0c 100644
@@ -27,6 +27,7 @@
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "qemu/module.h"
+#include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qjson.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/sysemu.h"

@@ -875,6 +876,19 @@ static void bdrv_assign_node_name(BlockDriverState *bs,
     QTAILQ_INSERT_TAIL(&graph_bdrv_states, bs, node_list);
 }
 
+static QemuOptsList bdrv_runtime_opts = {
+    .name = "bdrv_common",
+    .head = QTAILQ_HEAD_INITIALIZER(bdrv_runtime_opts.head),
+    .desc = {
+        {
+            .name = "node-name",
+            .type = QEMU_OPT_STRING,
+            .help = "Node name of the block device node",
+        },
+        { /* end of list */ }
+    },
+};
+
 /*
  * Common part for opening disk images and files
  *

@@ -886,6 +900,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     int ret, open_flags;
     const char *filename;
     const char *node_name = NULL;
+    QemuOpts *opts;
     Error *local_err = NULL;
 
     assert(drv != NULL);

@@ -906,19 +921,28 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
 
     trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
 
-    node_name = qdict_get_try_str(options, "node-name");
+    opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        ret = -EINVAL;
+        goto fail_opts;
+    }
+
+    node_name = qemu_opt_get(opts, "node-name");
     bdrv_assign_node_name(bs, node_name, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-        return -EINVAL;
+        ret = -EINVAL;
+        goto fail_opts;
     }
-    qdict_del(options, "node-name");
 
     /* bdrv_open() with directly using a protocol as drv. This layer is already
      * opened, so assign it to bs (while file becomes a closed BlockDriverState)
      * and return immediately. */
     if (file != NULL && drv->bdrv_file_open) {
         bdrv_swap(file, bs);
+        qemu_opts_del(opts);
         return 0;
     }
 

@@ -936,7 +960,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
                         ? "Driver '%s' can only be used for read-only devices"
                         : "Driver '%s' is not whitelisted",
                    drv->format_name);
-        return -ENOTSUP;
+        ret = -ENOTSUP;
+        goto fail_opts;
     }
 
     assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */

@@ -945,7 +970,8 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
             bdrv_enable_copy_on_read(bs);
         } else {
             error_setg(errp, "Can't use copy-on-read on read-only device");
-            return -EINVAL;
+            ret = -EINVAL;
+            goto fail_opts;
         }
     }
 

@@ -1010,6 +1036,8 @@ free_and_fail:
     g_free(bs->opaque);
     bs->opaque = NULL;
     bs->drv = NULL;
+fail_opts:
+    qemu_opts_del(opts);
     return ret;
 }
Signature · 1 line
1.7.9.5
Reviewed-by on v3 2/6
#5 lore raw

Re: [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts

Quoted 10 lines · Peter Lieven

This patch (orginally by Kevin) adds a bdrv_runtime_opts QemuOptsList. The list will absorb all options that belong to the BDS (and not the BlockBackend) and will be parsed and handled in bdrv_open_common.

Signed-off-by Kevin Wolf <[email protected]>
Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 block.c |   38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

Is this purely because the QemuOptsList API is more convenient than qdict?

I don't see a deeper reason why we must use QemuOptsList here.

The code is fine, however:

Reviewed-by Stefan Hajnoczi <[email protected]>
#6 lore raw

Re: [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts

Quoted 11 lines · Stefan Hajnoczi
Quoted 10 lines · Peter Lieven

This patch (orginally by Kevin) adds a bdrv_runtime_opts QemuOptsList. The list will absorb all options that belong to the BDS (and not the BlockBackend) and will be parsed and handled in bdrv_open_common.

Signed-off-by Kevin Wolf <[email protected]>
Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
  block.c |   38 +++++++++++++++++++++++++++++++++-----
  1 file changed, 33 insertions(+), 5 deletions(-)
Quoted 4 lines

Is this purely because the QemuOptsList API is more convenient than qdict?

I don't see a deeper reason why we must use QemuOptsList here.

If I remember correctly, it was because the command line options are only strings in the QDict. To be able to use types other than strings, it has to be converted to QemuOpts.

Max

author
#7 lore raw

PATCH v3 3/6 block: add a knob to disable multiwrite_merge

In reply to Peter Lieven

The block layer silently merges write requests since commit 40b4f539. This patch adds a knob to disable this feature as there has been some discussion lately if multiwrite is a good idea at all and as it falsifies benchmarks.

Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 block.c                   |    9 +++++++++
 block/qapi.c              |    1 +
 hmp.c                     |    4 ++++
 include/block/block_int.h |    1 +
 qapi/block-core.json      |   10 +++++++++-
 qemu-options.hx           |    1 +
 qmp-commands.hx           |    2 ++
 7 files changed, 27 insertions(+), 1 deletion(-)
block.c +9 -0
index f05ea0c..f3da5dd 100644
@@ -884,6 +884,10 @@ static QemuOptsList bdrv_runtime_opts = {
             .name = "node-name",
             .type = QEMU_OPT_STRING,
             .help = "Node name of the block device node",
+        },{
+            .name = "write-merging",
+            .type = QEMU_OPT_BOOL,
+            .help = "enable write merging (default: true)",
         },
         { /* end of list */ }
     },

@@ -986,6 +990,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     bs->opaque = g_malloc0(drv->instance_size);
 
     bs->enable_write_cache = !!(flags & BDRV_O_CACHE_WB);
+    bs->write_merging = qemu_opt_get_bool(opts, "write-merging", true);
 
     /* Open the image, either directly or using a protocol */
     if (drv->bdrv_file_open) {

@@ -4451,6 +4456,10 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs,
 {
     int i, outidx;
 
+    if (!bs->write_merging) {
+        return num_reqs;
+    }
+
     // Sort requests by start sector
     qsort(reqs, num_reqs, sizeof(*reqs), &multiwrite_req_compare);
block/qapi.c +1 -0
index 28ea4db..9136705 100644
@@ -59,6 +59,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs)
 
     info->backing_file_depth = bdrv_get_backing_file_depth(bs);
     info->detect_zeroes = bs->detect_zeroes;
+    info->write_merging = bs->write_merging;
 
     if (bs->io_limits_enabled) {
         ThrottleConfig cfg;
hmp.c +4 -0
index 5741dfd..48b85dd 100644
@@ -348,6 +348,10 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
                            BlockdevDetectZeroesOptions_lookup[info->value->inserted->detect_zeroes]);
         }
 
+        if (!info->value->inserted->write_merging) {
+            monitor_printf(mon, "    Write Merging:    off\n");
+        }
+
         if (info->value->inserted->bps
             || info->value->inserted->bps_rd
             || info->value->inserted->bps_wr
include/block/block_int.h +1 -0
index 8898c6c..e3d382f 100644
@@ -402,6 +402,7 @@ struct BlockDriverState {
 
     QDict *options;
     BlockdevDetectZeroesOptions detect_zeroes;
+    bool write_merging;
 
     /* The error object in use for blocking operations on backing_hd */
     Error *backing_blocker;
qapi/block-core.json +9 -1
index 2095f9e..74d1960 100644
@@ -214,6 +214,8 @@
 #
 # @detect_zeroes: detect and optimize zero writes (Since 2.1)
 #
+# @write_merging: true if write merging is enabled (Since 2.2)
+#
 # @bps: total throughput limit in bytes per second is specified
 #
 # @bps_rd: read throughput limit in bytes per second is specified

@@ -250,6 +252,7 @@
             '*backing_file': 'str', 'backing_file_depth': 'int',
             'encrypted': 'bool', 'encryption_key_missing': 'bool',
             'detect_zeroes': 'BlockdevDetectZeroesOptions',
+            'write_merging': 'bool',
             'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
             'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
             'image': 'ImageInfo',

@@ -1185,6 +1188,10 @@
 #                 (default: false)
 # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
 #                 (default: off)
+# @write-merging: #optional enable the merging of write requests
+#                 also known as multiwrite_merge (Since 2.2)
+#                 (default: true, but this might change in the future
+#                 depending on format/protocol/features used)
 #
 # Since: 1.7
 ##

@@ -1198,7 +1205,8 @@
             '*rerror': 'BlockdevOnError',
             '*werror': 'BlockdevOnError',
             '*read-only': 'bool',
-            '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
+            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
+            '*write-merging': 'bool' } }
 
 ##
 # @BlockdevOptionsFile
qemu-options.hx +1 -0
index 22cf3b9..d2f756f 100644
@@ -432,6 +432,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
+    "       [,write-merging=on|off]\n"
     "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
     "       [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
     "       [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
qmp-commands.hx +2 -0
index 1abd619..2c20207 100644
@@ -2104,6 +2104,7 @@ Each json-object contain the following:
          - "iops_size": I/O size when limiting by iops (json-int)
          - "detect_zeroes": detect and optimize zero writing (json-string)
              - Possible values: "off", "on", "unmap"
+         - "write_merging": enable merging of write requests (json-bool)
          - "image": the detail of the image, it is a json-object containing
             the following:
              - "filename": image file name (json-string)

@@ -2181,6 +2182,7 @@ Example:
                "iops_wr_max": 0,
                "iops_size": 0,
                "detect_zeroes": "on",
+               "write_merging": "true",
                "image":{
                   "filename":"disks/test.qcow2",
                   "format":"qcow2",
Signature · 1 line
1.7.9.5
#8 lore raw

Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge

block.c static QemuOptsList bdrv_runtime_opts = { · line 890

Quoted 12 lines · Peter Lieven
block.c +4 -0
index f05ea0c..f3da5dd 100644
@@ -884,6 +884,10 @@ static QemuOptsList bdrv_runtime_opts = {
             .name = "node-name",
             .type = QEMU_OPT_STRING,
             .help = "Node name of the block device node",
+        },{
+            .name = "write-merging",
+            .type = QEMU_OPT_BOOL,
+            .help = "enable write merging (default: true)",

QEMU_OPT_BOOL takes "on"/"off" instead of "true"/"false":
s/true/on/

Quoted 67 lines
qapi/block-core.json +9 -1
index 2095f9e..74d1960 100644
@@ -214,6 +214,8 @@
 #
 # @detect_zeroes: detect and optimize zero writes (Since 2.1)
 #
+# @write_merging: true if write merging is enabled (Since 2.2)
+#
 # @bps: total throughput limit in bytes per second is specified
 #
 # @bps_rd: read throughput limit in bytes per second is specified

@@ -250,6 +252,7 @@
             '*backing_file': 'str', 'backing_file_depth': 'int',
             'encrypted': 'bool', 'encryption_key_missing': 'bool',
             'detect_zeroes': 'BlockdevDetectZeroesOptions',
+            'write_merging': 'bool',
             'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
             'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
             'image': 'ImageInfo',

@@ -1185,6 +1188,10 @@
 #                 (default: false)
 # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
 #                 (default: off)
+# @write-merging: #optional enable the merging of write requests
+#                 also known as multiwrite_merge (Since 2.2)
+#                 (default: true, but this might change in the future
+#                 depending on format/protocol/features used)
 #
 # Since: 1.7
 ##

@@ -1198,7 +1205,8 @@
             '*rerror': 'BlockdevOnError',
             '*werror': 'BlockdevOnError',
             '*read-only': 'bool',
-            '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
+            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
+            '*write-merging': 'bool' } }
 
 ##
 # @BlockdevOptionsFile
qemu-options.hx +1 -0
index 22cf3b9..d2f756f 100644
@@ -432,6 +432,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
+    "       [,write-merging=on|off]\n"
     "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
     "       [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
     "       [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
qmp-commands.hx +1 -0
index 1abd619..2c20207 100644
@@ -2104,6 +2104,7 @@ Each json-object contain the following:
          - "iops_size": I/O size when limiting by iops (json-int)
          - "detect_zeroes": detect and optimize zero writing (json-string)
              - Possible values: "off", "on", "unmap"
+         - "write_merging": enable merging of write requests (json-bool)
          - "image": the detail of the image, it is a json-object containing
             the following:
              - "filename": image file name (json-string)

qmp-commands.hx Example: · line 2188

Quoted 7 lines
qmp-commands.hx +1 -0
@@ -2181,6 +2182,7 @@ Example:
                "iops_wr_max": 0,
                "iops_size": 0,
                "detect_zeroes": "on",
+               "write_merging": "true",
                "image":{
                   "filename":"disks/test.qcow2",
                   "format":"qcow2",

The big question is whether these user-visible interfaces make sense if write merging will be moved from block.c into virtio-blk.c in the future. Once an interface has been added it cannot be removed.

I think write merging in block.c is fine since other emulated storage controllers like virtio-scsi might also choose to use it.

But I want to raise the point for discussion in case others have strong feelings.

Stefan

author
#9 lore raw

Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge

Quoted 94 lines · Stefan Hajnoczi

block.c static QemuOptsList bdrv_runtime_opts = { · line 890

Quoted 12 lines · Peter Lieven
block.c +4 -0
index f05ea0c..f3da5dd 100644
@@ -884,6 +884,10 @@ static QemuOptsList bdrv_runtime_opts = {
              .name = "node-name",
              .type = QEMU_OPT_STRING,
              .help = "Node name of the block device node",
+        },{
+            .name = "write-merging",
+            .type = QEMU_OPT_BOOL,
+            .help = "enable write merging (default: true)",

QEMU_OPT_BOOL takes "on"/"off" instead of "true"/"false":
s/true/on/

Quoted 67 lines
qapi/block-core.json +9 -1
index 2095f9e..74d1960 100644
@@ -214,6 +214,8 @@
  #
  # @detect_zeroes: detect and optimize zero writes (Since 2.1)
  #
+# @write_merging: true if write merging is enabled (Since 2.2)
+#
  # @bps: total throughput limit in bytes per second is specified
  #
  # @bps_rd: read throughput limit in bytes per second is specified

@@ -250,6 +252,7 @@
              '*backing_file': 'str', 'backing_file_depth': 'int',
              'encrypted': 'bool', 'encryption_key_missing': 'bool',
              'detect_zeroes': 'BlockdevDetectZeroesOptions',
+            'write_merging': 'bool',
              'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
              'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
              'image': 'ImageInfo',

@@ -1185,6 +1188,10 @@
  #                 (default: false)
  # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
  #                 (default: off)
+# @write-merging: #optional enable the merging of write requests
+#                 also known as multiwrite_merge (Since 2.2)
+#                 (default: true, but this might change in the future
+#                 depending on format/protocol/features used)
  #
  # Since: 1.7
  ##

@@ -1198,7 +1205,8 @@
              '*rerror': 'BlockdevOnError',
              '*werror': 'BlockdevOnError',
              '*read-only': 'bool',
-            '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
+            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
+            '*write-merging': 'bool' } }
 
  ##
  # @BlockdevOptionsFile
qemu-options.hx +1 -0
index 22cf3b9..d2f756f 100644
@@ -432,6 +432,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
      "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
      "       [,readonly=on|off][,copy-on-read=on|off]\n"
      "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
+    "       [,write-merging=on|off]\n"
      "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
      "       [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
      "       [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
qmp-commands.hx +1 -0
index 1abd619..2c20207 100644
@@ -2104,6 +2104,7 @@ Each json-object contain the following:
           - "iops_size": I/O size when limiting by iops (json-int)
           - "detect_zeroes": detect and optimize zero writing (json-string)
               - Possible values: "off", "on", "unmap"
+         - "write_merging": enable merging of write requests (json-bool)
           - "image": the detail of the image, it is a json-object containing
              the following:
               - "filename": image file name (json-string)

qmp-commands.hx Example: · line 2188

Quoted 7 lines
qmp-commands.hx +1 -0
@@ -2181,6 +2182,7 @@ Example:
                 "iops_wr_max": 0,
                 "iops_size": 0,
                 "detect_zeroes": "on",
+               "write_merging": "true",
                 "image":{
                    "filename":"disks/test.qcow2",
                    "format":"qcow2",

The big question is whether these user-visible interfaces make sense if write merging will be moved from block.c into virtio-blk.c in the future. Once an interface has been added it cannot be removed.

Quoted 5 lines

I think write merging in block.c is fine since other emulated storage controllers like virtio-scsi might also choose to use it.

But I want to raise the point for discussion in case others have strong feelings.

It would be interesting to hear from Kevin what was the point why having the merging outside block.c was an issue for his coroutine optimizations.

Maybe if you have the time the other alpha patch I sent you see that moving it into virtio-blk.c makes some sense.

The reason merging was added in the past was that virtio-blk chops requests. Maybe virtio-scsi doesn't do that. I personally would feel better if qemu would not merge requests at all. But for virtio-blk it seems to be a big benefit in some cases also for reading.

Peter

author
#10 lore raw

Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge

In reply to Stefan Hajnoczi

Am 28.10.2014 um 12:11 schrieb Stefan Hajnoczi:

Quoted 85 lines · Peter Lieven

block.c static QemuOptsList bdrv_runtime_opts = { · line 890

Quoted 12 lines
block.c +4 -0
index f05ea0c..f3da5dd 100644
@@ -884,6 +884,10 @@ static QemuOptsList bdrv_runtime_opts = {
             .name = "node-name",
             .type = QEMU_OPT_STRING,
             .help = "Node name of the block device node",
+        },{
+            .name = "write-merging",
+            .type = QEMU_OPT_BOOL,
+            .help = "enable write merging (default: true)",

QEMU_OPT_BOOL takes "on"/"off" instead of "true"/"false":
s/true/on/

Quoted 69 lines
qapi/block-core.json +9 -1
index 2095f9e..74d1960 100644
@@ -214,6 +214,8 @@
 #
 # @detect_zeroes: detect and optimize zero writes (Since 2.1)
 #
+# @write_merging: true if write merging is enabled (Since 2.2)
+#
 # @bps: total throughput limit in bytes per second is specified
 #
 # @bps_rd: read throughput limit in bytes per second is specified

@@ -250,6 +252,7 @@
             '*backing_file': 'str', 'backing_file_depth': 'int',
             'encrypted': 'bool', 'encryption_key_missing': 'bool',
             'detect_zeroes': 'BlockdevDetectZeroesOptions',
+            'write_merging': 'bool',
             'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
             'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
             'image': 'ImageInfo',

@@ -1185,6 +1188,10 @@
 #                 (default: false)
 # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
 #                 (default: off)
+# @write-merging: #optional enable the merging of write requests
+#                 also known as multiwrite_merge (Since 2.2)
+#                 (default: true, but this might change in the future
+#                 depending on format/protocol/features used)
 #
 # Since: 1.7
 ##

@@ -1198,7 +1205,8 @@
             '*rerror': 'BlockdevOnError',
             '*werror': 'BlockdevOnError',
             '*read-only': 'bool',
-            '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
+            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
+            '*write-merging': 'bool' } }
 
 ##
 # @BlockdevOptionsFile
qemu-options.hx +1 -0
index 22cf3b9..d2f756f 100644
@@ -432,6 +432,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "       [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
+    "       [,write-merging=on|off]\n"
     "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
     "       [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
     "       [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
qmp-commands.hx +1 -0
index 1abd619..2c20207 100644
@@ -2104,6 +2104,7 @@ Each json-object contain the following:
          - "iops_size": I/O size when limiting by iops (json-int)
          - "detect_zeroes": detect and optimize zero writing (json-string)
              - Possible values: "off", "on", "unmap"
+         - "write_merging": enable merging of write requests (json-bool)
          - "image": the detail of the image, it is a json-object containing
             the following:
              - "filename": image file name (json-string)

@@ -2181,6 +2182,7 @@ Example:
                "iops_wr_max": 0,
                "iops_size": 0,
Quoted 8 lines

qmp-commands.hx Example: · line 2188

Quoted 5 lines
qmp-commands.hx +1 -0
                "detect_zeroes": "on",
+               "write_merging": "true",
                "image":{
                   "filename":"disks/test.qcow2",
                   "format":"qcow2",

The big question is whether these user-visible interfaces make sense if write merging will be moved from block.c into virtio-blk.c in the future. Once an interface has been added it cannot be removed.

Would you make it a feature of the virtio-blk-pci device then?

Peter

Quoted 8 lines

I think write merging in block.c is fine since other emulated storage controllers like virtio-scsi might also choose to use it.

But I want to raise the point for discussion in case others have strong feelings.

Stefan

#11 lore raw

Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge

Quoted 7 lines · Peter Lieven

Am 28.10.2014 um 12:11 schrieb Stefan Hajnoczi:

Quoted 4 lines · Peter Lieven

The big question is whether these user-visible interfaces make sense if write merging will be moved from block.c into virtio-blk.c in the future. Once an interface has been added it cannot be removed.

Would you make it a feature of the virtio-blk-pci device then?

No, I myself feel that it is a generic feature and moving it into the storage controller emulation code would be a step backwards.

Stefan

author
#12 lore raw

Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge

Quoted 9 lines · Stefan Hajnoczi
Quoted 6 lines · Peter Lieven

Am 28.10.2014 um 12:11 schrieb Stefan Hajnoczi:

Quoted 4 lines · Peter Lieven

The big question is whether these user-visible interfaces make sense if write merging will be moved from block.c into virtio-blk.c in the future. Once an interface has been added it cannot be removed.

Would you make it a feature of the virtio-blk-pci device then?

No, I myself feel that it is a generic feature and moving it into the storage controller emulation code would be a step backwards.

As noone else seems to have a strong opinion here I would go for making bdrv_multiwrite a bdrv_multireq and adjust everything accordingly?

Ok?

Peter

#13 lore raw

Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge

In reply to Peter Lieven
Quoted 18 lines · Peter Lieven

The block layer silently merges write requests since commit 40b4f539. This patch adds a knob to disable this feature as there has been some discussion lately if multiwrite is a good idea at all and as it falsifies benchmarks.

Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>

---

qapi/block-core.json +4 -0
+# @write-merging: #optional enable the merging of write requests
+#                 also known as multiwrite_merge (Since 2.2)
+#                 (default: true, but this might change in the future
+#                 depending on format/protocol/features used)
 #
 # Since: 1.7
 ##

qapi/block-core.json line 1209

Quoted 6 lines
qapi/block-core.json +2 -1
@@ -1198,7 +1205,8 @@
             '*rerror': 'BlockdevOnError',
             '*werror': 'BlockdevOnError',
             '*read-only': 'bool',
-            '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
+            '*detect-zeroes': 'BlockdevDetectZeroesOptions',
+            '*write-merging': 'bool' } }

This says it is boolean...

Quoted 10 lines
qmp-commands.hx +2 -0
+++ b/qmp-commands.hx

@@ -2104,6 +2104,7 @@ Each json-object contain the following:
          - "iops_size": I/O size when limiting by iops (json-int)
          - "detect_zeroes": detect and optimize zero writing (json-string)
              - Possible values: "off", "on", "unmap"
+         - "write_merging": enable merging of write requests (json-bool)
          - "image": the detail of the image, it is a json-object containing
             the following:
              - "filename": image file name (json-string)

qmp-commands.hx Example: · line 2185

Quoted 4 lines
qmp-commands.hx +1 -0
@@ -2181,6 +2182,7 @@ Example:
                "iops_wr_max": 0,
                "iops_size": 0,
                "detect_zeroes": "on",
+               "write_merging": "true",

...but this is not a JSON bool. s/"true"/true/

Signature · 2 lines
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org
author
#14 lore raw

PATCH v3 4/6 hw/virtio-blk: add a constant for max number of merged requests

In reply to Peter Lieven

As it was not obvious (at least for me) where the 32 comes from; add a constant for it.

Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 hw/block/virtio-blk.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
hw/block/virtio-blk.c +3 -1
index b19b102..ee7789f 100644
@@ -308,6 +308,8 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
     return true;
 }
 
+#define MAX_MERGE_REQS 32
+
 static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb)
 {
     BlockRequest *blkreq;

@@ -326,7 +328,7 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb)
     block_acct_start(blk_get_stats(req->dev->blk), &req->acct, req->qiov.size,
                      BLOCK_ACCT_WRITE);
 
-    if (mrb->num_writes == 32) {
+    if (mrb->num_writes == MAX_MERGE_REQS) {
         virtio_submit_multiwrite(req->dev->blk, mrb);
     }
Signature · 1 line
1.7.9.5
Reviewed-by on v3 4/6
#15 lore raw

Re: [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests

Quoted 8 lines · Peter Lieven

As it was not obvious (at least for me) where the 32 comes from; add a constant for it.

Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 hw/block/virtio-blk.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by Stefan Hajnoczi <[email protected]>
author
#16 lore raw

Re: [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests

Am 28.10.2014 um 12:17 schrieb Stefan Hajnoczi:

Quoted 10 lines · Peter Lieven
Quoted 8 lines

As it was not obvious (at least for me) where the 32 comes from; add a constant for it.

Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 hw/block/virtio-blk.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by Stefan Hajnoczi <[email protected]>

Found out later that this patch is incomplete. The 32 appears in vitio-blk.h for the limit of requests. So it has to be defined and also used there.

Peter

author
#17 lore raw

PATCH v3 5/6 block: add qemu-iotest for write-merge parameter

In reply to Peter Lieven
Signed-off-by Peter Lieven <[email protected]>
---
 tests/qemu-iotests/109     |  113 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/109.out |   68 ++++++++++++++++++++++++++
 tests/qemu-iotests/group   |    1 +
 3 files changed, 182 insertions(+)
 create mode 100755 tests/qemu-iotests/109
 create mode 100644 tests/qemu-iotests/109.out
tests/qemu-iotests/109 new file +113 -0
new file mode 100755
index 0000000..5ee0383
@@ -0,0 +1,113 @@
+#!/bin/bash
+#
+# Test if write-merging parameter is applied correctly
+#
+# Copyright (C) 2014 Peter Lieven <[email protected]>
+#
+# 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
[email protected]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+function do_run_qemu()
+{
+    echo Testing: "$@"
+    $QEMU -nographic -qmp stdio -serial none "$@"
+    echo
+}
+
+function run_qemu()
+{
+    do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp
+}
+
+size=128M
+
+_make_test_img $size
+
+echo
+echo === write-merging not specified ===
+echo
+
+run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=virtio,file.node-name=file <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-block" }
+{ "execute": "query-named-block-nodes" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo === write-merging set to on ===
+echo
+
+run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=virtio,file.node-name=file,write-merging=on <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-block" }
+{ "execute": "query-named-block-nodes" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo === write-merging set to off ===
+echo
+
+run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=virtio,file.node-name=file,write-merging=off <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-block" }
+{ "execute": "query-named-block-nodes" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo === file.write-merging set to on ===
+echo
+
+run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=virtio,file.node-name=file,file.write-merging=on <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-block" }
+{ "execute": "query-named-block-nodes" }
+{ "execute": "quit" }
+EOF
+
+echo
+echo === file.write-merging set to off ===
+echo
+
+run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=virtio,file.node-name=file,file.write-merging=off <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "query-block" }
+{ "execute": "query-named-block-nodes" }
+{ "execute": "quit" }
+EOF
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
tests/qemu-iotests/109.out new file +22 -0
new file mode 100644
index 0000000..3f43003
@@ -0,0 +1,68 @@
+QA output created by 109
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+
+=== write-merging not specified ===
+
+Testing: -drive file=TEST_DIR/t.raw,format=raw,if=virtio,file.node-name=file
+QMP_VERSION
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "virtio0", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.raw", "format": "raw", "actual-size": 0, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": [{"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {}, "iops_wr": 0, "ro": false, "node-name": "file", "backing_file_depth": 0, "drv": "file", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}]}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+
+
+=== write-merging set to on ===
+
+Testing: -drive file=TEST_DIR/t.raw,format=raw,if=virtio,file.node-name=file,write-merging=on
+QMP_VERSION
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "virtio0", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "json:{\"write-merging\": \"on\", \"driver\": \"raw\", \"file\": {\"driver\": \"file\", \"filename\": \"TEST_DIR/t.raw\"}}", "format": "raw", "actual-size": 0, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "json:{\"write-merging\": \"on\", \"driver\": \"raw\", \"file\": {\"driver\": \"file\", \"filename\": \"TEST_DIR/t.raw\"}}", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tra

y_open":
false, "type": "unknown"}]}
+{"return": [{"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {}, "iops_wr": 0, "ro": false, "node-name": "file", "backing_file_depth": 0, "drv": "file", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}]} +{"return": {}}

+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}

+
+
+=== write-merging set to off ===
+
+Testing: -drive file=TEST_DIR/t.raw,format=raw,if=virtio,file.node-name=file,write-merging=off
+QMP_VERSION
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "virtio0", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": false, "image": {"virtual-size": 134217728, "filename": "json:{\"write-merging\": \"off\", \"driver\": \"raw\", \"file\": {\"driver\": \"file\", \"filename\": \"TEST_DIR/t.raw\"}}", "format": "raw", "actual-size": 0, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "json:{\"write-merging\": \"off\", \"driver\": \"raw\", \"file\": {\"driver\": \"file\", \"filename\": \"TEST_DIR/t.raw\"}}", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "
tray_ope
n": false, "type": "unknown"}]}
+{"return": [{"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {}, "iops_wr": 0, "ro": false, "node-name": "file", "backing_file_depth": 0, "drv": "file", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}]}
+{"return": {}}

+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}

+
+
+=== file.write-merging set to on ===
+
+Testing: -drive file=TEST_DIR/t.raw,format=raw,if=virtio,file.node-name=file,file.write-merging=on
+QMP_VERSION
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "virtio0", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.raw", "format": "raw", "actual-size": 0, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} +{"return": [{"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {}, "iops_wr": 0, "ro": false, "node-name": "file", "backing_file_depth": 0, "drv": "file", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}]}
+{"return": {}}

+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}

+
+
+=== file.write-merging set to off ===
+
+Testing: -drive file=TEST_DIR/t.raw,format=raw,if=virtio,file.node-name=file,file.write-merging=off
+QMP_VERSION
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "virtio0", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.raw", "format": "raw", "actual-size": 0, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} +{"return": [{"iops_rd": 0, "detect_zeroes": "off", "write_merging": false, "image": {}, "iops_wr": 0, "ro": false, "node-name": "file", "backing_file_depth": 0, "drv": "file", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.raw", "encryption_key_missing": false}]}
+{"return": {}}

+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}

+
+*** done

tests/qemu-iotests/group +1 -0
index 9bbd5d3..659b086 100644
@@ -109,3 +109,4 @@
 105 rw auto quick
 107 rw auto quick
 108 rw auto quick
+109 rw auto quick
Signature · 1 line
1.7.9.5
Reviewed-by on v3 5/6
#18 lore raw

Re: [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter

Quoted 8 lines · Peter Lieven
Signed-off-by Peter Lieven <[email protected]>
---
  tests/qemu-iotests/109     |  113 ++++++++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/109.out |   68 ++++++++++++++++++++++++++
  tests/qemu-iotests/group   |    1 +
  3 files changed, 182 insertions(+)

create mode 100755 tests/qemu-iotests/109
create mode 100644 tests/qemu-iotests/109.out

First:

Reviewed-by Max Reitz <[email protected]>

Second: Why are you using query-block at all? Just give both the format and the protocol BDS a (different) node name and you could use only query-named-block-nodes.

Max

author
#19 lore raw

Re: [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter

Quoted 10 lines · Max Reitz
Quoted 8 lines · Peter Lieven
Signed-off-by Peter Lieven <[email protected]>
---
  tests/qemu-iotests/109     |  113 ++++++++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/109.out |   68 ++++++++++++++++++++++++++
  tests/qemu-iotests/group   |    1 +
  3 files changed, 182 insertions(+)

create mode 100755 tests/qemu-iotests/109
create mode 100644 tests/qemu-iotests/109.out

Quoted 5 lines

First:

Reviewed-by Max Reitz <[email protected]>

Second: Why are you using query-block at all? Just give both the format and the protocol BDS a (different) node name and you could use only query-named-block-nodes.

It was the useless try to make the lines shorter than 995 characters...

Peter

Quoted 2 lines

Max

Signature · 16 lines
Mit freundlichen Grüßen

Peter Lieven

...........................................................

  KAMP Netzwerkdienste GmbH
  Vestische Str. 89-91 | 46117 Oberhausen
  Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
  [email protected] | http://www.kamp.de

  Geschäftsführer: Heiner Lante | Michael Lante
  Amtsgericht Duisburg | HRB Nr. 12154
  USt-Id-Nr.: DE 120607556

...........................................................
author
#20 lore raw

PATCH v3 6/6 block: fix qemu-iotest reference output for test 067

In reply to Peter Lieven

Output is changed by the addition of the write-merging parameter

Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 tests/qemu-iotests/067.out |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tests/qemu-iotests/067.out +5 -5
index 0f72dcf..1944b9a 100644
@@ -6,7 +6,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0
 QMP_VERSION
 {"return": {}}
-{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
 {"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}}

@@ -24,7 +24,7 @@ QMP_VERSION
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk
 QMP_VERSION
 {"return": {}}
-{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
+{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
 {"return": {}}
 {"return": {}}
 {"return": {}}

@@ -44,7 +44,7 @@ Testing:
 QMP_VERSION
 {"return": {}}
 {"return": "OK\r\n"}
-{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
 {"return": {}}
 {"return": {}}
 {"return": {}}

@@ -64,14 +64,14 @@ Testing:
 QMP_VERSION
 {"return": {}}
 {"return": {}}
-{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
 {"return": {}}
 {"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"path": "/machine/peripheral/virtio0/virtio-backend"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_DELETED", "data": {"device": "virtio0", "path": "/machine/peripheral/virtio0"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "RESET"}
-{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
+{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "write_merging": true, "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
Signature · 1 line
1.7.9.5
#21 lore raw

Re: [Qemu-devel] [PATCHv3 6/6] block: fix qemu-iotest reference output for test 067

Quoted 7 lines · Peter Lieven

Output is changed by the addition of the write-merging parameter

Signed-off-by Peter Lieven <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 tests/qemu-iotests/067.out |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Please squash this into the commit that introduces the new output.

A patch series must be bisectable. qemu-iotests is not allowed to fail in the middle of the patch series, because that would be make it very hard to use git-bisect(1) later on.

Stefan