| Message ID | 1324563805-4285-1-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 22 December 2011 14:23, Anthony Liguori <aliguori@us.ibm.com> wrote: > This is a pretty simple test that just confirms that virtio-serial shows up and > is writable. It also tests the alias for virtio-serial-pci. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > tests/virtio-serial.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 52 insertions(+), 0 deletions(-) > create mode 100755 tests/virtio-serial.sh > > diff --git a/tests/virtio-serial.sh b/tests/virtio-serial.sh > new file mode 100755 > index 0000000..e95ae6e > --- /dev/null > +++ b/tests/virtio-serial.sh > @@ -0,0 +1,52 @@ > +#!/bin/sh > + > +canary="** waiting for... **" > + > +in_host() { > + tmpchr=$tmpdir/chr.log By far the most common error likely to appear in any of these test scripts is insufficient shell quoting causing e.g. breakages when the scripts are run from a directory with spaces. Having looked at qemu-test, I see that $tmpdir is set to .tmp-$$ and so can never contain spaces, and so it's fine to not quote the variable expansion. That said, perhaps it would make it easier to review scripts and less likely for these issues to creep in to the codebase if there was a blanket rule that all variable expansions be quoted? Alex
On 12/22/2011 08:46 AM, Alex Bradbury wrote: >> + >> +in_host() { >> + tmpchr=$tmpdir/chr.log > > By far the most common error likely to appear in any of these test > scripts is insufficient shell quoting causing e.g. breakages when the > scripts are run from a directory with spaces. Shell assignments do not need quoting, since they are not subject to argument splitting or filename expansion in the first place. Blindly requiring double-quoting of all shell variables and command substitutions can actually introduce problems if you care about portability to ancient shells, since constructs like: var=`command "with quotes"` are actually MORE portable than: var="`command "with quotes"`"
On 22 December 2011 15:58, Eric Blake <eblake@redhat.com> wrote: > Shell assignments do not need quoting, since they are not subject to > argument splitting or filename expansion in the first place. You're right, that was an incorrectly chosen example. > Blindly requiring double-quoting of all shell variables and command > substitutions can actually introduce problems if you care about > portability to ancient shells Is there really a need to care about anything but POSIX compliant shells? Alex
On 12/22/2011 09:46 AM, Alex Bradbury wrote: > On 22 December 2011 14:23, Anthony Liguori<aliguori@us.ibm.com> wrote: >> This is a pretty simple test that just confirms that virtio-serial shows up and >> is writable. It also tests the alias for virtio-serial-pci. >> >> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> >> --- >> tests/virtio-serial.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 52 insertions(+), 0 deletions(-) >> create mode 100755 tests/virtio-serial.sh >> >> diff --git a/tests/virtio-serial.sh b/tests/virtio-serial.sh >> new file mode 100755 >> index 0000000..e95ae6e >> --- /dev/null >> +++ b/tests/virtio-serial.sh >> @@ -0,0 +1,52 @@ >> +#!/bin/sh >> + >> +canary="** waiting for... **" >> + >> +in_host() { >> + tmpchr=$tmpdir/chr.log > > By far the most common error likely to appear in any of these test > scripts is insufficient shell quoting causing e.g. breakages when the > scripts are run from a directory with spaces. Having looked at > qemu-test, I see that $tmpdir is set to .tmp-$$ and so can never > contain spaces, and so it's fine to not quote the variable expansion. > That said, perhaps it would make it easier to review scripts and less > likely for these issues to creep in to the codebase if there was a > blanket rule that all variable expansions be quoted? In general, I try to be very careful about that. The only time tests ever refer to filenames is relative to tmpdir and as you've noted, tmpdir purposefully cannot have a space in it. So I don't think it's going to be a problem. I can't see an immediate reason why tests should use anything outside of tmpdir. Regards, Anthony Liguori > > Alex >
diff --git a/tests/virtio-serial.sh b/tests/virtio-serial.sh new file mode 100755 index 0000000..e95ae6e --- /dev/null +++ b/tests/virtio-serial.sh @@ -0,0 +1,52 @@ +#!/bin/sh + +canary="** waiting for... **" + +in_host() { + tmpchr=$tmpdir/chr.log + + # Also test alias + devname=`choose virtio-serial virtio-serial-pci` + + qemu -nographic -enable-kvm -device $devname \ + -device virtserialport,name=org.qemu.test,chardev=chr0 \ + -chardev file,path=$tmpchr,id=chr0 + rc=$? + + if test $rc = 0; then + if ! grep "$canary" $tmpchr >/dev/null; then + echo "Failed to see output from guest!" + rc=1 + fi + fi + + rm -f $tmpchr + + return $rc +} + +in_guest() { + sysfspath=/sys/bus/virtio/devices/virtio0/virtio-ports/vport0p1 + if ! test -e $sysfspath/name; then + echo "Device not visible!" + return 1 + fi + + name=`cat $sysfspath/name` + + if test "$name" != "org.qemu.test"; then + echo "Device has wrong name!" + echo "Expected 'org.qemu.test', got '$name'" + return 2 + fi + + echo "$canary" > /dev/vport0p1 + + return 0 +} + +if test $QEMU_TEST; then + in_host +else + in_guest +fi
This is a pretty simple test that just confirms that virtio-serial shows up and is writable. It also tests the alias for virtio-serial-pci. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- tests/virtio-serial.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 52 insertions(+), 0 deletions(-) create mode 100755 tests/virtio-serial.sh