On Wed, 10/16 20:45, Max Reitz wrote:
> On 2013-10-15 04:41, Fam Zheng wrote:
> >If the block job completes too fast, the test can fail. Change the
> >numbers so the qmp events are more stably captured by the script.
> >
> >A sleep is removed for the same reason.
> >
> >Signed-off-by: Fam Zheng <f...@redhat.com>
> >---
> > tests/qemu-iotests/030 | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> >diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> >index ae56f3b..188b182 100755
> >--- a/tests/qemu-iotests/030
> >+++ b/tests/qemu-iotests/030
> >@@ -403,14 +403,13 @@ class TestStreamStop(iotests.QMPTestCase):
> > result = self.vm.qmp('block-stream', device='drive0')
> > self.assert_qmp(result, 'return', {})
> >- time.sleep(0.1)
>
> Hm, I'm not sure whether removing the sleep actually removes the
> underlying race condition… It should work in most cases and the
> foreseeable future, though.
>
> > events = self.vm.get_qmp_events(wait=False)
> > self.assertEqual(events, [], 'unexpected QMP event: %s' % events)
> > self.cancel_and_wait()
> > class TestSetSpeed(iotests.QMPTestCase):
> >- image_len = 80 * 1024 * 1024 # MB
> >+ image_len = 512 * 1024 * 1024 # MB
> > def setUp(self):
> > qemu_img('create', backing_img, str(TestSetSpeed.image_len))
> >@@ -457,23 +456,23 @@ class TestSetSpeed(iotests.QMPTestCase):
> > self.assert_qmp(result, 'return[0]/device', 'drive0')
> > self.assert_qmp(result, 'return[0]/speed', 0)
> >- result = self.vm.qmp('block-job-set-speed', device='drive0',
> >speed=8 * 1024 * 1024)
> >+ result = self.vm.qmp('block-job-set-speed', device='drive0',
> >speed=8 * 1024)
>
> So the limit was already 8 MB/s? Doesn't this mean that the job
> should have taken 10 seconds anyway? Sounds to me like the block job
> speed is basically disregarded anyway.
No, see below...
>
> If I re-add the sleep you removed in this patch, this test fails
> again for me. This further suggests block-job-set-speed to be kind
> of a no-op and the changes concerning the image length and the block
> job speed not really contributing to fixing the test.
>
> So I think removing the sleep is all that would have to be done
> right now. OTOH, this is not really a permanent fix, either (the
> fundamental race condition remains). Furthermore, I guess there is
> some reason for having a sleep there - else it would not exist in
> the first place (and it apparently already caused problems some time
> ago which were "fixed" by replacing the previous "sleep(1)" by
> "sleep(0.1)").
>
> All in all, if someone can assure me of the uneccessity of the sleep
> in question, I think removing it is all that's needed.
>
> Max
>
Both failure cases are just that setting speed or checking status comes too
late: the streaming finishes or goes close to finish in negligible no time once
the job is started. In other words dropping the speed change but only increase
image_len and remove sleep will fix it for me too.
Fam
