[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 37/37] iotests: Add test for change-related Q

From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v2 37/37] iotests: Add test for change-related QMP commands
Date: Tue, 10 Feb 2015 15:37:07 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0

On 2015-02-09 at 19:06, Eric Blake wrote:
On 02/09/2015 10:11 AM, Max Reitz wrote:
Signed-off-by: Max Reitz <address@hidden>
  tests/qemu-iotests/118     | 653 +++++++++++++++++++++++++++++++++++++++++++++
  tests/qemu-iotests/118.out |   5 +
  tests/qemu-iotests/group   |   1 +
  3 files changed, 659 insertions(+)
  create mode 100755 tests/qemu-iotests/118
  create mode 100644 tests/qemu-iotests/118.out

+    def process_events(self):
+        for event in self.vm.get_qmp_events(wait=True):
+            if event['event'] == 'DEVICE_TRAY_MOVED' and 
event['data']['device'] == 'drive0':
Long line; I think you can wrap it as:

if (event['event'] == 'DEVICE_TRAY_MOVED' and
     event['data']['device'] == 'drive0'):

If that works, I'll be glad to (if I have to respin, which I don't deem too unlikely).

+class GeneralChangeTestsBaseClass(ChangeBaseClass):
+    def test_change(self):
+        result = self.vm.qmp('change', device='drive0', target=new_img,
+                                       arg=iotests.imgfmt)
Unusual indentation. [1]

+        self.assert_qmp(result, 'return', {})
+        while not self.has_opened:
+            self.process_events()
+        while not self.has_closed:
+            self.process_events()
Are we guaranteed that loops like this will gracefully timeout and fail
the test if the event doesn't happen, instead of hanging forever?

Well, let's say it should be guaranteed, but this is a test so it may break. I will try adding a timeout if I respin, though.

that's probably something affecting multiple tests, so I won't hold up
review of this one.

+        result = self.vm.qmp('query-block')
+        self.assert_qmp(result, 'return[0]/tray_open', False)
+        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
+    def test_blockdev_change_medium(self):
+        result = self.vm.qmp('blockdev-change-medium', device='drive0',
+                                                       filename=new_img,
+                                                       format=iotests.imgfmt)
[1] I guess your choice of not flushing arguments to the ( is intentional.

The long line thing is minor, so:
Reviewed-by: Eric Blake <address@hidden>

Once again, thank you!


reply via email to

[Prev in Thread] Current Thread [Next in Thread]