[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests |
Date: |
Wed, 19 Mar 2014 10:19:48 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Mar 19, 2014 at 02:39:25PM +0100, Benoît Canet wrote:
> The Monday 17 Mar 2014 à 21:24:37 (-0400), Jeff Cody wrote :
> > 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
>
> "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.
> > 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 <address@hidden>
> > ---
> > tests/qemu-iotests/common.qemu | 164
> > +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 164 insertions(+)
> > create mode 100644 tests/qemu-iotests/common.qemu
> >
> > diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
> > new file mode 100644
> > index 0000000..8068395
> > --- /dev/null
> > +++ b/tests/qemu-iotests/common.qemu
> > @@ -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
^^^
This is a typo, it should say "$3" ----------------------------------|
> > +# 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:address@hidden" >&${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)
> > + 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} "address@hidden" 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} ?
>
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
> > + 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 "address@hidden"
> > + 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
> > +}
> > +
> > --
> > 1.8.3.1
> >
- [Qemu-devel] [PATCH 0/4] Add common QEMU control functionality to qemu-iotests, Jeff Cody, 2014/03/17
- [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests, Jeff Cody, 2014/03/17
- Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests, Benoît Canet, 2014/03/19
- Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests,
Jeff Cody <=
- Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests, Eric Blake, 2014/03/19
- Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests, Eric Blake, 2014/03/19
- Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests, Jeff Cody, 2014/03/19
- Re: [Qemu-devel] [PATCH 1/4] block: qemu-iotests - add common.qemu, for bash-controlled qemu tests, Eric Blake, 2014/03/19
[Qemu-devel] [PATCH 2/4] block: qemu-iotests - update 085 to use common.qemu, Jeff Cody, 2014/03/17
[Qemu-devel] [PATCH 3/4] block: qemu-iotests - fix image cleanup when using spaced pathnames, Jeff Cody, 2014/03/17
[Qemu-devel] [PATCH 4/4] block: qemu-iotests: make test 019 and 086 work with spaced pathnames, Jeff Cody, 2014/03/17