On 02/09/2015 10:11 AM, Max Reitz wrote: > Signed-off-by: Max Reitz > --- > 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'): > +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? But 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 -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org