All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2
@ 2014-10-25 16:55 Peter Lieven
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests Peter Lieven
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Peter Lieven @ 2014-10-25 16:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, famz, benoit, Peter Lieven, armbru, mreitz, stefanha

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:
 git@github.com: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

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests
  2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
@ 2014-10-25 16:55 ` Peter Lieven
  2014-10-27  8:50   ` Max Reitz
  2014-10-28 11:01   ` Stefan Hajnoczi
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts Peter Lieven
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 23+ messages in thread
From: Peter Lieven @ 2014-10-25 16:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, famz, benoit, Peter Lieven, armbru, mreitz, stefanha

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 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(-)

diff --git a/block.c b/block.c
index 88f6d9b..d13b87e 100644
--- a/block.c
+++ b/block.c
@@ -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;
 }
 
diff --git a/block/accounting.c b/block/accounting.c
index edbb1cc..f3162d1 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -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;
+}
diff --git a/block/qapi.c b/block/qapi.c
index 1301144..28ea4db 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -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];
diff --git a/hmp.c b/hmp.c
index 63d7686..5741dfd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -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);
diff --git a/include/block/accounting.h b/include/block/accounting.h
index 50b42b3..07394f7 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -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
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8f7089e..2095f9e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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:
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts
  2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests Peter Lieven
@ 2014-10-25 16:55 ` Peter Lieven
  2014-10-28 11:14   ` Stefan Hajnoczi
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge Peter Lieven
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Peter Lieven @ 2014-10-25 16:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, famz, benoit, Peter Lieven, armbru, mreitz, stefanha

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 <kwolf@redhat.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c |   38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index d13b87e..f05ea0c 100644
--- a/block.c
+++ b/block.c
@@ -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;
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge
  2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests Peter Lieven
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts Peter Lieven
@ 2014-10-25 16:55 ` Peter Lieven
  2014-10-28 11:11   ` Stefan Hajnoczi
  2014-10-28 15:22   ` Eric Blake
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests Peter Lieven
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 23+ messages in thread
From: Peter Lieven @ 2014-10-25 16:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, famz, benoit, Peter Lieven, armbru, mreitz, stefanha

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 <pl@kamp.de>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 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(-)

diff --git a/block.c b/block.c
index f05ea0c..f3da5dd 100644
--- a/block.c
+++ b/block.c
@@ -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);
 
diff --git a/block/qapi.c b/block/qapi.c
index 28ea4db..9136705 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -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;
diff --git a/hmp.c b/hmp.c
index 5741dfd..48b85dd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -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
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8898c6c..e3d382f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -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;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2095f9e..74d1960 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -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
diff --git a/qemu-options.hx b/qemu-options.hx
index 22cf3b9..d2f756f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -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"
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1abd619..2c20207 100644
--- a/qmp-commands.hx
+++ 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)
@@ -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",
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests
  2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
                   ` (2 preceding siblings ...)
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge Peter Lieven
@ 2014-10-25 16:55 ` Peter Lieven
  2014-10-28 11:17   ` Stefan Hajnoczi
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter Peter Lieven
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Peter Lieven @ 2014-10-25 16:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, famz, benoit, Peter Lieven, armbru, mreitz, stefanha

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

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 hw/block/virtio-blk.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b19b102..ee7789f 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -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);
     }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter
  2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
                   ` (3 preceding siblings ...)
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests Peter Lieven
@ 2014-10-25 16:55 ` Peter Lieven
  2014-10-27  9:08   ` Max Reitz
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 6/6] block: fix qemu-iotest reference output for test 067 Peter Lieven
  2014-10-28 10:58 ` [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Stefan Hajnoczi
  6 siblings, 1 reply; 23+ messages in thread
From: Peter Lieven @ 2014-10-25 16:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, famz, benoit, Peter Lieven, armbru, mreitz, stefanha

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 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

diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
new file mode 100755
index 0000000..5ee0383
--- /dev/null
+++ b/tests/qemu-iotests/109
@@ -0,0 +1,113 @@
+#!/bin/bash
+#
+# Test if write-merging parameter is applied correctly
+#
+# Copyright (C) 2014 Peter Lieven <pl@kamp.de>
+#
+# 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=pl@kamp.de
+
+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
diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
new file mode 100644
index 0000000..3f43003
--- /dev/null
+++ b/tests/qemu-iotests/109.out
@@ -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
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 9bbd5d3..659b086 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -109,3 +109,4 @@
 105 rw auto quick
 107 rw auto quick
 108 rw auto quick
+109 rw auto quick
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCHv3 6/6] block: fix qemu-iotest reference output for test 067
  2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
                   ` (4 preceding siblings ...)
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter Peter Lieven
@ 2014-10-25 16:55 ` Peter Lieven
  2014-10-28 11:20   ` Stefan Hajnoczi
  2014-10-28 10:58 ` [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Stefan Hajnoczi
  6 siblings, 1 reply; 23+ messages in thread
From: Peter Lieven @ 2014-10-25 16:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, famz, benoit, Peter Lieven, armbru, mreitz, stefanha

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

Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/067.out |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
index 0f72dcf..1944b9a 100644
--- a/tests/qemu-iotests/067.out
+++ b/tests/qemu-iotests/067.out
@@ -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}}
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests Peter Lieven
@ 2014-10-27  8:50   ` Max Reitz
  2014-10-28 11:01   ` Stefan Hajnoczi
  1 sibling, 0 replies; 23+ messages in thread
From: Max Reitz @ 2014-10-27  8:50 UTC (permalink / raw)
  To: Peter Lieven, qemu-devel; +Cc: kwolf, famz, benoit, armbru, stefanha

On 2014-10-25 at 18:55, Peter Lieven wrote:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>   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 <mreitz@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter Peter Lieven
@ 2014-10-27  9:08   ` Max Reitz
  2014-10-27  9:12     ` Peter Lieven
  0 siblings, 1 reply; 23+ messages in thread
From: Max Reitz @ 2014-10-27  9:08 UTC (permalink / raw)
  To: Peter Lieven, qemu-devel; +Cc: kwolf, famz, benoit, armbru, stefanha

On 2014-10-25 at 18:55, Peter Lieven wrote:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>   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 <mreitz@redhat.com>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter
  2014-10-27  9:08   ` Max Reitz
@ 2014-10-27  9:12     ` Peter Lieven
  0 siblings, 0 replies; 23+ messages in thread
From: Peter Lieven @ 2014-10-27  9:12 UTC (permalink / raw)
  To: Max Reitz, qemu-devel; +Cc: kwolf, famz, benoit, armbru, stefanha

On 27.10.2014 10:08, Max Reitz wrote:
> On 2014-10-25 at 18:55, Peter Lieven wrote:
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>   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 <mreitz@redhat.com>
>
> 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

>
> Max


-- 

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
   pl@kamp.de | http://www.kamp.de

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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2
  2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
                   ` (5 preceding siblings ...)
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 6/6] block: fix qemu-iotest reference output for test 067 Peter Lieven
@ 2014-10-28 10:58 ` Stefan Hajnoczi
  6 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-10-28 10:58 UTC (permalink / raw)
  To: Peter Lieven; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

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

On Sat, Oct 25, 2014 at 06:55:47PM +0200, Peter Lieven wrote:
> This adds some preparing patches for upcoming multiwrite modifications.
> I will leave the dangerous patches for after 2.2 release.

Soft freeze was 1st October:
http://qemu-project.org/Planning/2.2

This series seems to be a new feature, not a bug fix, so it would have
to wait for 2.3 anyway.

Stefan

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests Peter Lieven
  2014-10-27  8:50   ` Max Reitz
