[Qemu-devel] [PATCH v5 0/5] block: Add a qemu-iotests case for IO throttling

[Qemu-devel] [PATCH v5 0/5] block: Add a qemu-iotests case for IO throttling

[Fam Zheng]

v5: Rebase and improve the test. Please review again.

    Patch dependencies:

    This test depends on the qtest timer fix to run correctly.
    http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01865.html

    Also depends on the os check fix to run at all:
    http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01848.html

Original cover letter
---------------------

There is a change in qemu-io sub-commands "aio_read" and "aio_write", which
makes the aio requests accounted and the statistics reflected in blockstats.

Note that IO throttling implementation allows overcommiting of requests, so the
actual IO happened in a time unit may be a bit larger than given limits. In the
test case, the stats numbers are compared with a 10% error tolerance, to make
room for such flexibility in order to improve determinism.

Fam


Fam Zheng (5):
  qemu-io: Account IO by aio_read and aio_write
  qtest: Add scripts/qtest.py
  qemu-iotests: Add VM method qtest() to iotests.py
  qemu-iotests: Allow caller to disable underscore convertion for qmp
  qemu-iotests: Add 093 for IO throttling

 qemu-io-cmds.c                |  11 +++++
 scripts/qtest.py              |  71 +++++++++++++++++++++++++++++
 tests/qemu-iotests/093        | 103 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/093.out    |   5 ++
 tests/qemu-iotests/group      |   1 +
 tests/qemu-iotests/iotests.py |  23 ++++++++--
 6 files changed, 210 insertions(+), 4 deletions(-)
 create mode 100644 scripts/qtest.py
 create mode 100755 tests/qemu-iotests/093
 create mode 100644 tests/qemu-iotests/093.out

-- 
1.9.3

[Qemu-devel] [PATCH v5 1/5] qemu-io: Account IO by aio_read and aio_write

[Fam Zheng]

This will enable accounting of aio requests issued from qemu-io aio
read/write commands.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 qemu-io-cmds.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e708552..29377cd 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -13,6 +13,7 @@
 #include "block/qapi.h"
 #include "qemu/main-loop.h"
 #include "qemu/timer.h"
+#include "sysemu/block-backend.h"
 
 #define CMD_NOFILE_OK   0x01
 
@@ -1337,6 +1338,7 @@ out:
 }
 
 struct aio_ctx {
+    BlockDriverState *bs;
     QEMUIOVector qiov;
     int64_t offset;
     char *buf;
@@ -1344,6 +1346,7 @@ struct aio_ctx {
     int vflag;
     int Cflag;
     int Pflag;
+    BlockAcctCookie acct;
     int pattern;
     struct timeval t1;
 };
@@ -1361,6 +1364,8 @@ static void aio_write_done(void *opaque, int ret)
         goto out;
     }
 
+    block_acct_done(&ctx->bs->stats, &ctx->acct);
+
     if (ctx->qflag) {
         goto out;
     }
@@ -1398,6 +1403,8 @@ static void aio_read_done(void *opaque, int ret)
         g_free(cmp_buf);
     }
 
+    block_acct_done(&ctx->bs->stats, &ctx->acct);
+
     if (ctx->qflag) {
         goto out;
     }
@@ -1453,6 +1460,7 @@ static int aio_read_f(BlockDriverState *bs, int argc, char **argv)
     int nr_iov, c;
     struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
 
+    ctx->bs = bs;
     while ((c = getopt(argc, argv, "CP:qv")) != EOF) {
         switch (c) {
         case 'C':
@@ -1506,6 +1514,7 @@ static int aio_read_f(BlockDriverState *bs, int argc, char **argv)
     }
 
     gettimeofday(&ctx->t1, NULL);
+    block_acct_start(&bs->stats, &ctx->acct, ctx->qiov.size, BLOCK_ACCT_READ);
     bdrv_aio_readv(bs, ctx->offset >> 9, &ctx->qiov,
                    ctx->qiov.size >> 9, aio_read_done, ctx);
     return 0;
@@ -1549,6 +1558,7 @@ static int aio_write_f(BlockDriverState *bs, int argc, char **argv)
     int pattern = 0xcd;
     struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
 
+    ctx->bs = bs;
     while ((c = getopt(argc, argv, "CqP:")) != EOF) {
         switch (c) {
         case 'C':
@@ -1598,6 +1608,7 @@ static int aio_write_f(BlockDriverState *bs, int argc, char **argv)
     }
 
     gettimeofday(&ctx->t1, NULL);
+    block_acct_start(&bs->stats, &ctx->acct, ctx->qiov.size, BLOCK_ACCT_WRITE);
     bdrv_aio_writev(bs, ctx->offset >> 9, &ctx->qiov,
                     ctx->qiov.size >> 9, aio_write_done, ctx);
     return 0;
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 1/5] qemu-io: Account IO by aio_read and aio_write

[Max Reitz]

On 2015-01-16 at 03:46, Fam Zheng wrote:
> This will enable accounting of aio requests issued from qemu-io aio
> read/write commands.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   qemu-io-cmds.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)

