Re: [Qemu-block] [PATCH 09/14] qemu-iotests: Rewrite 207 for blockdev-create job

[Max Reitz]

On 2018-05-25 18:33, Kevin Wolf wrote:
> This rewrites the test case 207 to work with the new x-blockdev-create
> job rather than the old synchronous version of the command.
> 
> Most of the test cases stay the same as before (the exception being some
> improved 'size' options that allow distinguishing which command created
> the image), but in order to be able to implement proper job handling,
> the test case is rewritten in Python.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  tests/qemu-iotests/207        | 435 
> +++++++++++++++++++-----------------------
>  tests/qemu-iotests/207.out    |  89 +++++----
>  tests/qemu-iotests/group      |   6 +-
>  tests/qemu-iotests/iotests.py |  23 ++-
>  4 files changed, 264 insertions(+), 289 deletions(-)
> 
> diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207
> index f5c77852d1..91c1f7e811 100755
> --- a/tests/qemu-iotests/207
> +++ b/tests/qemu-iotests/207

[...]

> +    #
> +    # Test host-key-check options
> +    #
> +    iotests.log("=== Test host-key-check options ===")
> +    iotests.log("")

[...]

> +    md5_key = subprocess.check_output(
> +        'ssh-keyscan -t rsa 127.0.0.1 2>/dev/null | grep -v "\\^#" | ' +
> +        'cut -d" " -f3 | base64 -d | md5sum -b | cut -d" " -f1',
> +        shell=True).rstrip()
> +
> +    vm.launch()
> +    blockdev_create(vm, { 'driver': 'ssh',
> +                          'location': {
> +                              'path': disk_path,
> +                              'server': {
> +                                  'host': '127.0.0.1',
> +                                  'port': '22'
> +                              },
> +                              'host-key-check': {
> +                                  'mode': 'hash',
> +                                  'type': 'md5',
> +                                  'hash': 'wrong',
> +                              }
> +                          },
> +                          'size': 2097152 })

Technically a change from before where it was 8M, but not a change I'm
opposed to.

[...]

> +    #
> +    # Invalid path and user
> +    #
> +    iotests.log("=== Invalid path and user ===")
> +    iotests.log("")
> +
> +    vm.launch()
> +    blockdev_create(vm, { 'driver': 'ssh',
> +                          'location': {
> +                              'path': '/this/is/not/an/existing/path',
> +                              'server': {
> +                                  'host': '127.0.0.1',
> +                                  'port': '22'
> +                              },
> +                              'host-key-check': {
> +                                  'mode': 'none'
> +                              }
> +                          },
> +                          'size': 4194304 })
> +    blockdev_create(vm, { 'driver': 'ssh',
> +                          'location': {
> +                              'path': disk_path,
> +                              'user': 'invalid user',
> +                              'server': {
> +                                  'host': '127.0.0.1',
> +                                  'port': '22'
> +                              },
> +                              'host-key-check': {
> +                                  'mode': 'none'
> +                              }
> +                          },
> +                          'size': 4194304 })

Technical changes again (the previous test didn't have host-key-check),
but these are good changes.

[...]

> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index f0f4ef32f0..e945caa6bb 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -109,9 +109,11 @@ def qemu_img_pipe(*args):
>          sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' 
> '.join(qemu_img_args + list(args))))
>      return subp.communicate()[0]
>  
> -def img_info_log(filename):
> +def img_info_log(filename, filter_path=None):
>      output = qemu_img_pipe('info', '-f', imgfmt, filename)
> -    log(filter_img_info(output, filename))
> +    if not filter_path:
> +        filter_path = filename
> +    log(filter_img_info(output, filter_path))
>  
>  def qemu_io(*args):
>      '''Run qemu-io and return the stdout data'''
> @@ -301,6 +303,13 @@ def file_path(*names):
>  
>      return paths[0] if len(paths) == 1 else paths
>  
> +def remote_filename(path):

I don't really understand why you have two patches in this series to add
functions to iotests.py, but then you keep on adding more functions in
other patches on the side.

> +    if imgproto == 'file':
> +        return imgproto

Shouldn't this be path?

With that fixed:

Reviewed-by: Max Reitz <address@hidden>

> +    elif imgproto == 'ssh':
> +        return "ssh://127.0.0.1%s" % (path)
> +    else:
> +        raise Exception("Protocol %s not supported" % (imgproto))
>  
>  class VM(qtest.QEMUQtestMachine):
>      '''A QEMU VM'''
> @@ -595,6 +604,16 @@ def verify_image_format(supported_fmts=[], 
> unsupported_fmts=[]):
>      if not_sup or (imgfmt in unsupported_fmts):
>          notrun('not suitable for this image format: %s' % imgfmt)
>  
> +def verify_protocol(supported=[], unsupported=[]):
> +    assert not (supported and unsupported)
> +
> +    if 'generic' in supported:
> +        return
> +
> +    not_sup = supported and (imgproto not in supported)
> +    if not_sup or (imgproto in unsupported):
> +        notrun('not suitable for this protocol: %s' % imgproto)
> +
>  def verify_platform(supported_oses=['linux']):
>      if True not in [sys.platform.startswith(x) for x in supported_oses]:
>          notrun('not suitable for this OS: %s' % sys.platform)
>

signature.asc
Description: OpenPGP digital signature