From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34428) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLHYa-0007jU-E3 for qemu-devel@nongnu.org; Tue, 10 Feb 2015 15:37:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLHYV-00036o-Gk for qemu-devel@nongnu.org; Tue, 10 Feb 2015 15:37:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59835) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLHYV-00036P-5T for qemu-devel@nongnu.org; Tue, 10 Feb 2015 15:37:11 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1AKbAKG031572 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 10 Feb 2015 15:37:10 -0500 Message-ID: <54DA6BF3.8060004@redhat.com> Date: Tue, 10 Feb 2015 15:37:07 -0500 From: Max Reitz MIME-Version: 1.0 References: <1423501897-30410-1-git-send-email-mreitz@redhat.com> <1423501897-30410-38-git-send-email-mreitz@redhat.com> <54D94B6B.1020004@redhat.com> In-Reply-To: <54D94B6B.1020004@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 37/37] iotests: Add test for change-related QMP commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , John Snow , Markus Armbruster , Stefan Hajnoczi 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 >> --- >> 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. > 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 Once again, thank you! Max