diff mbox

qemu-iotests: Avoid unnecessary sleeps

Message ID 1500649820-30450-1-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf July 21, 2017, 3:10 p.m. UTC 
Test cases 030, 041 and 055 used to sleep for a second after calling
block-job-pause to make sure that the block job had time to actually
get into paused state. We can instead poll its status and use that one
second only as a timeout.

The tests also slept a second for checking that the block jobs don't
make progress while being paused. Half a second is more than enough for
this.

These changes reduce the total time for the three tests by 25 seconds on
my laptop (from 155 seconds to 130).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/030        |  7 ++++---
 tests/qemu-iotests/041        | 12 ++++--------
 tests/qemu-iotests/055        | 15 +++++++++------
 tests/qemu-iotests/iotests.py | 27 +++++++++++++++++++++++++++
 4 files changed, 44 insertions(+), 17 deletions(-)

Comments

Eric Blake July 21, 2017, 3:34 p.m. UTC | #1 
On 07/21/2017 10:10 AM, Kevin Wolf wrote:
> Test cases 030, 041 and 055 used to sleep for a second after calling
> block-job-pause to make sure that the block job had time to actually
> get into paused state. We can instead poll its status and use that one
> second only as a timeout.
> 
> The tests also slept a second for checking that the block jobs don't
> make progress while being paused. Half a second is more than enough for
> this.
> 
> These changes reduce the total time for the three tests by 25 seconds on
> my laptop (from 155 seconds to 130).

Nice!

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>
Kevin Wolf July 24, 2017, 9:33 a.m. UTC | #2 
Am 21.07.2017 um 17:10 hat Kevin Wolf geschrieben:
> Test cases 030, 041 and 055 used to sleep for a second after calling
> block-job-pause to make sure that the block job had time to actually
> get into paused state. We can instead poll its status and use that one
> second only as a timeout.
> 
> The tests also slept a second for checking that the block jobs don't
> make progress while being paused. Half a second is more than enough for
> this.
> 
> These changes reduce the total time for the three tests by 25 seconds on
> my laptop (from 155 seconds to 130).
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Applied to the block branch.

Kevin
no-reply@patchew.org July 27, 2017, 1:06 p.m. UTC | #3 
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH] qemu-iotests: Avoid unnecessary sleeps
Message-id: 1500649820-30450-1-git-send-email-kwolf@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
8d5e114 qemu-iotests: Avoid unnecessary sleeps

=== OUTPUT BEGIN ===
Checking PATCH 1/1: qemu-iotests: Avoid unnecessary sleeps...
ERROR: line over 90 characters
#181: FILE: tests/qemu-iotests/iotests.py:372:
+                    if job['device'] == job_id and job['paused'] == True and job['busy'] == False:

total: 1 errors, 0 warnings, 141 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
diff mbox

Patch

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index feee861..d745cb4 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -89,18 +89,19 @@  class TestSingleDrive(iotests.QMPTestCase):
         result = self.vm.qmp('block-job-pause', device='drive0')
         self.assert_qmp(result, 'return', {})
 
-        time.sleep(1)
+        self.vm.resume_drive('drive0')
+        self.pause_job('drive0')
+
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
 
-        time.sleep(1)
+        time.sleep(0.5)
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return[0]/offset', offset)
 
         result = self.vm.qmp('block-job-resume', device='drive0')
         self.assert_qmp(result, 'return', {})
 
-        self.vm.resume_drive('drive0')
         self.wait_until_completed()
 
         self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 60f09cc..4cda540 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -103,14 +103,12 @@  class TestSingleDrive(iotests.QMPTestCase):
                              target=self.qmp_target)
         self.assert_qmp(result, 'return', {})
 
-        result = self.vm.qmp('block-job-pause', device='drive0')
-        self.assert_qmp(result, 'return', {})
+        self.pause_job('drive0')
 
-        time.sleep(1)
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
 
-        time.sleep(1)
+        time.sleep(0.5)
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return[0]/offset', offset)
 
@@ -896,14 +894,12 @@  class TestRepairQuorum(iotests.QMPTestCase):
                              target=quorum_repair_img, format=iotests.imgfmt)
         self.assert_qmp(result, 'return', {})
 
-        result = self.vm.qmp('block-job-pause', device='job0')
-        self.assert_qmp(result, 'return', {})
+        self.pause_job('job0')
 
-        time.sleep(1)
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
 
-        time.sleep(1)
+        time.sleep(0.5)
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return[0]/offset', offset)
 
diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index ba4da65..e1206ca 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -89,11 +89,12 @@  class TestSingleDrive(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         self.vm.resume_drive('drive0')
-        time.sleep(1)
+        self.pause_job('drive0')
+
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
 
-        time.sleep(1)
+        time.sleep(0.5)
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return[0]/offset', offset)
 
@@ -302,11 +303,12 @@  class TestSingleTransaction(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         self.vm.resume_drive('drive0')
-        time.sleep(1)
+        self.pause_job('drive0')
+
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
 
-        time.sleep(1)
+        time.sleep(0.5)
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return[0]/offset', offset)
 
@@ -529,11 +531,12 @@  class TestDriveCompression(iotests.QMPTestCase):
         self.assert_qmp(result, 'return', {})
 
         self.vm.resume_drive('drive0')
-        time.sleep(1)
+        self.pause_job('drive0')
+
         result = self.vm.qmp('query-block-jobs')
         offset = self.dictpath(result, 'return[0]/offset')
 
-        time.sleep(1)
+        time.sleep(0.5)
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return[0]/offset', offset)
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index abcf3c1..22439c4 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -27,6 +27,7 @@  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts'))
 import qtest
 import struct
 import json
+import signal
 
 
 # This will not work if arguments contain spaces but is necessary if we
@@ -137,6 +138,20 @@  def log(msg, filters=[]):
         msg = flt(msg)
     print msg
 
+class Timeout:
+    def __init__(self, seconds, errmsg = "Timeout"):
+        self.seconds = seconds
+        self.errmsg = errmsg
+    def __enter__(self):
+        signal.signal(signal.SIGALRM, self.timeout)
+        signal.setitimer(signal.ITIMER_REAL, self.seconds)
+        return self
+    def __exit__(self, type, value, traceback):
+        signal.setitimer(signal.ITIMER_REAL, 0)
+        return False
+    def timeout(self, signum, frame):
+        raise Exception(self.errmsg)
+
 class VM(qtest.QEMUQtestMachine):
     '''A QEMU VM'''
 
@@ -346,6 +361,18 @@  class QMPTestCase(unittest.TestCase):
         event = self.wait_until_completed(drive=drive)
         self.assert_qmp(event, 'data/type', 'mirror')
 
+    def pause_job(self, job_id='job0'):
+        result = self.vm.qmp('block-job-pause', device=job_id)
+        self.assert_qmp(result, 'return', {})
+
+        with Timeout(1, "Timeout waiting for job to pause"):
+            while True:
+                result = self.vm.qmp('query-block-jobs')
+                for job in result['return']:
+                    if job['device'] == job_id and job['paused'] == True and job['busy'] == False:
+                        return job
+
+
 def notrun(reason):
     '''Skip this test suite'''
     # Each test in qemu-iotests has a number ("seq")