[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it
From: |
Kashyap Chamarthy |
Subject: |
Re: [Qemu-devel] [PATCH] qemu-iotests: Make test 192 use QMP; convert it to Python |
Date: |
Mon, 4 Sep 2017 15:28:59 +0200 |
User-agent: |
Mutt/1.6.0.1 (2016-04-01) |
On Mon, Sep 04, 2017 at 08:55:23PM +0800, Fam Zheng wrote:
> Hi Kashyap,
>
> On Mon, 09/04 13:16, Kashyap Chamarthy wrote:
[...]
> > +dest_vm = (iotests.VM('dest').add_drive(test_img_path)
> > + .add_incoming('defer'))
>
> Superfluous parenthesis?
Yes, the paranthesis can be removed and turn it into a single line:
dest_vm = iotests.VM('dest').add_drive(test_img_path).add_incoming('defer')
> > +dest_vm.launch()
> > +atexit.register(dest_vm.shutdown)
>
> As an improvement maybe you could rebase to Stefan's "iotests: clean up
> resources using context managers" series and switch to "with" for the temp
> file
> and VM.
Good point. I did notice that thread[*] about resource clean up. And I
see it's already applied to 'block-next'. Will rebase and change to the
'with' statement.
[*] https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg04639.html
> Otherwise looks good to me.
Thanks for the quick review!
[...]
--
/kashyap