Re: [Qemu-block] [PATCH 10/10] qemu-iotests/118: Test media change with qdev name

[Eric Blake]

On 08/19/2016 11:50 AM, Kevin Wolf wrote:
> We just added the option to use qdev device names in all device related
> block QMP commands. This patch converts some of the test cases in 118 to
> use qdev device names instead of BlockBackend names to cover the new
> way. It converts cases for each of the media change commands, but only
> for CD-ROM and not everywhere, so that the old way is still tested, too.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  tests/qemu-iotests/118        | 85 
> ++++++++++++++++++++++++++++++++++---------
>  tests/qemu-iotests/iotests.py |  5 +++
>  2 files changed, 73 insertions(+), 17 deletions(-)
> 

> @@ -76,9 +79,15 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
>          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)
> +        if self.device_name is not None:
> +            result = self.vm.qmp('blockdev-change-medium',
> +                                 id=self.device_name, filename=new_img,
> +                                 format=iotests.imgfmt)
> +        else:
> +            result = self.vm.qmp('blockdev-change-medium',
> +                                 device='drive0', filename=new_img,
> +                                 format=iotests.imgfmt)

I'm not enough of a python guru to know if there is any way to compress
this to a shorter format (I do know, however, that the lack of an
obvious ?: operator in python can indeed result in verbose if/else
clauses compared to other languages).

At any rate, the ultimate test is whether the change still passes; and
looks like you have good coverage of using exactly one or the other
argument.  Do you also want to add tests (either here, or in 11/10) that
validate that providing neither 'device' nor 'id' gives a sane error,
likewise that providing both has sane behavior?  (For now, our behavior
is that we fail, although it could also be argued that sane behavior
would validate that 'id' happens to be currently in use by 'device' and
only fail if they are not pointing to the same backend).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature