Re: [Qemu-block] [Qemu-devel] [PATCH 3/3] qemu-iotests: Test exiting qemu with running job

[Eric Blake]

On 06/09/2017 06:50 AM, Kevin Wolf wrote:
> When qemu is exited, all running jobs should be cancelled successfully.
> This adds a test for this for all types of block jobs that currently
> exist in qemu.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  tests/qemu-iotests/185     | 189 
> +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/185.out |  59 ++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 249 insertions(+)
>  create mode 100755 tests/qemu-iotests/185
>  create mode 100644 tests/qemu-iotests/185.out
> 

> +
> +_send_qemu_cmd $h \
> +    "{ 'execute': 'human-monitor-command',
> +       'arguments': { 'command-line':
> +                      'qemu-io disk \"write 0 4M\"' } }" \
> +    "return"

My first reaction? "Why are we still dropping back to HMP?"  My second -
"Oh yeah - qemu-io is a debugging interface, and we really don't
need/want it in QMP".  This part is fine.

> +_send_qemu_cmd $h \
> +    "{ 'execute': 'drive-backup',
> +       'arguments': { 'device': 'disk',
> +                      'target': '$TEST_IMG.copy',
> +                      'format': '$IMGFMT',
> +                      'sync': 'full',
> +                      'speed': 65536 } }" \

Fun with slow speeds :)

64k is slow enough compared to your 4M chunk that you should be fairly
immune to a heavy load allowing the job to converge.  However,

> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN", "data": {"guest": false}}
> +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 
> 524288, "speed": 65536, "type": "commit"}}

I'm worried that if you don't sanitize at least offset, you will still
be prone to some race conditions changing the output.  You may want to
add in some additional filtering on the output to be safer.

Other than that, the patch looks good to me.

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

signature.asc
Description: OpenPGP digital signature