qemu-block
[Top][All Lists]
Advanced

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

Re: [Qemu-block] [PATCH] qemu-iotests: Make test 192 use QMP; convert it


From: Kashyap Chamarthy
Subject: Re: [Qemu-block] [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



reply via email to

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