Add common QEMU control functionality to qemu-iotests
cover Add common QEMU control functionality to qemu-iotests
This adds some common functionality to control QEMU for qemu-iotests.
Additionally, test 085 is updated to use this new functionality.
Some minor fixups along the way, to clear up spaced pathname issues, for common.rc, test 019, and test 086.
Jeff Cody (4):
block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
block: qemu-iotests - update 085 to use common.qemu
block: qemu-iotests - fix image cleanup when using spaced pathnames
block: qemu-iotests: make test 019 and 086 work with spaced pathnames
tests/qemu-iotests/019 | 2 +- tests/qemu-iotests/085 | 73 +++--------------- tests/qemu-iotests/086 | 8 +- tests/qemu-iotests/common.qemu | 164 +++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/common.rc | 4 +- 5 files changed, 183 insertions(+), 68 deletions(-) create mode 100644 tests/qemu-iotests/common.qemu
Signature · 1 line
1.8.3.1
PATCH 1/4 block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
This creates some common functions for bash language qemu-iotests to control, and communicate with, a running QEMU process.
4 functions are introduced:
1. _launch_qemu()
This launches the QEMU process(es), and sets up the file descriptors and fifos for communication. You can choose to launch each QEMU process listening for either QMP or HMP monitor. You can call this function multiple times, and save the handle returned from each.
Commands 2 and 3 use the handle received from _launch_qemu(), to talk to the appropriate process.
2. _send_qemu_cmd()
Sends a command string, specified by $2, to QEMU. If $2 is non-NULL, will wait for it as the required resulting. Failure to receive $3 will cause the test to fail.
3. _timed_wait_for()
Waits for a response, for up to a default of 10 seconds. If $2 is not seen in that time (anywhere in the response), then the test fails. Primarily used by _send_qemu_cmd, but could be useful standalone, as well.
4. _cleanup_qemu()
Kills the running QEMU processes, and removes the fifos.
--- tests/qemu-iotests/common.qemu | 164 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 tests/qemu-iotests/common.qemu
tests/qemu-iotests/common.qemu new file +164 -0
@@ -0,0 +1,164 @@ +#!/bin/bash +# +# This allows for launching of multiple QEMU instances, with independent +# communication possible to each instance. +# +# Each instance can choose, at launch, to use either the QMP or the +# HMP (monitor) interface. +# +# All instances are cleaned up via _cleanup_qemu, including killing the +# running qemu instance. +# +# Copyright (C) 2014 Red Hat, Inc. +# +# 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/>. +# + +QEMU_COMM_TIMEOUT=10 + +QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$" +QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$" + +QEMU_PID= +_QEMU_HANDLE=0 +QEMU_HANDLE=0 + +# If bash version is >= 4.1, these will be overwritten and dynamic +# file descriptor values assigned. +_out_fd=3 +_in_fd=4 + +# Wait for expected QMP response from QEMU. Will time out +# after 10 seconds, which counts as failure. +# +# Override QEMU_COMM_TIMEOUT for a timeout different than the +# default 10 seconds +# +# $1: The handle to use +# $2+ All remaining arguments comprise the string to search for +# in the response. +# +# If $silent is set to anything but an empty string, then +# response is not echoed out. +function _timed_wait_for() +{ + local h=${1} + shift + while read -t ${QEMU_COMM_TIMEOUT} resp <&${QEMU_OUT[$h]} + do + if [ -z "${silent}" ]; then + echo "${resp}" | _filter_testdir | _filter_qemu + fi + grep -q "${*}" < <(echo ${resp}) + if [ $? -eq 0 ]; then + return + fi + done + echo "Timeout waiting for ${*} on handle ${h}" + exit 1 # Timeout means the test failed +} + + +# Sends QMP or HMP command to QEMU, and waits for the expected response +# +# $1: QEMU handle to use +# $2: String of the QMP command to send +# ${@: -1} (Last string passed) +# String that the QEMU response should contain. If $2 is a null +# string, do not wait for a response +function _send_qemu_cmd() +{ + local h=${1} + shift + # This array element extraction is done to accomodate pathnames with spaces + echo "${@: 1:${#@}-1}" >&${QEMU_IN[${h}]} + shift + + if [ -n "${1}" ] + then + _timed_wait_for ${h} "${@: -1}" + fi +} + + +# Launch a QEMU process. +# +# Input parameters: +# $qemu_comm_method: set this variable to 'monitor' (case insensitive) +# to use the QEMU HMP monitor for communication. +# Otherwise, the default of QMP is used. +# Returns: +# $QEMU_HANDLE: set to a handle value to communicate with this QEMU instance. +# +function _launch_qemu() +{ + local comm= + local fifo_out= + local fifo_in= + + if (shopt -s nocasematch; [[ "${qemu_comm_method}" == "monitor" ]]) + then + comm="-monitor stdio -qmp none" + else + local qemu_comm_method="qmp" + comm="-monitor none -qmp stdio" + fi + + fifo_out=${QEMU_FIFO_OUT}_${_QEMU_HANDLE} + fifo_in=${QEMU_FIFO_IN}_${_QEMU_HANDLE} + mkfifo "${fifo_out}" + mkfifo "${fifo_in}" + + "${QEMU}" -nographic -serial none ${comm} "${@}" 2>&1 \ + >"${fifo_out}" \ + <"${fifo_in}" & + QEMU_PID[${_QEMU_HANDLE}]=$! + + if [ "${BASH_VERSINFO[0]}" -ge "4" ] && [ "${BASH_VERSINFO[1]}" -ge "1" ] + then + # bash >= 4.1 required for automatic fd + exec {_out_fd}<"${fifo_out}" + exec {_in_fd}>"${fifo_in}" + else + let _out_fd++ + let _in_fd++ + eval "exec ${_out_fd}<'${fifo_out}'" + eval "exec ${_in_fd}>'${fifo_in}'" + fi + + QEMU_OUT[${_QEMU_HANDLE}]=${_out_fd} + QEMU_IN[${_QEMU_HANDLE}]=${_in_fd} + + if [ "${qemu_comm_method}" == "qmp" ] + then + # Don't print response, since it has version information in it + silent=yes _timed_wait_for ${_QEMU_HANDLE} "capabilities" + fi + QEMU_HANDLE=${_QEMU_HANDLE} + let _QEMU_HANDLE++ +} + + +# Silenty kills the QEMU process +function _cleanup_qemu() +{ + # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices + for i in "${!QEMU_OUT[@]}" + do + kill -KILL ${QEMU_PID[$i]} + wait ${QEMU_PID[$i]} 2>/dev/null # silent kill + rm -f "${QEMU_FIFO_IN}_${i}" "${QEMU_FIFO_OUT}_${i}" + done +} +
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
Quoted 12 lines · Jeff Cody
This creates some common functions for bash language qemu-iotests to control, and communicate with, a running QEMU process.
4 functions are introduced:
1.
_launch_qemu()
This launches the QEMU process(es), and sets up the file descriptors and fifos for communication. You can choose to launch each QEMU process listening for either QMP or HMP monitor. You can call this function multiple times, and save the handle returned from each.
Quoted 6 lines
Commands 2 and 3 use the handle received from
_launch_qemu(), to talk to the appropriate process.2.
_send_qemu_cmd()
Sends a command string, specified by $2, to QEMU. If $2 is non-NULL, will wait for it as the required resulting. Failure
"will wait for it as the required resulting" I don't understand this part of the sentence, probably because I am not a native speaker.
Quoted 103 lines
to receive $3 will cause the test to fail.
3.
_timed_wait_for()
Waits for a response, for up to a default of 10 seconds. If $2 is not seen in that time (anywhere in the response), then the test fails. Primarily used by_send_qemu_cmd, but could be useful standalone, as well.4.
_cleanup_qemu()
Kills the running QEMU processes, and removes the fifos.Signed-off-by Jeff Cody <[email protected]>--- tests/qemu-iotests/common.qemu | 164 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 tests/qemu-iotests/common.qemutests/qemu-iotests/common.qemu new file +80
-0@@ -0,0 +1,164 @@ +#!/bin/bash +# +# This allows for launching of multiple QEMU instances, with independent +# communication possible to each instance. +# +# Each instance can choose, at launch, to use either the QMP or the +# HMP (monitor) interface. +# +# All instances are cleaned up via _cleanup_qemu, including killing the +# running qemu instance. +# +# Copyright (C) 2014 Red Hat, Inc. +# +# 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/>. +# + +QEMU_COMM_TIMEOUT=10 + +QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$" +QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$" + +QEMU_PID= +_QEMU_HANDLE=0 +QEMU_HANDLE=0 + +# If bash version is >= 4.1, these will be overwritten and dynamic +# file descriptor values assigned. +_out_fd=3 +_in_fd=4 + +# Wait for expected QMP response from QEMU. Will time out +# after 10 seconds, which counts as failure. +# +# Override QEMU_COMM_TIMEOUT for a timeout different than the +# default 10 seconds +# +# $1: The handle to use +# $2+ All remaining arguments comprise the string to search for +# in the response. +# +# If $silent is set to anything but an empty string, then +# response is not echoed out. +function _timed_wait_for() +{ + local h=${1} + shift + while read -t ${QEMU_COMM_TIMEOUT} resp <&${QEMU_OUT[$h]} + do + if [ -z "${silent}" ]; then + echo "${resp}" | _filter_testdir | _filter_qemu + fi + grep -q "${*}" < <(echo ${resp}) + if [ $? -eq 0 ]; then + return + fi + done + echo "Timeout waiting for ${*} on handle ${h}" + exit 1 # Timeout means the test failed +} + + +# Sends QMP or HMP command to QEMU, and waits for the expected response +# +# $1: QEMU handle to use +# $2: String of the QMP command to send +# ${@: -1} (Last string passed) +# String that the QEMU response should contain. If $2 is a null +# string, do not wait for a response +function _send_qemu_cmd()
tests/qemu-iotests/common.qemu line 90
Quoted 10 lines
tests/qemu-iotests/common.qemu new file +10
-0+{ + local h=${1} + shift + # This array element extraction is done to accomodate pathnames with spaces + echo "${@: 1:${#@}-1}" >&${QEMU_IN[${h}]} + shift + + if [ -n "${1}" ] + then + _timed_wait_for ${h} "${@: -1}"
You have done shift before this. Aren't ${*} the remaining strings to wait for ?
Quoted 32 lines
tests/qemu-iotests/common.qemu new file +32
-0+ fi +} + + +# Launch a QEMU process. +# +# Input parameters: +# $qemu_comm_method: set this variable to 'monitor' (case insensitive) +# to use the QEMU HMP monitor for communication. +# Otherwise, the default of QMP is used. +# Returns: +# $QEMU_HANDLE: set to a handle value to communicate with this QEMU instance. +# +function _launch_qemu() +{ + local comm= + local fifo_out= + local fifo_in= + + if (shopt -s nocasematch; [[ "${qemu_comm_method}" == "monitor" ]]) + then + comm="-monitor stdio -qmp none" + else + local qemu_comm_method="qmp" + comm="-monitor none -qmp stdio" + fi + + fifo_out=${QEMU_FIFO_OUT}_${_QEMU_HANDLE} + fifo_in=${QEMU_FIFO_IN}_${_QEMU_HANDLE} + mkfifo "${fifo_out}" + mkfifo "${fifo_in}" +
tests/qemu-iotests/common.qemu line 132
Quoted 10 lines
tests/qemu-iotests/common.qemu new file +10
-0+ "${QEMU}" -nographic -serial none ${comm} "${@}" 2>&1 \ + >"${fifo_out}" \ + <"${fifo_in}" & + QEMU_PID[${_QEMU_HANDLE}]=$! + + if [ "${BASH_VERSINFO[0]}" -ge "4" ] && [ "${BASH_VERSINFO[1]}" -ge "1" ] + then + # bash >= 4.1 required for automatic fd + exec {_out_fd}<"${fifo_out}" + exec {_in_fd}>"${fifo_in}"
Isn't it ${_out_fd} and ${_in_fd} ?
Quoted 35 lines
tests/qemu-iotests/common.qemu new file +32
-0+ else + let _out_fd++ + let _in_fd++ + eval "exec ${_out_fd}<'${fifo_out}'" + eval "exec ${_in_fd}>'${fifo_in}'" + fi + + QEMU_OUT[${_QEMU_HANDLE}]=${_out_fd} + QEMU_IN[${_QEMU_HANDLE}]=${_in_fd} + + if [ "${qemu_comm_method}" == "qmp" ] + then + # Don't print response, since it has version information in it + silent=yes _timed_wait_for ${_QEMU_HANDLE} "capabilities" + fi + QEMU_HANDLE=${_QEMU_HANDLE} + let _QEMU_HANDLE++ +} + + +# Silenty kills the QEMU process +function _cleanup_qemu() +{ + # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices + for i in "${!QEMU_OUT[@]}" + do + kill -KILL ${QEMU_PID[$i]} + wait ${QEMU_PID[$i]} 2>/dev/null # silent kill + rm -f "${QEMU_FIFO_IN}_${i}" "${QEMU_FIFO_OUT}_${i}" + done +} +Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
Quoted 16 lines · Benoît Canet
Quoted 15 lines · Jeff Cody
This creates some common functions for bash language qemu-iotests to control, and communicate with, a running QEMU process.
4 functions are introduced:
1.
_launch_qemu()
This launches the QEMU process(es), and sets up the file descriptors and fifos for communication. You can choose to launch each QEMU process listening for either QMP or HMP monitor. You can call this function multiple times, and save the handle returned from each.Commands 2 and 3 use the handle received from
_launch_qemu(), to talk to the appropriate process.
Quoted 7 lines
Quoted 3 lines
2.
_send_qemu_cmd()
Sends a command string, specified by $2, to QEMU. If $2 is non-NULL, will wait for it as the required resulting. Failure"will wait for it as the required resulting" I don't understand this part of the sentence, probably because I am not a native speaker.
I wrote that sentence, and I don't understand it either :) I think I merged two sentences inadvertently.
Here is what I meant:
If $2 is non-NULL, _send_qemu_cmd() will wait to receive $2 as a required result string from QEMU.
Quoted 93 lines
Quoted 93 lines
to receive $3 will cause the test to fail.
3.
_timed_wait_for()
Waits for a response, for up to a default of 10 seconds. If $2 is not seen in that time (anywhere in the response), then the test fails. Primarily used by_send_qemu_cmd, but could be useful standalone, as well.4.
_cleanup_qemu()
Kills the running QEMU processes, and removes the fifos.Signed-off-by Jeff Cody <[email protected]>--- tests/qemu-iotests/common.qemu | 164 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 tests/qemu-iotests/common.qemutests/qemu-iotests/common.qemu new file +70
-0@@ -0,0 +1,164 @@ +#!/bin/bash +# +# This allows for launching of multiple QEMU instances, with independent +# communication possible to each instance. +# +# Each instance can choose, at launch, to use either the QMP or the +# HMP (monitor) interface. +# +# All instances are cleaned up via _cleanup_qemu, including killing the +# running qemu instance. +# +# Copyright (C) 2014 Red Hat, Inc. +# +# 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/>. +# + +QEMU_COMM_TIMEOUT=10 + +QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$" +QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$" + +QEMU_PID= +_QEMU_HANDLE=0 +QEMU_HANDLE=0 + +# If bash version is >= 4.1, these will be overwritten and dynamic +# file descriptor values assigned. +_out_fd=3 +_in_fd=4 + +# Wait for expected QMP response from QEMU. Will time out +# after 10 seconds, which counts as failure. +# +# Override QEMU_COMM_TIMEOUT for a timeout different than the +# default 10 seconds +# +# $1: The handle to use +# $2+ All remaining arguments comprise the string to search for +# in the response. +# +# If $silent is set to anything but an empty string, then +# response is not echoed out. +function _timed_wait_for() +{ + local h=${1} + shift + while read -t ${QEMU_COMM_TIMEOUT} resp <&${QEMU_OUT[$h]} + do + if [ -z "${silent}" ]; then + echo "${resp}" | _filter_testdir | _filter_qemu + fi + grep -q "${*}" < <(echo ${resp}) + if [ $? -eq 0 ]; then + return + fi + done + echo "Timeout waiting for ${*} on handle ${h}" + exit 1 # Timeout means the test failed +}
Quoted 8 lines
tests/qemu-iotests/common.qemu line 78
Quoted 8 lines
tests/qemu-iotests/common.qemu new file +8
-0+ + +# Sends QMP or HMP command to QEMU, and waits for the expected response +# +# $1: QEMU handle to use +# $2: String of the QMP command to send +# ${@: -1} (Last string passed) +# String that the QEMU response should contain. If $2 is a null
^^^
This is a typo, it should say "$3" ----------------------------------|
Quoted 6 lines
Quoted 6 lines
tests/qemu-iotests/common.qemu new file +6
-0+# string, do not wait for a response +function _send_qemu_cmd() +{ + local h=${1} + shift + # This array element extraction is done to accomodate pathnames with spaces
Quoted 9 lines
tests/qemu-iotests/common.qemu line 90
Quoted 6 lines
tests/qemu-iotests/common.qemu new file +6
-0+ echo "${@: 1:${#@}-1}" >&${QEMU_IN[${h}]} + shift + + if [ -n "${1}" ] + then + _timed_wait_for ${h} "${@: -1}"You have done shift before this. Aren't ${*} the remaining strings to wait for ?
I could probably get rid of the 2nd shift, although I would have to adjust the conditional below.
I do ${@: -1} because I want the very last whole string to be the item to wait for - this is only needed to accommodate pathnames with spaces inside the QMP string.
The value of ${@: -1} should be the same regardless of the shift.
Actually, there is a subtle bug here - the intent was to allow the 3rd argument to be a NULL string, to not wait for any response. If we have spaced pathnames, after the shift ${1} could still be inadvertently non-NULL. The easiest fix will probably just be to add a function _send_qemu_cmd_nowait(), or perhaps a variable to check (e.g. qemu_nowait)
Quoted 36 lines
Quoted 36 lines
tests/qemu-iotests/common.qemu new file +36
-0+ fi +} + + +# Launch a QEMU process. +# +# Input parameters: +# $qemu_comm_method: set this variable to 'monitor' (case insensitive) +# to use the QEMU HMP monitor for communication. +# Otherwise, the default of QMP is used. +# Returns: +# $QEMU_HANDLE: set to a handle value to communicate with this QEMU instance. +# +function _launch_qemu() +{ + local comm= + local fifo_out= + local fifo_in= + + if (shopt -s nocasematch; [[ "${qemu_comm_method}" == "monitor" ]]) + then + comm="-monitor stdio -qmp none" + else + local qemu_comm_method="qmp" + comm="-monitor none -qmp stdio" + fi + + fifo_out=${QEMU_FIFO_OUT}_${_QEMU_HANDLE} + fifo_in=${QEMU_FIFO_IN}_${_QEMU_HANDLE} + mkfifo "${fifo_out}" + mkfifo "${fifo_in}" + + "${QEMU}" -nographic -serial none ${comm} "${@}" 2>&1 \ + >"${fifo_out}" \ + <"${fifo_in}" & + QEMU_PID[${_QEMU_HANDLE}]=$!
Quoted 9 lines
tests/qemu-iotests/common.qemu line 132
Quoted 6 lines
tests/qemu-iotests/common.qemu new file +6
-0+ + if [ "${BASH_VERSINFO[0]}" -ge "4" ] && [ "${BASH_VERSINFO[1]}" -ge "1" ] + then + # bash >= 4.1 required for automatic fd + exec {_out_fd}<"${fifo_out}" + exec {_in_fd}>"${fifo_in}"Isn't it ${_out_fd} and ${_in_fd} ?
No, when doing the dynamic file descriptor assignment the '$' is left off - think of it as assigning a value to a variable (that is essentially what is happening).
More info: http://www.gnu.org/software/bash/manual/bashref.html#Redirections
Quoted 35 lines
Quoted 35 lines
tests/qemu-iotests/common.qemu new file +32
-0+ else + let _out_fd++ + let _in_fd++ + eval "exec ${_out_fd}<'${fifo_out}'" + eval "exec ${_in_fd}>'${fifo_in}'" + fi + + QEMU_OUT[${_QEMU_HANDLE}]=${_out_fd} + QEMU_IN[${_QEMU_HANDLE}]=${_in_fd} + + if [ "${qemu_comm_method}" == "qmp" ] + then + # Don't print response, since it has version information in it + silent=yes _timed_wait_for ${_QEMU_HANDLE} "capabilities" + fi + QEMU_HANDLE=${_QEMU_HANDLE} + let _QEMU_HANDLE++ +} + + +# Silenty kills the QEMU process +function _cleanup_qemu() +{ + # QEMU_PID[], QEMU_IN[], QEMU_OUT[] all use same indices + for i in "${!QEMU_OUT[@]}" + do + kill -KILL ${QEMU_PID[$i]} + wait ${QEMU_PID[$i]} 2>/dev/null # silent kill + rm -f "${QEMU_FIFO_IN}_${i}" "${QEMU_FIFO_OUT}_${i}" + done +} +Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
Quoted 6 lines · Jeff Cody
Quoted 5 lines
tests/qemu-iotests/common.qemu line 90
Quoted 2 lines
tests/qemu-iotests/common.qemu new file +2
-0+ then + _timed_wait_for ${h} "${@: -1}"You have done shift before this. Aren't ${*} the remaining strings to wait for ?
Quoted 6 lines
I could probably get rid of the 2nd shift, although I would have to adjust the conditional below.
I do ${@: -1} because I want the very last whole string to be the item to wait for - this is only needed to accommodate pathnames with spaces inside the QMP string.
${@: -1} is not portable:
$ bash -c 'set 1 2 3; echo ${@: -1}'
3
$ dash -c 'set 1 2 3; echo ${@: -1}'
dash: 1: Bad substitutionIf you want the last argument, you'll have to do something hideous like:
eval \${$#}
Short of using eval, there is no portable way to get at the last positional argument in dash.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
Quoted 9 lines · Eric Blake
$ dash -c 'set 1 2 3; echo ${@: -1}' dash: 1: Bad substitutionIf you want the last argument, you'll have to do something hideous like:
eval \${$#}
Short of using eval, there is no portable way to get at the last positional argument in dash.
If you are sure you don't need the other positional arguments, you could avoid eval with:
shift $(($# - 1)); echo $1
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
In reply to Eric BlakeQuoted 22 lines · Eric Blake
Quoted 6 lines · Jeff Cody
Quoted 5 lines
tests/qemu-iotests/common.qemu line 90
Quoted 2 lines
tests/qemu-iotests/common.qemu new file +2
-0+ then + _timed_wait_for ${h} "${@: -1}"You have done shift before this. Aren't ${*} the remaining strings to wait for ?
Quoted 6 lines
I could probably get rid of the 2nd shift, although I would have to adjust the conditional below.
I do ${@: -1} because I want the very last whole string to be the item to wait for - this is only needed to accommodate pathnames with spaces inside the QMP string.
${@: -1} is not portable:
$ bash -c 'set 1 2 3; echo ${@: -1}' 3 $ dash -c 'set 1 2 3; echo ${@: -1}' dash: 1: Bad substitution
Quoted 7 lines
If you want the last argument, you'll have to do something hideous like:
eval \${$#}
Short of using eval, there is no portable way to get at the last positional argument in dash.
Yes, and there are likely other bash-isms in some of the shell scripts in qemu-iotests. Since #!/bin/bash is explicitly specified, it seems reasonable that bash-isms would be allowed. If it was #!/bin/sh specified as the interpreter, then I would understand remaining constrained to POSIX-only.
But I think in your next message you have a nice POSIX compatible method of doing it with shifts, and it is probably best to default to POSIX when practical. I'll go ahead and change it to the 'shift $(($# - 1))' method.
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
Quoted 8 lines · Jeff Cody
Quoted 3 lines
${@: -1} is not portable:
Yes, and there are likely other bash-isms in some of the shell scripts in qemu-iotests. Since #!/bin/bash is explicitly specified, it seems reasonable that bash-isms would be allowed.
Ah, indeed, I missed the shebang line. Where we KNOW we are using bash, I have no problem using the additional features that bash gives us as a guarantee.
Quoted 4 lines
But I think in your next message you have a nice POSIX compatible method of doing it with shifts, and it is probably best to default to POSIX when practical. I'll go ahead and change it to the 'shift $(($# - 1))' method.
Where the portable way doesn't cost any additional forks, then yes, I agree that being portable is nice for the sake of anyone copying-and-pasting from our explicit bash script over to a more generic /bin/sh script.
Signature · 2 lines
Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests
In reply to Jeff CodyQuoted 21 lines · Jeff Cody
(quoted diff) +21
-0+# Launch a QEMU process. +# +# Input parameters: +# $qemu_comm_method: set this variable to 'monitor' (case insensitive) +# to use the QEMU HMP monitor for communication. +# Otherwise, the default of QMP is used. +# Returns: +# $QEMU_HANDLE: set to a handle value to communicate with this QEMU instance. +# +function _launch_qemu() +{ + local comm= + local fifo_out= + local fifo_in= + + if (shopt -s nocasematch; [[ "${qemu_comm_method}" == "monitor" ]]) + then + comm="-monitor stdio -qmp none" + else + local qemu_comm_method="qmp" + comm="-monitor none -qmp stdio"
Review
Quoted 10 lines
(quoted diff) +10
-0+ fi + + fifo_out=${QEMU_FIFO_OUT}_${_QEMU_HANDLE} + fifo_in=${QEMU_FIFO_IN}_${_QEMU_HANDLE} + mkfifo "${fifo_out}" + mkfifo "${fifo_in}" + + "${QEMU}" -nographic -serial none ${comm} "${@}" 2>&1 \ + >"${fifo_out}" \ + <"${fifo_in}" &
Shall we use '-machine accel=qtest' as we do in iotests.py (to run no guest code)? Because below patch has a big difference of 067's stability and run time in my case:
tests/qemu-iotests/067 +1 -1
@@ -39,7 +39,7 @@ _supported_os Linux function do_run_qemu() { echo Testing: "$@" - $QEMU -nographic -qmp stdio -serial none "$@" + $QEMU -nographic -machine accel=qtest -qmp stdio -serial none "$@" echo }
Fam
Quoted 25 lines
(quoted diff) +25
-0+ QEMU_PID[${_QEMU_HANDLE}]=$! + + if [ "${BASH_VERSINFO[0]}" -ge "4" ] && [ "${BASH_VERSINFO[1]}" -ge "1" ] + then + # bash >= 4.1 required for automatic fd + exec {_out_fd}<"${fifo_out}" + exec {_in_fd}>"${fifo_in}" + else + let _out_fd++ + let _in_fd++ + eval "exec ${_out_fd}<'${fifo_out}'" + eval "exec ${_in_fd}>'${fifo_in}'" + fi + + QEMU_OUT[${_QEMU_HANDLE}]=${_out_fd} + QEMU_IN[${_QEMU_HANDLE}]=${_in_fd} + + if [ "${qemu_comm_method}" == "qmp" ] + then + # Don't print response, since it has version information in it + silent=yes _timed_wait_for ${_QEMU_HANDLE} "capabilities" + fi + QEMU_HANDLE=${_QEMU_HANDLE} + let _QEMU_HANDLE++ +}
PATCH 2/4 block: qemu-iotests - update 085 to use common.qemu
In reply to Jeff CodyThe new functionality of common.qemu implements the QEMU control and communication functionality that was originally in test 085.
This removes that now-duplicate functionality, and uses the common.qemu functions.
--- tests/qemu-iotests/085 | 73 +++++++++----------------------------------------- 1 file changed, 12 insertions(+), 61 deletions(-)
tests/qemu-iotests/085 +12 -61
@@ -30,10 +30,6 @@ echo "QA output created by $seq" here=`pwd` status=1 # failure is the default! -qemu_pid= - -QMP_IN="${TEST_DIR}/qmp-in-$$" -QMP_OUT="${TEST_DIR}/qmp-out-$$" snapshot_virt0="snapshot-v0.qcow2" snapshot_virt1="snapshot-v1.qcow2"
@@ -42,10 +38,7 @@ MAX_SNAPSHOTS=10 _cleanup() { - kill -KILL ${qemu_pid} - wait ${qemu_pid} 2>/dev/null # silent kill - - rm -f "${QMP_IN}" "${QMP_OUT}" + _cleanup_qemu for i in $(seq 1 ${MAX_SNAPSHOTS}) do rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"
@@ -59,43 +52,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # get standard environment, filters and checks . ./common.rc . ./common.filter +. ./common.qemu _supported_fmt qcow2 _supported_proto file _supported_os Linux -# Wait for expected QMP response from QEMU. Will time out -# after 10 seconds, which counts as failure. -# -# $1 is the string to expect -# -# If $silent is set to anything but an empty string, then -# response is not echoed out. -function timed_wait_for() -{ - while read -t 10 resp <&5 - do - if [ "${silent}" == "" ]; then - echo "${resp}" | _filter_testdir | _filter_qemu - fi - grep -q "${1}" < <(echo ${resp}) - if [ $? -eq 0 ]; then - return - fi - done - echo "Timeout waiting for ${1}" - exit 1 # Timeout means the test failed -} - -# Sends QMP command to QEMU, and waits for the expected response -# -# ${1}: String of the QMP command to send -# ${2}: String that the QEMU response should contain -function send_qmp_cmd() -{ - echo "${1}" >&6 - timed_wait_for "${2}" -} # ${1}: unique identifier for the snapshot filename function create_single_snapshot()
@@ -104,7 +66,7 @@ function create_single_snapshot() 'arguments': { 'device': 'virtio0', 'snapshot-file':'"${TEST_DIR}/${1}-${snapshot_virt0}"', 'format': 'qcow2' } }" - send_qmp_cmd "${cmd}" "return" + _send_qemu_cmd $h "${cmd}" "return" } # ${1}: unique identifier for the snapshot filename
@@ -120,14 +82,11 @@ function create_group_snapshot() 'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt1}"' } } ] } }" - send_qmp_cmd "${cmd}" "return" + _send_qemu_cmd $h "${cmd}" "return" } size=128M -mkfifo "${QMP_IN}" -mkfifo "${QMP_OUT}" - _make_test_img $size mv "${TEST_IMG}" "${TEST_IMG}.orig" _make_test_img $size
@@ -136,23 +95,15 @@ echo echo === Running QEMU === echo -"${QEMU}" -nographic -monitor none -serial none -qmp stdio\ - -drive file="${TEST_IMG}.orig",if=virtio\ - -drive file="${TEST_IMG}",if=virtio 2>&1 >"${QMP_OUT}" <"${QMP_IN}"& -qemu_pid=$! - -# redirect fifos to file descriptors, to keep from blocking -exec 5<"${QMP_OUT}" -exec 6>"${QMP_IN}" - -# Don't print response, since it has version information in it -silent=yes timed_wait_for "capabilities" +qemu_comm_method="qmp" +_launch_qemu -drive file="${TEST_IMG}.orig",if=virtio -drive file="${TEST_IMG}",if=virtio +h=$QEMU_HANDLE echo echo === Sending capabilities === echo -send_qmp_cmd "{ 'execute': 'qmp_capabilities' }" "return" +_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return" echo echo === Create a single snapshot on virtio0 ===
@@ -165,16 +116,16 @@ echo echo === Invalid command - missing device and nodename === echo -send_qmp_cmd "{ 'execute': 'blockdev-snapshot-sync', - 'arguments': { 'snapshot-file':'"${TEST_DIR}"/1-${snapshot_virt0}', +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync', + 'arguments': { 'snapshot-file':'"${TEST_DIR}/1-${snapshot_virt0}"', 'format': 'qcow2' } }" "error" echo echo === Invalid command - missing snapshot-file === echo -send_qmp_cmd "{ 'execute': 'blockdev-snapshot-sync', - 'arguments': { 'device': 'virtio0', +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync', + 'arguments': { 'device': 'virtio0', 'format': 'qcow2' } }" "error" echo echo
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 2/4] block: qemu-iotests - update 085 to use common.qemu
Quoted 161 lines · Jeff Cody
The new functionality of common.qemu implements the QEMU control and communication functionality that was originally in test 085.
This removes that now-duplicate functionality, and uses the common.qemu functions.
Signed-off-by Jeff Cody <[email protected]>--- tests/qemu-iotests/085 | 73 +++++++++----------------------------------------- 1 file changed, 12 insertions(+), 61 deletions(-)tests/qemu-iotests/085 +12
-61@@ -30,10 +30,6 @@ echo "QA output created by $seq" here=`pwd` status=1 # failure is the default! -qemu_pid= - -QMP_IN="${TEST_DIR}/qmp-in-$$" -QMP_OUT="${TEST_DIR}/qmp-out-$$" snapshot_virt0="snapshot-v0.qcow2" snapshot_virt1="snapshot-v1.qcow2"@@ -42,10 +38,7 @@ MAX_SNAPSHOTS=10 _cleanup() { - kill -KILL ${qemu_pid} - wait ${qemu_pid} 2>/dev/null # silent kill - - rm -f "${QMP_IN}" "${QMP_OUT}" + _cleanup_qemu for i in $(seq 1 ${MAX_SNAPSHOTS}) do rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"@@ -59,43 +52,12 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # get standard environment, filters and checks . ./common.rc . ./common.filter +. ./common.qemu _supported_fmt qcow2 _supported_proto file _supported_os Linux -# Wait for expected QMP response from QEMU. Will time out -# after 10 seconds, which counts as failure. -# -# $1 is the string to expect -# -# If $silent is set to anything but an empty string, then -# response is not echoed out. -function timed_wait_for() -{ - while read -t 10 resp <&5 - do - if [ "${silent}" == "" ]; then - echo "${resp}" | _filter_testdir | _filter_qemu - fi - grep -q "${1}" < <(echo ${resp}) - if [ $? -eq 0 ]; then - return - fi - done - echo "Timeout waiting for ${1}" - exit 1 # Timeout means the test failed -} - -# Sends QMP command to QEMU, and waits for the expected response -# -# ${1}: String of the QMP command to send -# ${2}: String that the QEMU response should contain -function send_qmp_cmd() -{ - echo "${1}" >&6 - timed_wait_for "${2}" -} # ${1}: unique identifier for the snapshot filename function create_single_snapshot()@@ -104,7 +66,7 @@ function create_single_snapshot() 'arguments': { 'device': 'virtio0', 'snapshot-file':'"${TEST_DIR}/${1}-${snapshot_virt0}"', 'format': 'qcow2' } }" - send_qmp_cmd "${cmd}" "return" + _send_qemu_cmd $h "${cmd}" "return" } # ${1}: unique identifier for the snapshot filename@@ -120,14 +82,11 @@ function create_group_snapshot() 'snapshot-file': '"${TEST_DIR}/${1}-${snapshot_virt1}"' } } ] } }" - send_qmp_cmd "${cmd}" "return" + _send_qemu_cmd $h "${cmd}" "return" } size=128M -mkfifo "${QMP_IN}" -mkfifo "${QMP_OUT}" - _make_test_img $size mv "${TEST_IMG}" "${TEST_IMG}.orig" _make_test_img $size@@ -136,23 +95,15 @@ echo echo === Running QEMU === echo -"${QEMU}" -nographic -monitor none -serial none -qmp stdio\ - -drive file="${TEST_IMG}.orig",if=virtio\ - -drive file="${TEST_IMG}",if=virtio 2>&1 >"${QMP_OUT}" <"${QMP_IN}"& -qemu_pid=$! - -# redirect fifos to file descriptors, to keep from blocking -exec 5<"${QMP_OUT}" -exec 6>"${QMP_IN}" - -# Don't print response, since it has version information in it -silent=yes timed_wait_for "capabilities" +qemu_comm_method="qmp" +_launch_qemu -drive file="${TEST_IMG}.orig",if=virtio -drive file="${TEST_IMG}",if=virtio +h=$QEMU_HANDLE echo echo === Sending capabilities === echo -send_qmp_cmd "{ 'execute': 'qmp_capabilities' }" "return" +_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return" echo echo === Create a single snapshot on virtio0 ===@@ -165,16 +116,16 @@ echo echo === Invalid command - missing device and nodename === echo -send_qmp_cmd "{ 'execute': 'blockdev-snapshot-sync', - 'arguments': { 'snapshot-file':'"${TEST_DIR}"/1-${snapshot_virt0}', +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync', + 'arguments': { 'snapshot-file':'"${TEST_DIR}/1-${snapshot_virt0}"', 'format': 'qcow2' } }" "error" echo echo === Invalid command - missing snapshot-file === echo -send_qmp_cmd "{ 'execute': 'blockdev-snapshot-sync', - 'arguments': { 'device': 'virtio0', +_send_qemu_cmd $h "{ 'execute': 'blockdev-snapshot-sync', + 'arguments': { 'device': 'virtio0', 'format': 'qcow2' } }" "error" echo echoSignature · 1 line
1.8.3.1
PATCH 3/4 block: qemu-iotests - fix image cleanup when using spaced pathnames
In reply to Jeff CodyThe _rm_test_img() function in common.rc did not quote the image file, which left droppings in the scratch directory (and performed a potentially unsafe rm -f).
This adds the necessary quotes.
--- tests/qemu-iotests/common.rc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tests/qemu-iotests/common.rc +2 -2
@@ -178,10 +178,10 @@ _rm_test_img() local img=$1 if [ "$IMGFMT" = "vmdk" ]; then # Remove all the extents for vmdk - $QEMU_IMG info $img 2>/dev/null | grep 'filename:' | cut -f 2 -d: \ + "$QEMU_IMG" info "$img" 2>/dev/null | grep 'filename:' | cut -f 2 -d: \ | xargs -I {} rm -f "{}" fi - rm -f $img + rm -f "$img" } _cleanup_test_img()
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 3/4] block: qemu-iotests - fix image cleanup when using spaced pathnames
Quoted 32 lines · Jeff Cody
The
_rm_test_img()function in common.rc did not quote the image file, which left droppings in the scratch directory (and performed a potentially unsafe rm -f).This adds the necessary quotes.
Signed-off-by Jeff Cody <[email protected]>--- tests/qemu-iotests/common.rc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)tests/qemu-iotests/common.rc +2
-2@@ -178,10 +178,10 @@ _rm_test_img() local img=$1 if [ "$IMGFMT" = "vmdk" ]; then # Remove all the extents for vmdk - $QEMU_IMG info $img 2>/dev/null | grep 'filename:' | cut -f 2 -d: \ + "$QEMU_IMG" info "$img" 2>/dev/null | grep 'filename:' | cut -f 2 -d: \ | xargs -I {} rm -f "{}" fi - rm -f $img + rm -f "$img" } _cleanup_test_img()Signature · 1 line
1.8.3.1
PATCH 4/4 block: qemu-iotests: make test 019 and 086 work with spaced pathnames
In reply to Jeff CodyBoth tests 019 and 086 need proper quotations to work with pathnames that contain spaces.
--- tests/qemu-iotests/019 | 2 +- tests/qemu-iotests/086 | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)
tests/qemu-iotests/019 +1 -1
@@ -96,7 +96,7 @@ mv "$TEST_IMG" "$TEST_IMG.orig" for backing_option in "-B " "-o backing_file="; do echo - echo Testing conversion with $backing_option$TEST_IMG.base | _filter_testdir | _filter_imgfmt + echo Testing conversion with $backing_option"$TEST_IMG.base" | _filter_testdir | _filter_imgfmt echo $QEMU_IMG convert -O $IMGFMT $backing_option"$TEST_IMG.base" "$TEST_IMG.orig" "$TEST_IMG"
tests/qemu-iotests/086 +4 -4
@@ -51,10 +51,10 @@ function run_qemu_img() size=128M _make_test_img $size -$QEMU_IO -c 'write 0 1M' $TEST_IMG | _filter_qemu_io -$QEMU_IO -c 'write 2M 1M' $TEST_IMG | _filter_qemu_io -$QEMU_IO -c 'write 4M 1M' $TEST_IMG | _filter_qemu_io -$QEMU_IO -c 'write 32M 1M' $TEST_IMG | _filter_qemu_io +$QEMU_IO -c 'write 0 1M' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'write 2M 1M' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'write 4M 1M' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'write 32M 1M' "$TEST_IMG" | _filter_qemu_io $QEMU_IMG convert -p -O $IMGFMT -f $IMGFMT "$TEST_IMG" "$TEST_IMG".base 2>&1 |\ _filter_testdir | sed -e 's/\r/\n/g'
Signature · 1 line
1.8.3.1
Re: [Qemu-devel] [PATCH 4/4] block: qemu-iotests: make test 019 and 086 work with spaced pathnames
Quoted 45 lines · Jeff Cody
Both tests 019 and 086 need proper quotations to work with pathnames that contain spaces.
Signed-off-by Jeff Cody <[email protected]>--- tests/qemu-iotests/019 | 2 +- tests/qemu-iotests/086 | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)tests/qemu-iotests/019 +1
-1@@ -96,7 +96,7 @@ mv "$TEST_IMG" "$TEST_IMG.orig" for backing_option in "-B " "-o backing_file="; do echo - echo Testing conversion with $backing_option$TEST_IMG.base | _filter_testdir | _filter_imgfmt + echo Testing conversion with $backing_option"$TEST_IMG.base" | _filter_testdir | _filter_imgfmt echo $QEMU_IMG convert -O $IMGFMT $backing_option"$TEST_IMG.base" "$TEST_IMG.orig" "$TEST_IMG"tests/qemu-iotests/086 +4
-4@@ -51,10 +51,10 @@ function run_qemu_img() size=128M _make_test_img $size -$QEMU_IO -c 'write 0 1M' $TEST_IMG | _filter_qemu_io -$QEMU_IO -c 'write 2M 1M' $TEST_IMG | _filter_qemu_io -$QEMU_IO -c 'write 4M 1M' $TEST_IMG | _filter_qemu_io -$QEMU_IO -c 'write 32M 1M' $TEST_IMG | _filter_qemu_io +$QEMU_IO -c 'write 0 1M' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'write 2M 1M' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'write 4M 1M' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'write 32M 1M' "$TEST_IMG" | _filter_qemu_io $QEMU_IMG convert -p -O $IMGFMT -f $IMGFMT "$TEST_IMG" "$TEST_IMG".base 2>&1 |\ _filter_testdir | sed -e 's/\r/\n/g'Signature · 1 line
1.8.3.1