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

[Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>]

On 12/04/2017 12:50 PM, Max Reitz wrote:
> 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 <kwolf@redhat.com>
>>>> CC: Max Reitz <mreitz@redhat.com>
>>>> CC: Cleber Rosa <crosa@redhat.com>
>>>> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
>>>>
>>>> ---
>>>> 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 : danielhb@linux.vnet.ibm.com
>>>> +
>>>> +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?

I don't think it's needed. The command prior to it was:

+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)"


So it's guaranteed that the next 'info migrate' will contain the string
"completed" or "failed".


Daniel


>
> Max
>