@ 2014-10-28 11:01   ` Stefan Hajnoczi
  1 sibling, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-10-28 11:01 UTC (permalink / raw)
  To: Peter Lieven; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

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

On Sat, Oct 25, 2014 at 06:55:48PM +0200, Peter Lieven wrote:
> diff --git a/block/accounting.c b/block/accounting.c
> index edbb1cc..f3162d1 100644
> --- a/block/accounting.c
> +++ b/block/accounting.c
> @@ -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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge Peter Lieven
@ 2014-10-28 11:11   ` Stefan Hajnoczi
  2014-10-28 11:26     ` Peter Lieven
  2014-10-31  8:31     ` Peter Lieven
  2014-10-28 15:22   ` Eric Blake
  1 sibling, 2 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-10-28 11:11 UTC (permalink / raw)
  To: Peter Lieven; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

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

On Sat, Oct 25, 2014 at 06:55:50PM +0200, Peter Lieven wrote:
> diff --git a/block.c b/block.c
> index f05ea0c..f3da5dd 100644
> --- a/block.c
> +++ b/block.c
> @@ -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/

> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 2095f9e..74d1960 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -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
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 22cf3b9..d2f756f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -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"
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 1abd619..2c20207 100644
> --- a/qmp-commands.hx
> +++ 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)
> @@ -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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts Peter Lieven
@ 2014-10-28 11:14   ` Stefan Hajnoczi
  2014-10-28 11:41     ` Max Reitz
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-10-28 11:14 UTC (permalink / raw)
  To: Peter Lieven; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

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

On Sat, Oct 25, 2014 at 06:55:49PM +0200, Peter Lieven wrote:
> 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 <kwolf@redhat.com>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  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 <stefanha@redhat.com>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests Peter Lieven
@ 2014-10-28 11:17   ` Stefan Hajnoczi
  2014-10-31  8:32     ` Peter Lieven
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-10-28 11:17 UTC (permalink / raw)
  To: Peter Lieven; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

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

On Sat, Oct 25, 2014 at 06:55:51PM +0200, Peter Lieven wrote:
> As it was not obvious (at least for me) where the 32 comes from;
> add a constant for it.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  hw/block/virtio-blk.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 6/6] block: fix qemu-iotest reference output for test 067
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 6/6] block: fix qemu-iotest reference output for test 067 Peter Lieven
@ 2014-10-28 11:20   ` Stefan Hajnoczi
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-10-28 11:20 UTC (permalink / raw)
  To: Peter Lieven; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

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

On Sat, Oct 25, 2014 at 06:55:53PM +0200, Peter Lieven wrote:
> Output is changed by the addition of the write-merging parameter
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge
  2014-10-28 11:11   ` Stefan Hajnoczi
@ 2014-10-28 11:26     ` Peter Lieven
  2014-10-31  8:31     ` Peter Lieven
  1 sibling, 0 replies; 23+ messages in thread
From: Peter Lieven @ 2014-10-28 11:26 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

On 28.10.2014 12:11, Stefan Hajnoczi wrote:
> On Sat, Oct 25, 2014 at 06:55:50PM +0200, Peter Lieven wrote:
>> diff --git a/block.c b/block.c
>> index f05ea0c..f3da5dd 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -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/
>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 2095f9e..74d1960 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -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
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 22cf3b9..d2f756f 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -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"
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 1abd619..2c20207 100644
>> --- a/qmp-commands.hx
>> +++ 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)
>> @@ -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.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts
  2014-10-28 11:14   ` Stefan Hajnoczi
@ 2014-10-28 11:41     ` Max Reitz
  0 siblings, 0 replies; 23+ messages in thread
From: Max Reitz @ 2014-10-28 11:41 UTC (permalink / raw)
  To: Stefan Hajnoczi, Peter Lieven; +Cc: kwolf, famz, benoit, qemu-devel, armbru

On 2014-10-28 at 12:14, Stefan Hajnoczi wrote:
> On Sat, Oct 25, 2014 at 06:55:49PM +0200, Peter Lieven wrote:
>> 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 <kwolf@redhat.com>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   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.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge
  2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge Peter Lieven
  2014-10-28 11:11   ` Stefan Hajnoczi
