Re: [Qemu-devel] [PATCH v4 5/5] qemu-iotests: Add 093 for IO throttling
On Tue, 08/26 14:50, Stefan Hajnoczi wrote:
On Thu, Jun 05, 2014 at 04:47:46PM +0800, Fam Zheng wrote:
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, 1G)
+#self.vm = iotests.VM().add_drive(test_img, bps=1024,bps_max=1)
Commented out lines should be dropped
+self.vm = iotests.VM().add_drive(test_img)
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(test_img)
+
+def do_test_throttle(self, seconds=10, **limits):
+def check_limit(limit, num):
+# IO throttling algorithm is discrete, allow 10% error so the
test
+# is more deterministic
+return limit == 0 or num seconds * limit * 1.1
+
+nsec_per_sec = 10
+
+limits['bps_max'] = 1
+limits['iops_max'] = 1
+
+# Enqueue many requests to throttling queue
This comment is wrong, it actually happens further down
+result = self.vm.qmp(block_set_io_throttle, conv_keys=False,
**limits)
+self.assert_qmp(result, 'return', {})
+
+# Set vm clock to a known value
+ns = nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+
+# Append many requests into the throttle queue
+# They drain bps_max and iops_max
+# The rest requests won't get executed until qtest clock is driven
+for i in range(1000):
+self.vm.hmp_qemu_io(drive0, aio_read -a -q 0 512)
+self.vm.hmp_qemu_io(drive0, aio_write -a -q 0 512)
+
+start_rd_bytes, start_rd_iops, start_wr_bytes, start_wr_iops =
self.blockstats('drive0')
+
+ns += seconds * nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+# wait for a while to let requests take off
+time.sleep(1)
This is not a reliable testing approach. If the system is under heavy
load maybe only a few requests completed. We don't know whether that is
due to I/O throttling or not.
A reliable test would not perform real disk I/O so the test is
independent of disk/system speed. And it would not use time.sleep(1) to
wait since there is no guarantee that anything happened in the
meantime.
Do you think this can be improved?
Throttling only sets the upper limit of IO, this test checks the IO doesn't
cross it: when the test fails, something must be wrong with the throttling;
when the check passes, we can't guarantee that everything is correct. That's
not uncommon across many other test cases we have. The other half is very hard,
because of host load, etc., which are out of control of this script.
Regarding to disk IO, I've submitted a separate patch, the null:// protocol,
which can be used to sidestep the host disk load uncertainty. I can base this
test on top.
+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
+
+assert check_limit(limits['bps'], rd_bytes)
+assert check_limit(limits['bps_rd'], rd_bytes)
+assert check_limit(limits['bps'], wr_bytes)
+assert check_limit(limits['bps_wr'], wr_bytes)
+assert check_limit(limits['iops'], rd_iops)
+assert check_limit(limits['iops_rd'], rd_iops)
+assert check_limit(limits['iops'], wr_iops)
+assert check_limit(limits['iops_wr'], wr_iops)
Please use TestCase.assert*() methods instead of plain assert. They
produce humand-readable error messages including the failing values.
OK.
+
+def test_bps(self):
+self.do_test_throttle(**{
+'device': 'drive0',
+'bps': 1000,
+'bps_rd': 0,
+'bps_wr': 0,
+'iops': 0,
+'iops_rd': 0,
+'iops_wr': 0,
+})
Keyword argument syntax is more concise:
self.do_test_throttle(device='drive0',
bps=1000,
bps_rd=0,
...)
OK, will change.
Thanks,
Fam
Re: [Qemu-devel] [PATCH v4 5/5] qemu-iotests: Add 093 for IO throttling
On Wed, Aug 27, 2014 at 02:19:44PM +0800, Fam Zheng wrote:
On Tue, 08/26 14:50, Stefan Hajnoczi wrote:
On Thu, Jun 05, 2014 at 04:47:46PM +0800, Fam Zheng wrote:
+result = self.vm.qmp(block_set_io_throttle, conv_keys=False,
**limits)
+self.assert_qmp(result, 'return', {})
+
+# Set vm clock to a known value
+ns = nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+
+# Append many requests into the throttle queue
+# They drain bps_max and iops_max
+# The rest requests won't get executed until qtest clock is
driven
+for i in range(1000):
+self.vm.hmp_qemu_io(drive0, aio_read -a -q 0 512)
+self.vm.hmp_qemu_io(drive0, aio_write -a -q 0 512)
+
+start_rd_bytes, start_rd_iops, start_wr_bytes, start_wr_iops =
self.blockstats('drive0')
+
+ns += seconds * nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+# wait for a while to let requests take off
+time.sleep(1)
This is not a reliable testing approach. If the system is under heavy
load maybe only a few requests completed. We don't know whether that is
due to I/O throttling or not.
A reliable test would not perform real disk I/O so the test is
independent of disk/system speed. And it would not use time.sleep(1) to
wait since there is no guarantee that anything happened in the
meantime.
Do you think this can be improved?
Throttling only sets the upper limit of IO, this test checks the IO doesn't
cross it: when the test fails, something must be wrong with the throttling;
when the check passes, we can't guarantee that everything is correct. That's
not uncommon across many other test cases we have. The other half is very
hard,
because of host load, etc., which are out of control of this script.
Regarding to disk IO, I've submitted a separate patch, the null:// protocol,
which can be used to sidestep the host disk load uncertainty. I can base this
test on top.
If both a fake disk and fake timers are used then the execution is
deterministic. I'll take a look at the null:// protocol you have
posted.
Although I would love this test to be deterministic, I understand it is
tricky to achieve that. I'd like to discuss making this deterministic
but I'm not opposed to merging the current approach.
Stefan
pgpJp8DWeoagx.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH v4 5/5] qemu-iotests: Add 093 for IO throttling
On Thu, Jun 05, 2014 at 04:47:46PM +0800, Fam Zheng wrote:
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, 1G)
+#self.vm = iotests.VM().add_drive(test_img, bps=1024,bps_max=1)
Commented out lines should be dropped
+self.vm = iotests.VM().add_drive(test_img)
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(test_img)
+
+def do_test_throttle(self, seconds=10, **limits):
+def check_limit(limit, num):
+# IO throttling algorithm is discrete, allow 10% error so the
test
+# is more deterministic
+return limit == 0 or num seconds * limit * 1.1
+
+nsec_per_sec = 10
+
+limits['bps_max'] = 1
+limits['iops_max'] = 1
+
+# Enqueue many requests to throttling queue
This comment is wrong, it actually happens further down
+result = self.vm.qmp(block_set_io_throttle, conv_keys=False,
**limits)
+self.assert_qmp(result, 'return', {})
+
+# Set vm clock to a known value
+ns = nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+
+# Append many requests into the throttle queue
+# They drain bps_max and iops_max
+# The rest requests won't get executed until qtest clock is driven
+for i in range(1000):
+self.vm.hmp_qemu_io(drive0, aio_read -a -q 0 512)
+self.vm.hmp_qemu_io(drive0, aio_write -a -q 0 512)
+
+start_rd_bytes, start_rd_iops, start_wr_bytes, start_wr_iops =
self.blockstats('drive0')
+
+ns += seconds * nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+# wait for a while to let requests take off
+time.sleep(1)
This is not a reliable testing approach. If the system is under heavy
load maybe only a few requests completed. We don't know whether that is
due to I/O throttling or not.
A reliable test would not perform real disk I/O so the test is
independent of disk/system speed. And it would not use time.sleep(1) to
wait since there is no guarantee that anything happened in the
meantime.
Do you think this can be improved?
+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
+
+assert check_limit(limits['bps'], rd_bytes)
+assert check_limit(limits['bps_rd'], rd_bytes)
+assert check_limit(limits['bps'], wr_bytes)
+assert check_limit(limits['bps_wr'], wr_bytes)
+assert check_limit(limits['iops'], rd_iops)
+assert check_limit(limits['iops_rd'], rd_iops)
+assert check_limit(limits['iops'], wr_iops)
+assert check_limit(limits['iops_wr'], wr_iops)
Please use TestCase.assert*() methods instead of plain assert. They
produce humand-readable error messages including the failing values.
+
+def test_bps(self):
+self.do_test_throttle(**{
+'device': 'drive0',
+'bps': 1000,
+'bps_rd': 0,
+'bps_wr': 0,
+'iops': 0,
+'iops_rd': 0,
+'iops_wr': 0,
+})
Keyword argument syntax is more concise:
self.do_test_throttle(device='drive0',
bps=1000,
bps_rd=0,
...)
pgpZnfys40EXb.pgp
Description: PGP signature
[Qemu-devel] [PATCH v4 5/5] qemu-iotests: Add 093 for IO throttling
This case utilizes qemu-io command aio_{read,write} -a -q to verify
the effectiveness of IO throttling options.
The -a option will cause qemu-io requests to be accounted.
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 110% of bps and iops limits.
Signed-off-by: Fam Zheng f...@redhat.com
Reviewed-by: Benoit Canet ben...@irqsave.net
---
tests/qemu-iotests/093 | 164 +
tests/qemu-iotests/093.out | 5 ++
2 files changed, 169 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 000..222bb37
--- /dev/null
+++ b/tests/qemu-iotests/093
@@ -0,0 +1,164 @@
+#!/usr/bin/env python
+#
+# Tests for IO throttling
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see http://www.gnu.org/licenses/.
+#
+
+import time
+import os
+import iotests
+from iotests import qemu_img
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class ThrottleTestCase(iotests.QMPTestCase):
+image_len = 80 * 1024 * 1024 # MB
+
+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):
+qemu_img('create', '-f', iotests.imgfmt, test_img, 1G)
+#self.vm = iotests.VM().add_drive(test_img, bps=1024,bps_max=1)
+self.vm = iotests.VM().add_drive(test_img)
+self.vm.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+os.remove(test_img)
+
+def do_test_throttle(self, seconds=10, **limits):
+def check_limit(limit, num):
+# IO throttling algorithm is discrete, allow 10% error so the test
+# is more deterministic
+return limit == 0 or num seconds * limit * 1.1
+
+nsec_per_sec = 10
+
+limits['bps_max'] = 1
+limits['iops_max'] = 1
+
+# Enqueue many requests to throttling queue
+result = self.vm.qmp(block_set_io_throttle, conv_keys=False,
**limits)
+self.assert_qmp(result, 'return', {})
+
+# Set vm clock to a known value
+ns = nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+
+# Append many requests into the throttle queue
+# They drain bps_max and iops_max
+# The rest requests won't get executed until qtest clock is driven
+for i in range(1000):
+self.vm.hmp_qemu_io(drive0, aio_read -a -q 0 512)
+self.vm.hmp_qemu_io(drive0, aio_write -a -q 0 512)
+
+start_rd_bytes, start_rd_iops, start_wr_bytes, start_wr_iops =
self.blockstats('drive0')
+
+ns += seconds * nsec_per_sec
+self.vm.qtest_cmd(clock_step %d % ns)
+# wait for a while to let requests take off
+time.sleep(1)
+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
+
+assert check_limit(limits['bps'], rd_bytes)
+assert check_limit(limits['bps_rd'], rd_bytes)
+assert check_limit(limits['bps'], wr_bytes)
+assert check_limit(limits['bps_wr'], wr_bytes)
+assert check_limit(limits['iops'], rd_iops)
+assert check_limit(limits['iops_rd'], rd_iops)
+assert check_limit(limits['iops'], wr_iops)
+assert check_limit(limits['iops_wr'], wr_iops)
+
+def test_bps(self):
+self.do_test_throttle(**{
+'device': 'drive0',
+'bps': 1000,
+'bps_rd': 0,
+'bps_wr': 0,
+'iops': 0,
+'iops_rd': 0,
+'iops_wr': 0,
+})
+
+def test_bps_rd(self):
+self.do_test_throttle(**{
+'device': 'drive0',
+'bps': 0,
+'bps_rd': 1000,
+'bps_wr': 0,
+
