qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Daniel Henrique Barboza
Subject: Re: [Qemu-devel] [PATCH v3 2/2] tests/qemu-iotests: adding savevm/loadvm with postcopy flag test
Date: Mon, 4 Dec 2017 13:15:37 -0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0



On 12/04/2017 01:06 PM, Max Reitz wrote:
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 <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?
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.
Hmmm that's a good approach too I guess. It is shorter, but has the
drawback of, in case of failure, we'll repeat the command 10 times
before failing.

I'll respin the patch with the first approach I had in mind. If anyone
has a good case to use this second approach I can respin it again.


Daniel



Max





reply via email to

[Prev in Thread] Current Thread [Next in Thread]