Re: [Qemu-devel] [PATCH v3 2/2] tests/qemu-iotests: adding savevm/loadvm with postcopy flag test

[Max Reitz]

On 2017-12-03 21:13, Daniel Henrique Barboza wrote:
> Hi Max,
> 
> On 12/01/2017 06:13 PM, Max Reitz wrote:
>> On 2017-11-16 23:35, Daniel Henrique Barboza wrote:
>>> This patch implements a test case for the scenario that was failing
>>> prior to the patch "migration/ram.c: do not set 'postcopy_running' in
>>> POSTCOPY_INCOMING_END".
>>>
>>> This new test file 198 was derived from the test file 181 authored
>>> by Kevin Wolf.
>>>
>>> CC: Kevin Wolf <address@hidden>
>>> CC: Max Reitz <address@hidden>
>>> CC: Cleber Rosa <address@hidden>
>>> Signed-off-by: Daniel Henrique Barboza <address@hidden>
>>>
>>> ---
>>> I CCed Cleber Rosa because this patch was developed at the same
>>> time his patch series "[PATCH 00/10] I/O tests cleanups" hit the
>>> mailing list. Most of the cleanups/fixes he made in the series was
>>> done in this new test as well.
>>>
>>>  tests/qemu-iotests/198     | 110 
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>  tests/qemu-iotests/198.out |  20 +++++++++
>>>  tests/qemu-iotests/group   |   1 +
>>>  3 files changed, 131 insertions(+)
>>>  create mode 100755 tests/qemu-iotests/198
>>>  create mode 100644 tests/qemu-iotests/198.out
>> Looks good overall, just two nitpicks below.
>>
>>> diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198
>>> new file mode 100755
>>> index 0000000000..786e37fc95
>>> --- /dev/null
>>> +++ b/tests/qemu-iotests/198
>>> @@ -0,0 +1,110 @@
>>> +#!/bin/bash
>>> +#
>>> +# Test savevm and loadvm after live migration with postcopy flag
>>> +#
>>> +# Copyright (C) 2017, IBM Corporation.
>>> +#
>>> +# This file is derived from tests/qemu-iotests/181 by Kevin Wolf
>>> +#
>>> +# This program is free software; you can redistribute it and/or modify
>>> +# it under the terms of the GNU General Public License as published by
>>> +# the Free Software Foundation; either version 2 of the License, or
>>> +# (at your option) any later version.
>>> +#
>>> +# This program is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> +# GNU General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU General Public License
>>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>>> +#
>>> +# Creator/Owner : address@hidden
>>> +
>>> +seq=`basename $0`
>>> +echo "QA output created by $seq"
>>> +
>>> +status=1   # failure is the default!
>>> +
>>> +MIG_SOCKET="${TEST_DIR}/migrate"
>>> +
>>> +# get standard environment, filters and checks
>>> +. ./common.rc
>>> +. ./common.filter
>>> +. ./common.qemu
>>> +
>>> +_cleanup()
>>> +{
>>> +    rm -f "${MIG_SOCKET}"
>>> +    _cleanup_test_img
>>> +    _cleanup_qemu
>>> +}
>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>> +
>>> +_supported_fmt qcow2
>>> +_supported_proto generic
>>> +_supported_os Linux
>>> +
>>> +size=64M
>>> +_make_test_img $size
>>> +
>>> +echo
>>> +echo === Starting VMs ===
>>> +echo
>>> +
>>> +qemu_comm_method="monitor"
>>> +
>>> +if [ "$IMGOPTSSYNTAX" = "true" ]; then
>>> +    _launch_qemu \
>>> +        -drive "${TEST_IMG}",cache=${CACHEMODE},id=disk
>>> +else
>>> +    _launch_qemu \
>>> +        -drive file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk
>>> +fi
>>> +src=$QEMU_HANDLE
>>> +
>>> +if [ "$IMGOPTSSYNTAX" = "true" ]; then
>>> +    _launch_qemu \
>>> +        -drive "${TEST_IMG}",cache=${CACHEMODE},id=disk \
>>> +        -incoming "unix:${MIG_SOCKET}"
>>> +else
>>> +    _launch_qemu \
>>> +        -drive 
>>> file="${TEST_IMG}",cache=${CACHEMODE},driver=$IMGFMT,id=disk \
>>> +        -incoming "unix:${MIG_SOCKET}"
>>> +fi
>>> +dest=$QEMU_HANDLE
>>> +
>>> +echo
>>> +echo === Set \'migrate_set_capability postcopy-ram on\' and migrate ===
>>> +echo
>>> +
>>> +silent=yes
>>> +_send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)"
>>> +_send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
>>> +_send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
>>> +
>>> +QEMU_COMM_TIMEOUT=1 qemu_cmd_repeat=10 silent=yes \
>>> +    _send_qemu_cmd $src "info migrate" "completed\|failed"
>>> +silent=yes _send_qemu_cmd $src "" "(qemu)"
>> Would it make sense to query the migration status non-silently here,
>> too, so that the test output would verify that it has actually succeeded?
>>
>> (I suppose the savevm/loadvm coming up next would fail if the migration
>> failed, but maybe if it's easy enough...)
> 
> Not exactly non-silently, but we can do something like this:
> 
> 
> echo
> echo === Check if migration was successful ===
> echo
> 
> QEMU_COMM_TIMEOUT=1 silent=yes \
>     _send_qemu_cmd $src "info migrate" "completed"
> silent=yes _send_qemu_cmd $src "" "(qemu)"
> 
> 
> If migration failed, the test will fail with timeout error waiting for
> "completed"
> prior to savevm. What do you think?

Sounds good.  Shouldn't you keep the qemu_cmd_repeat=10, though?

Max

signature.asc
Description: OpenPGP digital signature