On 2017-12-04 16:01, Daniel Henrique Barboza wrote: > > > 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 <kw...@redhat.com> >>>>> CC: Max Reitz <mre...@redhat.com> >>>>> CC: Cleber Rosa <cr...@redhat.com> >>>>> Signed-off-by: Daniel Henrique Barboza <danie...@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 : danie...@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".
Ah, right. I thought you'd simply replace that by the other. But either way works for me. Max
signature.asc
Description: OpenPGP digital signature
