From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60060) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGq6A-0008A8-7h for qemu-devel@nongnu.org; Thu, 29 Jan 2015 09:29:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGq65-00042h-CQ for qemu-devel@nongnu.org; Thu, 29 Jan 2015 09:29:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40911) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGq65-00041P-55 for qemu-devel@nongnu.org; Thu, 29 Jan 2015 09:29:29 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0TETS9a024941 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Thu, 29 Jan 2015 09:29:28 -0500 Message-ID: <54CA43C6.5030103@redhat.com> Date: Thu, 29 Jan 2015 09:29:26 -0500 From: Max Reitz MIME-Version: 1.0 References: <1422412118-13151-1-git-send-email-famz@redhat.com> <1422412118-13151-6-git-send-email-famz@redhat.com> <54C90CD3.1050805@redhat.com> <20150129005307.GA12104@ad.nay.redhat.com> <20150129020616.GB12104@ad.nay.redhat.com> In-Reply-To: <20150129020616.GB12104@ad.nay.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v6 5/5] qemu-iotests: Add 093 for IO throttling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 2015-01-28 at 21:06, Fam Zheng wrote: > On Thu, 01/29 08:53, Fam Zheng wrote: >> On Wed, 01/28 11:22, Max Reitz wrote: >>> On 2015-01-27 at 21:28, 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 >>>> --- >>>> tests/qemu-iotests/093 | 120 +++++++++++++++++++++++++++++++++++++++++++++ >>>> tests/qemu-iotests/093.out | 5 ++ >>>> tests/qemu-iotests/group | 1 + >>>> 3 files changed, 126 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..2866536 >>>> --- /dev/null >>>> +++ b/tests/qemu-iotests/093 >>>> @@ -0,0 +1,120 @@ >>>> +#!/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 . >>>> +# >>>> + >>>> +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 robust >>>> + return limit == 0 or \ >>>> + (num < seconds * limit * 1.1 >>>> + and num > seconds * limit * 0.9) >>>> + >>>> + nsec_per_sec = 1000000000 >>>> + >>>> + params['device'] = 'drive0' >>>> + >>>> + 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) >>>> + >>>> + # Submit enough requests. 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 >>>> + rq_size = 512 >>>> + rd_nr = max(params['bps'] / rq_size / 2, >>>> + params['bps_rd'] / rq_size, >>>> + params['iops'] / 2, >>>> + params['iops_rd']) + \ >>>> + params['bps_max'] / rq_size / 2 + \ >>>> + params['iops_max'] >>> I guess the divisions by two are because those values represent read and >>> write operations combined. Shouldn't iops_max be divided by two, too, then? >>> >>>> + rd_nr *= seconds * 2 >>>> + wr_nr = max(params['bps'] / rq_size / 2, >>>> + params['bps_wr'] / rq_size, >>>> + params['iops'] / 2, >>>> + params['iops_wr']) + \ >>>> + params['bps_max'] / rq_size / 2 + \ >>>> + params['iops_max'] >>>> + wr_nr *= seconds * 2 >>>> + for i in range(rd_nr): >>>> + self.vm.hmp_qemu_io("drive0", "aio_read %d %d" % (i * rq_size, rq_size)) >>>> + for i in range(wr_nr): >>>> + self.vm.hmp_qemu_io("drive0", "aio_write %d %d" % (i * rq_size, rq_size)) >>>> + >>>> + 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)) >>> Hm, you're not checking bps_max and iops_max here. Should you be? >> I never really liked these two parameters, but now that you asked, probably >> yes (to this question and above). :) >> >> Fam > OK, messed for some time with *_max here and I'm giving up: > > /* fix bucket parameters */ > static void throttle_fix_bucket(LeakyBucket *bkt) > { > double min; > > /* zero bucket level */ > bkt->level = 0; > > /* The following is done to cope with the Linux CFQ block scheduler > * which regroup reads and writes by block of 100ms in the guest. > * When they are two process one making reads and one making writes cfq > * make a pattern looking like the following: > * WWWWWWWWWWWRRRRRRRRRRRRRRWWWWWWWWWWWWWwRRRRRRRRRRRRRRRRR > * Having a max burst value of 100ms of the average will help smooth the > * throttling > */ > min = bkt->avg / 10; > if (bkt->avg && !bkt->max) { > bkt->max = min; > } > } > > This is some magic that cannot be tested. So are you happy with this version? Would it be possible to just remove {b,io}ps_max completely from this test? I'd be fine with that. Max