Re: [Qemu-block] [PATCH v2 4/4] qemu-iotests: Block migration test

[Eric Blake]

On 05/30/2017 10:22 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  tests/qemu-iotests/183     | 143 
> +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/183.out |  46 +++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 190 insertions(+)
>  create mode 100755 tests/qemu-iotests/183
>  create mode 100644 tests/qemu-iotests/183.out
> 

In addition to Jeff's review comments:

> +
> +_cleanup()
> +{
> +    rm -f "${MIG_SOCKET}"
> +    rm -f "${TEST_IMG}.dest"
> +     _cleanup_test_img
> +    _cleanup_qemu

Odd indentation.

> +echo
> +echo === Do block migration to destination ===
> +echo
> +
> +reply="$(_send_qemu_cmd $src \
> +    "{ 'execute': 'migrate',
> +       'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \
> +    'return\|error')"
> +echo "$reply"
> +if echo "$reply" | grep "compiled without old-style" > /dev/null; then
> +    _notrun "migrate -b support not compiled in"
> +fi

Oh cool - I didn't realize that _notrun already existed as our way to
skip a test, even if we've already produced unexpected output (if we
ever want to play with exit status 77 in the future for skipped tests,
then _notrun would be a great place to centralize that).

> +
> +while _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" "return"  |
> +      grep '"status": "active"' > /dev/null
> +do
> +    sleep 0.1

More interesting indentation (I would have probably stuck to 4 spaces on
both lines)

With the nits fixed,
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature