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>]

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?


>
>> +
>> +echo
>> +echo === On destination, execute savevm and loadvm ===
>> +echo
>> +
>> +silent=
>> +_send_qemu_cmd $dest 'savevm state1"' "(qemu)"
>> +_send_qemu_cmd $dest 'loadvm state1"' "(qemu)"
> Any specific reason why the snapshot name has a trailing double quote?

No reason, just an ugly that slipped by. I'll remove it in the next version.



Thanks,


Daniel


>
> Max
>
>> +
>> +echo
>> +echo === Shut down and check image ===
>> +echo
>> +
>> +_send_qemu_cmd $src 'quit' ""
>> +_send_qemu_cmd $dest 'quit' ""
>> +wait=1 _cleanup_qemu
>> +
>> +_check_test_img
>> +
>> +# success, all done
>> +echo "*** done"
>> +status=0
>> diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
>> new file mode 100644
>> index 0000000000..6c27303857
>> --- /dev/null
>> +++ b/tests/qemu-iotests/198.out
>> @@ -0,0 +1,20 @@
>> +QA output created by 198
>> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>> +
>> +=== Starting VMs ===
>> +
>> +
>> +=== Set 'migrate_set_capability postcopy-ram on' and migrate ===
>> +
>> +
>> +=== On destination, execute savevm and loadvm ===
>> +
>> +(qemu) savevm state1"
>> +(qemu) loadvm state1"
>> +
>> +=== Shut down and check image ===
>> +
>> +(qemu) quit
>> +(qemu) quit
>> +No errors were found on the image.
>> +*** done
>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>> index 24e5ad1b79..84d082bc38 100644
>> --- a/tests/qemu-iotests/group
>> +++ b/tests/qemu-iotests/group
>> @@ -194,3 +194,4 @@
>>   194 rw auto migration quick
>>   195 rw auto quick
>>   197 rw auto quick
>> +198 rw auto migration
>>
>