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

[Stefan Hajnoczi]

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

pgpB96k_EkGsZ.pgp
Description: PGP signature