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