I'm not sure what the status of this series is (Paolo already asked 
Kevin and Stefan to apply this series, so I guess it's acked by him), 
but anyway:

Reviewed-by: Max Reitz <mreitz@redhat.com>

[Qemu-devel] [PATCH v5 2/5] qtest: Add scripts/qtest.py

[Fam Zheng]

This adds scripts/qtest.py as a python library for qtest protocol.

This is a skeleton with a basic "cmd" method to execute a command,
reading and parsing of qtest output could be added later on demand.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 scripts/qtest.py | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 scripts/qtest.py

diff --git a/scripts/qtest.py b/scripts/qtest.py
new file mode 100644
index 0000000..25391f5
--- /dev/null
+++ b/scripts/qtest.py
@@ -0,0 +1,71 @@
+# QEMU qtest library
+#
+# Copyright (C) 2015 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng <famz@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+# Based on qmp.py.
+#
+
+import errno
+import socket
+
+class QEMUQtestProtocol(object):
+    def __init__(self, address, server=False):
+        """
+        Create a QEMUQtestProtocol object.
+
+        @param address: QEMU address, can be either a unix socket path (string)
+                        or a tuple in the form ( address, port ) for a TCP
+                        connection
+        @param server: server mode listens on the socket (bool)
+        @raise socket.error on socket connection errors
+        @note No connection is established, this is done by the connect() or
+              accept() methods
+        """
+        self._address = address
+        self._sock = self._get_sock()
+        if server:
+            self._sock.bind(self._address)
+            self._sock.listen(1)
+
+    def _get_sock(self):
+        if isinstance(self._address, tuple):
+            family = socket.AF_INET
+        else:
+            family = socket.AF_UNIX
+        return socket.socket(family, socket.SOCK_STREAM)
+
+    def connect(self):
+        """
+        Connect to the qtest socket.
+
+        @raise socket.error on socket connection errors
+        """
+        self._sock.connect(self._address)
+
+    def accept(self):
+        """
+        Await connection from QEMU.
+
+        @raise socket.error on socket connection errors
+        """
+        self._sock, _ = self._sock.accept()
+
+    def cmd(self, qtest_cmd):
+        """
+        Send a qtest command on the wire.
+
+        @param qtest_cmd: qtest command text to be sent
+        """
+        self._sock.sendall(qtest_cmd + "\n")
+
+    def close(self):
+        self._sock.close()
+
+    def settimeout(self, timeout):
+        self._sock.settimeout(timeout)
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 2/5] qtest: Add scripts/qtest.py

[Max Reitz]

