[Qemu-devel] [PATCH] qemu-test: add virtio-serial test

[Qemu-devel] [PATCH] qemu-test: add virtio-serial test

[Anthony Liguori]

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
+
+    # 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
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH] qemu-test: add virtio-serial test

[Alex Bradbury]

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

Re: [Qemu-devel] [PATCH] qemu-test: add virtio-serial test

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 833 bytes --]

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"`"

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH] qemu-test: add virtio-serial test

[Alex Bradbury]

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

Re: [Qemu-devel] [PATCH] qemu-test: add virtio-serial test

[Anthony Liguori]

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
>