@ 2014-10-28 15:22   ` Eric Blake
  1 sibling, 0 replies; 23+ messages in thread
From: Eric Blake @ 2014-10-28 15:22 UTC (permalink / raw)
  To: Peter Lieven, qemu-devel; +Cc: kwolf, famz, benoit, armbru, mreitz, stefanha

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

On 10/25/2014 10:55 AM, Peter Lieven wrote:
> 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 <pl@kamp.de>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---

> +# @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' } }

This says it is boolean...


> +++ 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)
> @@ -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/

-- 
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 --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge
  2014-10-28 11:11   ` Stefan Hajnoczi
  2014-10-28 11:26     ` Peter Lieven
@ 2014-10-31  8:31     ` Peter Lieven
  2014-10-31 10:59       ` Stefan Hajnoczi
  1 sibling, 1 reply; 23+ messages in thread
From: Peter Lieven @ 2014-10-31  8:31 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

Am 28.10.2014 um 12:11 schrieb Stefan Hajnoczi:
> On Sat, Oct 25, 2014 at 06:55:50PM +0200, Peter Lieven wrote:
>> diff --git a/block.c b/block.c
>> index f05ea0c..f3da5dd 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -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/
>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 2095f9e..74d1960 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -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
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 22cf3b9..d2f756f 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -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"
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 1abd619..2c20207 100644
>> --- a/qmp-commands.hx
>> +++ 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)
>> @@ -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.

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

Peter

>
> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests
  2014-10-28 11:17   ` Stefan Hajnoczi
@ 2014-10-31  8:32     ` Peter Lieven
  0 siblings, 0 replies; 23+ messages in thread
From: Peter Lieven @ 2014-10-31  8:32 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

Am 28.10.2014 um 12:17 schrieb Stefan Hajnoczi:
> On Sat, Oct 25, 2014 at 06:55:51PM +0200, Peter Lieven wrote:
>> As it was not obvious (at least for me) where the 32 comes from;
>> add a constant for it.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  hw/block/virtio-blk.c |    4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge
  2014-10-31  8:31     ` Peter Lieven
@ 2014-10-31 10:59       ` Stefan Hajnoczi
  2014-11-27  9:32         ` Peter Lieven
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Hajnoczi @ 2014-10-31 10:59 UTC (permalink / raw)
  To: Peter Lieven; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

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

On Fri, Oct 31, 2014 at 09:31:53AM +0100, Peter Lieven wrote:
> Am 28.10.2014 um 12:11 schrieb Stefan Hajnoczi:
> > On Sat, Oct 25, 2014 at 06:55:50PM +0200, Peter Lieven wrote:
> > 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

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge
  2014-10-31 10:59       ` Stefan Hajnoczi
@ 2014-11-27  9:32         ` Peter Lieven
  0 siblings, 0 replies; 23+ messages in thread
From: Peter Lieven @ 2014-11-27  9:32 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kwolf, famz, benoit, armbru, qemu-devel, mreitz

On 31.10.2014 11:59, Stefan Hajnoczi wrote:
> On Fri, Oct 31, 2014 at 09:31:53AM +0100, Peter Lieven wrote:
>> Am 28.10.2014 um 12:11 schrieb Stefan Hajnoczi:
>>> On Sat, Oct 25, 2014 at 06:55:50PM +0200, Peter Lieven wrote:
>>> 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

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-11-27  9:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-25 16:55 [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Peter Lieven
2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 1/6] block: add accounting for merged requests Peter Lieven
2014-10-27  8:50   ` Max Reitz
2014-10-28 11:01   ` Stefan Hajnoczi
2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 2/6] block: introduce bdrv_runtime_opts Peter Lieven
2014-10-28 11:14   ` Stefan Hajnoczi
2014-10-28 11:41     ` Max Reitz
2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 3/6] block: add a knob to disable multiwrite_merge Peter Lieven
2014-10-28 11:11   ` Stefan Hajnoczi
2014-10-28 11:26     ` Peter Lieven
2014-10-31  8:31     ` Peter Lieven
2014-10-31 10:59       ` Stefan Hajnoczi
2014-11-27  9:32         ` Peter Lieven
2014-10-28 15:22   ` Eric Blake
2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 4/6] hw/virtio-blk: add a constant for max number of merged requests Peter Lieven
2014-10-28 11:17   ` Stefan Hajnoczi
2014-10-31  8:32     ` Peter Lieven
2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 5/6] block: add qemu-iotest for write-merge parameter Peter Lieven
2014-10-27  9:08   ` Max Reitz
2014-10-27  9:12     ` Peter Lieven
2014-10-25 16:55 ` [Qemu-devel] [PATCHv3 6/6] block: fix qemu-iotest reference output for test 067 Peter Lieven
2014-10-28 11:20   ` Stefan Hajnoczi
2014-10-28 10:58 ` [Qemu-devel] [PATCHv3 0/6] multiwrite patches for 2.2 Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.