On 2015-01-16 at 03:46, Fam Zheng wrote:
> This adds scripts/qtest.py as a python library for qtest protocol.
>
> This is a skeleton with a basic "cmd" method to execute a command,
> reading and parsing of qtest output could be added later on demand.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   scripts/qtest.py | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 71 insertions(+)
>   create mode 100644 scripts/qtest.py
>
> diff --git a/scripts/qtest.py b/scripts/qtest.py
> new file mode 100644
> index 0000000..25391f5
> --- /dev/null
> +++ b/scripts/qtest.py
> @@ -0,0 +1,71 @@
> +# QEMU qtest library
> +#
> +# Copyright (C) 2015 Red Hat Inc.
> +#
> +# Authors:
> +#  Fam Zheng <famz@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory.
> +#
> +# Based on qmp.py.
> +#
> +
> +import errno
> +import socket
> +
> +class QEMUQtestProtocol(object):
> +    def __init__(self, address, server=False):
> +        """
> +        Create a QEMUQtestProtocol object.
> +
> +        @param address: QEMU address, can be either a unix socket path (string)
> +                        or a tuple in the form ( address, port ) for a TCP
> +                        connection
> +        @param server: server mode listens on the socket (bool)

Maybe you could add a comma here ("server mode, listens...").

With or without:

Reviewed-by: Max Reitz <mreitz@redhat.com>

[Qemu-devel] [PATCH v5 3/5] qemu-iotests: Add VM method qtest() to iotests.py

[Fam Zheng]

This will allow test case to run command in qtest protocol. It's
write-only for now.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 241b5ee..85cb9a5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -21,8 +21,11 @@ import re
 import subprocess
 import string
 import unittest
-import sys; sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts', 'qmp'))
+import sys
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts'))
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts', 'qmp'))
 import qmp
+import qtest
 import struct
 
 __all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io',
@@ -81,10 +84,12 @@ class VM(object):
     def __init__(self):
         self._monitor_path = os.path.join(test_dir, 'qemu-mon.%d' % os.getpid())
         self._qemu_log_path = os.path.join(test_dir, 'qemu-log.%d' % os.getpid())
+        self._qtest_path = os.path.join(test_dir, 'qemu-qtest.%d' % os.getpid())
         self._args = qemu_args + ['-chardev',
                      'socket,id=mon,path=' + self._monitor_path,
                      '-mon', 'chardev=mon,mode=control',
-                     '-qtest', 'stdio', '-machine', 'accel=qtest',
+                     '-qtest', 'unix:path=' + self._qtest_path,
+                     '-machine', 'accel=qtest',
                      '-display', 'none', '-vga', 'none']
         self._num_drives = 0
 
@@ -160,9 +165,11 @@ class VM(object):
         qemulog = open(self._qemu_log_path, 'wb')
         try:
             self._qmp = qmp.QEMUMonitorProtocol(self._monitor_path, server=True)
+            self._qtest = qtest.QEMUQtestProtocol(self._qtest_path, server=True)
             self._popen = subprocess.Popen(self._args, stdin=devnull, stdout=qemulog,
                                            stderr=subprocess.STDOUT)
             self._qmp.accept()
+            self._qtest.accept()
         except:
             os.remove(self._monitor_path)
             raise
@@ -173,6 +180,7 @@ class VM(object):
             self._qmp.cmd('quit')
             self._popen.wait()
             os.remove(self._monitor_path)
+            os.remove(self._qtest_path)
             os.remove(self._qemu_log_path)
             self._popen = None
 
@@ -185,6 +193,10 @@ class VM(object):
 
         return self._qmp.cmd(cmd, args=qmp_args)
 
+    def qtest(self, cmd):
+        '''Send a qtest command to guest'''
+        return self._qtest.cmd(cmd)
+
     def get_qmp_event(self, wait=False):
         '''Poll for one queued QMP events and return it'''
         return self._qmp.pull_event(wait=wait)
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 3/5] qemu-iotests: Add VM method qtest() to iotests.py

[Max Reitz]

On 2015-01-16 at 03:46, Fam Zheng wrote:
> This will allow test case to run command in qtest protocol. It's

*test cases

> write-only for now.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[Qemu-devel] [PATCH v5 4/5] qemu-iotests: Allow caller to disable underscore convertion for qmp

[Fam Zheng]

QMP command "block_set_io_throttle" expects underscores in parameters
instead of dashes: {iops,bps}_{rd,wr,max}.

Add optional argument conv_keys (defaults to True, backward compatible),
it will be used in IO throttling test case.

Reviewed-by: Benoit Canet <benoit@irqsave.net>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 85cb9a5..1402854 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -185,11 +185,14 @@ class VM(object):
             self._popen = None
 
     underscore_to_dash = string.maketrans('_', '-')
-    def qmp(self, cmd, **args):
+    def qmp(self, cmd, conv_keys=True, **args):
         '''Invoke a QMP command and return the result dict'''
         qmp_args = dict()
         for k in args.keys():
-            qmp_args[k.translate(self.underscore_to_dash)] = args[k]
+            if conv_keys:
+                qmp_args[k.translate(self.underscore_to_dash)] = args[k]
+            else:
+                qmp_args[k] = args[k]
 
         return self._qmp.cmd(cmd, args=qmp_args)
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 4/5] qemu-iotests: Allow caller to disable underscore convertion for qmp

[Max Reitz]

On 2015-01-16 at 03:46, Fam Zheng wrote:
> QMP command "block_set_io_throttle" expects underscores in parameters
> instead of dashes: {iops,bps}_{rd,wr,max}.
>
> Add optional argument conv_keys (defaults to True, backward compatible),
> it will be used in IO throttling test case.
>
> Reviewed-by: Benoit Canet <benoit@irqsave.net>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[Qemu-devel] [PATCH v5 5/5] qemu-iotests: Add 093 for IO throttling

[Fam Zheng]

This case utilizes qemu-io command "aio_{read,write} -q" to verify the
effectiveness of IO throttling options.

It's implemented by driving the vm timer from qtest protocol, so the
throttling timers are signaled with determinied time duration. Then we
verify the completed IO requests are within 10% error of bps and iops
limits.

"null" protocol is used as the disk backend so that no actual disk IO is
performed on host, this will make the blockstats much more
deterministic. Both "null-aio" and "null-co" are covered, which is also
a simple cross validation test for the driver code.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/093     | 103 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/093.out |   5 +++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 109 insertions(+)
 create mode 100755 tests/qemu-iotests/093
 create mode 100644 tests/qemu-iotests/093.out

diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
new file mode 100755
index 0000000..d12cc25
--- /dev/null
+++ b/tests/qemu-iotests/093
@@ -0,0 +1,103 @@
+#!/usr/bin/env python
+#
+# Tests for IO throttling
+#
+# Copyright (C) 2015 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import iotests
+
+class ThrottleTestCase(iotests.QMPTestCase):
+    test_img = "null-aio://"
+
+    def blockstats(self, device):
+        result = self.vm.qmp("query-blockstats")
+        for r in result['return']:
+            if r['device'] == device:
+                stat = r['stats']
+                return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
+        raise Exception("Device not found for blockstats: %s" % device)
+
+    def setUp(self):
+        self.vm = iotests.VM().add_drive(self.test_img)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+
+    def do_test_throttle(self, seconds, params):
+        def check_limit(limit, num):
+            # IO throttling algorithm is discrete, allow 10% error so the test
+            # is more
+            return limit == 0 or \
+                   (num < seconds * limit * 1.1
+                   and num > seconds * limit * 0.9)
+
+        nsec_per_sec = 1000000000
+
+        params['device'] = 'drive0'
+        params['bps_max'] = 1
+        params['iops_max'] = 1
+
+        result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params)
+        self.assert_qmp(result, 'return', {})
+
+        # Set vm clock to a known value
+        ns = seconds * nsec_per_sec
+        self.vm.qtest("clock_step %d" % ns)
+
+        # Append many requests into the throttle queue. They will drain bps_max
+        # and iops_max, but the rest requests won't get executed until we
+        # advance the virtual clock with qtest interface
+        for i in range(1000):
+            self.vm.hmp_qemu_io("drive0", "aio_read %d 4096" % (i * 512))
+            self.vm.hmp_qemu_io("drive0", "aio_write %d 4096" % (i * 512))
+
+        start_rd_bytes, start_rd_iops, start_wr_bytes, start_wr_iops = self.blockstats('drive0')
+
+        self.vm.qtest("clock_step %d" % ns)
+        end_rd_bytes, end_rd_iops, end_wr_bytes, end_wr_iops = self.blockstats('drive0')
+
+        rd_bytes = end_rd_bytes - start_rd_bytes
+        rd_iops = end_rd_iops - start_rd_iops
+        wr_bytes = end_wr_bytes - start_wr_bytes
+        wr_iops = end_wr_iops - start_wr_iops
+
+        self.assertTrue(check_limit(params['bps'], rd_bytes + wr_bytes))
+        self.assertTrue(check_limit(params['bps_rd'], rd_bytes))
+        self.assertTrue(check_limit(params['bps_wr'], wr_bytes))
+        self.assertTrue(check_limit(params['iops'], rd_iops + wr_iops))
+        self.assertTrue(check_limit(params['iops_rd'], rd_iops))
+        self.assertTrue(check_limit(params['iops_wr'], wr_iops))
+
+    def test_all(self):
+        params = {"bps": 128000,
+                  "bps_rd": 128000,
+                  "bps_wr": 128000,
+                  "iops": 64,
+                  "iops_rd": 64,
+                  "iops_wr": 64
+                 }
+        for tk in params:
+            limits = dict([(k, 0) for k in params])
+            limits[tk] = params[tk]
+            self.do_test_throttle(10, limits)
+
+class ThrottleTestCoroutine(ThrottleTestCase):
+    test_img = "null-co://"
+
+if __name__ == '__main__':
+    iotests.main(supported_fmts=["raw"])
diff --git a/tests/qemu-iotests/093.out b/tests/qemu-iotests/093.out
new file mode 100644
index 0000000..fbc63e6
--- /dev/null
+++ b/tests/qemu-iotests/093.out
@@ -0,0 +1,5 @@
+..
+----------------------------------------------------------------------
+Ran 2 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index f8bf354..0272c9a 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -99,6 +99,7 @@
 090 rw auto quick
 091 rw auto
 092 rw auto quick
+093 auto
 095 rw auto quick
 097 rw auto backing
 098 rw auto backing quick
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 5/5] qemu-iotests: Add 093 for IO throttling

[Max Reitz]

On 2015-01-16 at 03:46, Fam Zheng wrote:
> This case utilizes qemu-io command "aio_{read,write} -q" to verify the
> effectiveness of IO throttling options.
>
> It's implemented by driving the vm timer from qtest protocol, so the
> throttling timers are signaled with determinied time duration. Then we
> verify the completed IO requests are within 10% error of bps and iops
> limits.
>
> "null" protocol is used as the disk backend so that no actual disk IO is
> performed on host, this will make the blockstats much more
> deterministic. Both "null-aio" and "null-co" are covered, which is also
> a simple cross validation test for the driver code.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   tests/qemu-iotests/093     | 103 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/093.out |   5 +++
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 109 insertions(+)
>   create mode 100755 tests/qemu-iotests/093
>   create mode 100644 tests/qemu-iotests/093.out

NACK. This literally kills my laptop (I can recover when running this 
test in tmpfs (for some reason inexplicable to me, since this uses the 
null block drivers...), but I cannot when running it on my HDD).

Would it be possible to use larger requests and smaller iops? (Or just 
the same request size but smaller bps as well)

PS: Feel free to tell me I'm doing something wrong, of course. But just 
./check -T -raw -c writethrough 093 just killed my laptop, and simply 
./check -raw 093 would have probably killed it, too, if I wouldn't have 
held down ^C after some seconds (I'm listening to music and that's when 
it began stuttering...).

> diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
> new file mode 100755
> index 0000000..d12cc25
> --- /dev/null
> +++ b/tests/qemu-iotests/093
> @@ -0,0 +1,103 @@
> +#!/usr/bin/env python
> +#
> +# Tests for IO throttling
> +#
> +# Copyright (C) 2015 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import iotests
> +
> +class ThrottleTestCase(iotests.QMPTestCase):
> +    test_img = "null-aio://"
> +
> +    def blockstats(self, device):
> +        result = self.vm.qmp("query-blockstats")
> +        for r in result['return']:
> +            if r['device'] == device:
> +                stat = r['stats']
> +                return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
> +        raise Exception("Device not found for blockstats: %s" % device)
> +
> +    def setUp(self):
> +        self.vm = iotests.VM().add_drive(self.test_img)
> +        self.vm.launch()
> +
> +    def tearDown(self):
> +        self.vm.shutdown()
> +
> +    def do_test_throttle(self, seconds, params):
> +        def check_limit(limit, num):
> +            # IO throttling algorithm is discrete, allow 10% error so the test
> +            # is more

"more robust"?

Max

Re: [Qemu-devel] [PATCH v5 5/5] qemu-iotests: Add 093 for IO throttling

[Fam Zheng]

On Mon, 01/26 15:45, Max Reitz wrote:
> On 2015-01-16 at 03:46, Fam Zheng wrote:
> >This case utilizes qemu-io command "aio_{read,write} -q" to verify the
> >effectiveness of IO throttling options.
> >
> >It's implemented by driving the vm timer from qtest protocol, so the
> >throttling timers are signaled with determinied time duration. Then we
> >verify the completed IO requests are within 10% error of bps and iops
> >limits.
> >
> >"null" protocol is used as the disk backend so that no actual disk IO is
> >performed on host, this will make the blockstats much more
> >deterministic. Both "null-aio" and "null-co" are covered, which is also
> >a simple cross validation test for the driver code.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> >  tests/qemu-iotests/093     | 103 +++++++++++++++++++++++++++++++++++++++++++++
> >  tests/qemu-iotests/093.out |   5 +++
> >  tests/qemu-iotests/group   |   1 +
> >  3 files changed, 109 insertions(+)
> >  create mode 100755 tests/qemu-iotests/093
> >  create mode 100644 tests/qemu-iotests/093.out
> 
> NACK. This literally kills my laptop (I can recover when running this test
> in tmpfs (for some reason inexplicable to me, since this uses the null block
> drivers...), but I cannot when running it on my HDD).
> 
> Would it be possible to use larger requests and smaller iops? (Or just the
> same request size but smaller bps as well)

Is it because of CPU or memory? 1000 requests for both read and write seem to
be overkilling since we are measuring 1000 bps and 10 iops, please try if
reducing to 100 requests works for you.

> 
> PS: Feel free to tell me I'm doing something wrong, of course. But just
> ./check -T -raw -c writethrough 093 just killed my laptop, and simply
> ./check -raw 093 would have probably killed it, too, if I wouldn't have held
> down ^C after some seconds (I'm listening to music and that's when it began
> stuttering...).
> 
> >diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
> >new file mode 100755
> >index 0000000..d12cc25
> >--- /dev/null
> >+++ b/tests/qemu-iotests/093
> >@@ -0,0 +1,103 @@
> >+#!/usr/bin/env python
> >+#
> >+# Tests for IO throttling
> >+#
> >+# Copyright (C) 2015 Red Hat, Inc.
> >+#
> >+# This program is free software; you can redistribute it and/or modify
> >+# it under the terms of the GNU General Public License as published by
> >+# the Free Software Foundation; either version 2 of the License, or
> >+# (at your option) any later version.
> >+#
> >+# This program is distributed in the hope that it will be useful,
> >+# but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+# GNU General Public License for more details.
> >+#
> >+# You should have received a copy of the GNU General Public License
> >+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >+#
> >+
> >+import iotests
> >+
> >+class ThrottleTestCase(iotests.QMPTestCase):
> >+    test_img = "null-aio://"
> >+
> >+    def blockstats(self, device):
> >+        result = self.vm.qmp("query-blockstats")
> >+        for r in result['return']:
> >+            if r['device'] == device:
> >+                stat = r['stats']
> >+                return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
> >+        raise Exception("Device not found for blockstats: %s" % device)
> >+
> >+    def setUp(self):
> >+        self.vm = iotests.VM().add_drive(self.test_img)
> >+        self.vm.launch()
> >+
> >+    def tearDown(self):
> >+        self.vm.shutdown()
> >+
> >+    def do_test_throttle(self, seconds, params):
> >+        def check_limit(limit, num):
> >+            # IO throttling algorithm is discrete, allow 10% error so the test
> >+            # is more
> 
> "more robust"?

Yes :)

Fam

Re: [Qemu-devel] [PATCH v5 5/5] qemu-iotests: Add 093 for IO throttling

[Max Reitz]

On 2015-01-26 at 22:03, Fam Zheng wrote:
> On Mon, 01/26 15:45, Max Reitz wrote:
>> On 2015-01-16 at 03:46, Fam Zheng wrote:
>>> This case utilizes qemu-io command "aio_{read,write} -q" to verify the
>>> effectiveness of IO throttling options.
>>>
>>> It's implemented by driving the vm timer from qtest protocol, so the
>>> throttling timers are signaled with determinied time duration. Then we
>>> verify the completed IO requests are within 10% error of bps and iops
>>> limits.
>>>
>>> "null" protocol is used as the disk backend so that no actual disk IO is
>>> performed on host, this will make the blockstats much more
>>> deterministic. Both "null-aio" and "null-co" are covered, which is also
>>> a simple cross validation test for the driver code.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>>   tests/qemu-iotests/093     | 103 +++++++++++++++++++++++++++++++++++++++++++++
>>>   tests/qemu-iotests/093.out |   5 +++
>>>   tests/qemu-iotests/group   |   1 +
>>>   3 files changed, 109 insertions(+)
>>>   create mode 100755 tests/qemu-iotests/093
>>>   create mode 100644 tests/qemu-iotests/093.out
>> NACK. This literally kills my laptop (I can recover when running this test
>> in tmpfs (for some reason inexplicable to me, since this uses the null block
>> drivers...), but I cannot when running it on my HDD).
>>
>> Would it be possible to use larger requests and smaller iops? (Or just the
>> same request size but smaller bps as well)
> Is it because of CPU or memory? 1000 requests for both read and write seem to
> be overkilling since we are measuring 1000 bps and 10 iops, please try if
> reducing to 100 requests works for you.

Probably memory, since I seem to recall you having the same model as me, 
but I can imagine you having more RAM...

100 requests do not work with 128,000 bps/64 iops/10 seconds (because 
that'd be more than 1 MB of data, whereas 100 requests of 4 kB are of 
course only 400 kB), but the following constellations work:
- 100 requests/128,000 bps/64 iops/1 second
- 100 requests/26,214 bps/8 iops/10 seconds (26,214 is about one tenth 
of 262,144 which is a multiple of 4,096, so the real value will not be 
too far off; the iops are limited by $number_of_requests / 
$duration_in_seconds)

I just hope there won't be some other poor guy for whom even this is too 
much...

Max

>> PS: Feel free to tell me I'm doing something wrong, of course. But just
>> ./check -T -raw -c writethrough 093 just killed my laptop, and simply
>> ./check -raw 093 would have probably killed it, too, if I wouldn't have held
>> down ^C after some seconds (I'm listening to music and that's when it began
>> stuttering...).
>>
>>> diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
>>> new file mode 100755
>>> index 0000000..d12cc25
>>> --- /dev/null
>>> +++ b/tests/qemu-iotests/093
>>> @@ -0,0 +1,103 @@
>>> +#!/usr/bin/env python
>>> +#
>>> +# Tests for IO throttling
>>> +#
>>> +# Copyright (C) 2015 Red Hat, Inc.
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 2 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +#
>>> +
>>> +import iotests
>>> +
>>> +class ThrottleTestCase(iotests.QMPTestCase):
>>> +    test_img = "null-aio://"
>>> +
>>> +    def blockstats(self, device):
>>> +        result = self.vm.qmp("query-blockstats")
>>> +        for r in result['return']:
>>> +            if r['device'] == device:
>>> +                stat = r['stats']
>>> +                return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
>>> +        raise Exception("Device not found for blockstats: %s" % device)
>>> +
>>> +    def setUp(self):
>>> +        self.vm = iotests.VM().add_drive(self.test_img)
>>> +        self.vm.launch()
>>> +
>>> +    def tearDown(self):
>>> +        self.vm.shutdown()
>>> +
>>> +    def do_test_throttle(self, seconds, params):
>>> +        def check_limit(limit, num):
>>> +            # IO throttling algorithm is discrete, allow 10% error so the test
>>> +            # is more
>> "more robust"?
> Yes :)
>
> Fam

Re: [Qemu-devel] [PATCH v5 5/5] qemu-iotests: Add 093 for IO throttling

[Fam Zheng]

On Tue, 01/27 11:14, Max Reitz wrote:
> On 2015-01-26 at 22:03, Fam Zheng wrote:
> >On Mon, 01/26 15:45, Max Reitz wrote:
> >>On 2015-01-16 at 03:46, Fam Zheng wrote:
> >>>This case utilizes qemu-io command "aio_{read,write} -q" to verify the
> >>>effectiveness of IO throttling options.
> >>>
> >>>It's implemented by driving the vm timer from qtest protocol, so the
> >>>throttling timers are signaled with determinied time duration. Then we
> >>>verify the completed IO requests are within 10% error of bps and iops
> >>>limits.
> >>>
> >>>"null" protocol is used as the disk backend so that no actual disk IO is
> >>>performed on host, this will make the blockstats much more
> >>>deterministic. Both "null-aio" and "null-co" are covered, which is also
> >>>a simple cross validation test for the driver code.
> >>>
> >>>Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>---
> >>>  tests/qemu-iotests/093     | 103 +++++++++++++++++++++++++++++++++++++++++++++
> >>>  tests/qemu-iotests/093.out |   5 +++
> >>>  tests/qemu-iotests/group   |   1 +
> >>>  3 files changed, 109 insertions(+)
> >>>  create mode 100755 tests/qemu-iotests/093
> >>>  create mode 100644 tests/qemu-iotests/093.out
> >>NACK. This literally kills my laptop (I can recover when running this test
> >>in tmpfs (for some reason inexplicable to me, since this uses the null block
> >>drivers...), but I cannot when running it on my HDD).
> >>
> >>Would it be possible to use larger requests and smaller iops? (Or just the
> >>same request size but smaller bps as well)
> >Is it because of CPU or memory? 1000 requests for both read and write seem to
> >be overkilling since we are measuring 1000 bps and 10 iops, please try if
> >reducing to 100 requests works for you.
> 
> Probably memory, since I seem to recall you having the same model as me, but
> I can imagine you having more RAM...
> 
> 100 requests do not work with 128,000 bps/64 iops/10 seconds (because that'd
> be more than 1 MB of data, whereas 100 requests of 4 kB are of course only
> 400 kB), but the following constellations work:

Oops, I changed bps and iops limits in v5 but was talking about 1000/10 here.
We can still lower the limits though. I'll send a v6 for you to try soon.

Fam

Re: [Qemu-devel] [PATCH v5 5/5] qemu-iotests: Add 093 for IO throttling

[Markus Armbruster]

Fam Zheng <famz@redhat.com> writes:

> On Tue, 01/27 11:14, Max Reitz wrote:
>> On 2015-01-26 at 22:03, Fam Zheng wrote:
>> >On Mon, 01/26 15:45, Max Reitz wrote:
>> >>On 2015-01-16 at 03:46, Fam Zheng wrote:
>> >>>This case utilizes qemu-io command "aio_{read,write} -q" to verify the
>> >>>effectiveness of IO throttling options.
>> >>>
>> >>>It's implemented by driving the vm timer from qtest protocol, so the
>> >>>throttling timers are signaled with determinied time duration. Then we
>> >>>verify the completed IO requests are within 10% error of bps and iops
>> >>>limits.
>> >>>
>> >>>"null" protocol is used as the disk backend so that no actual disk IO is
>> >>>performed on host, this will make the blockstats much more
>> >>>deterministic. Both "null-aio" and "null-co" are covered, which is also
>> >>>a simple cross validation test for the driver code.
>> >>>
>> >>>Signed-off-by: Fam Zheng <famz@redhat.com>
>> >>>---
>> >>>  tests/qemu-iotests/093 | 103
>> >>> +++++++++++++++++++++++++++++++++++++++++++++
>> >>>  tests/qemu-iotests/093.out |   5 +++
>> >>>  tests/qemu-iotests/group   |   1 +
>> >>>  3 files changed, 109 insertions(+)
>> >>>  create mode 100755 tests/qemu-iotests/093
>> >>>  create mode 100644 tests/qemu-iotests/093.out
>> >>NACK. This literally kills my laptop (I can recover when running this test
>> >>in tmpfs (for some reason inexplicable to me, since this uses the null block
>> >>drivers...), but I cannot when running it on my HDD).
>> >>
>> >>Would it be possible to use larger requests and smaller iops? (Or just the
>> >>same request size but smaller bps as well)
>> >Is it because of CPU or memory? 1000 requests for both read and write seem to
>> >be overkilling since we are measuring 1000 bps and 10 iops, please try if
>> >reducing to 100 requests works for you.
>> 
>> Probably memory, since I seem to recall you having the same model as me, but
>> I can imagine you having more RAM...
>> 
>> 100 requests do not work with 128,000 bps/64 iops/10 seconds (because that'd
>> be more than 1 MB of data, whereas 100 requests of 4 kB are of course only
>> 400 kB), but the following constellations work:
>
> Oops, I changed bps and iops limits in v5 but was talking about 1000/10 here.
> We can still lower the limits though. I'll send a v6 for you to try soon.

In general, please be mindful of test sizes, especially for tests in the
quick group.

Large sizes do cover more ground than small ones, but returns on
investment are diminishing sharply beyond a certain point.

We want everyone to run the quick tests all the time.  They better be
quick then, even on a somewhat underpowered laptop.  Not all
contributors work on company-sponsored, beefy new hardware.

When we have reason to believe that a big size is worthwhile to test, by
all means let's test it.  But let's test it outside the quick group.