Re: [PATCH v5 12/19] Introduce virtio-testdev

[Paolo Bonzini <pbonzini@redhat.com>]

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

Il 11/06/2014 16:01, Andrew Jones ha scritto:
> virtio-testdev is a communication channel to qemu through virtio that
> can be used by test code to send commands. The only command currently
> implemented is EXIT, which allows the test code to exit with a given
> status code.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

Hi Drew,

quite a few months ago I had promised to help with an alternative
implementation that does not require a new virtio device (which would
in turn need registration of the device ID and so on.

Since the return values are hardly used in your test device, I removed
them and made a simple write-only backend for virtio-serial.  With this
simplification, virtio-serial is almost as simple as your test device,
you just use the transmit queue (queue 1).  Receiving data would be a
bit more complicated, basically you would first put a buffer in queue
0, then issue the command, then wait for an interrupt on queue 0.

There is a single command implemented, "q".  It takes a (prefix)
argument for the exit code, so an error exit is implemented by writing
"1q" to the virtio-serial port.

Right now I'm using ASCII (because I tested it from a Linux guest with
just "echo 1q > /dev/vport0p1"), but of course you could just as well
encode arguments in binary format.

The QEMU implementation follows, it can be used as:

    qemu-system-arm ... \
      -chardev testdev,id=ctl \
      -device virtio-serial -device virtserialport,chardev=ctl ...

If you are okay with it, feel free to pick it up and send it to 
qemu-devel, and just send v6 of this one patch.  Unfortunately I cannot 
merge kvm-unit-tests until there is QEMU support for either 
virtio-testdev or chardev-testdev, but it should not take much time.

Paolo

[-- Attachment #2: char-testdev.patch --]
[-- Type: text/x-patch, Size: 6082 bytes --]

diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 591ddcf..5820608 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -1,7 +1,7 @@
 common-obj-y += rng.o rng-egd.o
 common-obj-$(CONFIG_POSIX) += rng-random.o
 
-common-obj-y += msmouse.o
+common-obj-y += msmouse.o testdev.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 baum.o-cflags := $(SDL_CFLAGS)
 
diff --git a/backends/testdev.c b/backends/testdev.c
new file mode 100644
index 0000000..8e93266
--- /dev/null
+++ b/backends/testdev.c
@@ -0,0 +1,131 @@
+/*
+ * QEMU Char Device for testsuite control
+ *
+ * Copyright (c) 2014 Red Hat, Inc.
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu-common.h"
+#include "sysemu/char.h"
+
+#define BUF_SIZE 32
+
+typedef struct {
+    CharDriverState *chr;
+    uint8_t in_buf[32];
+    int in_buf_used;
+} TestdevCharState;
+
+/* Try to interpret a whole incoming packet */
+static int testdev_eat_packet(TestdevCharState *testdev)
+{
+    const uint8_t *cur = testdev->in_buf;
+    int len = testdev->in_buf_used;
+    uint8_t c;
+    int arg;
+
+#define EAT(c) do { \
+    if (!len--) {   \
+        return 0;   \
+    }               \
+    c = *cur++;     \
+} while (0)
+
+    EAT(c);
+
+    while (isspace(c)) {
+        EAT(c);
+    }
+
+    arg = 0;
+    while (isdigit(c)) {
+        arg = arg * 10 + c - '0';
+        EAT(c);
+    }
+
+    while (isspace(c)) {
+        EAT(c);
+    }
+
+    switch (c) {
+    case 'q':
+        exit((arg << 1) | 1);
+        break;
+    default:
+        break;
+    }
+    return cur - testdev->in_buf;
+}
+
+/* The other end is writing some data.  Store it and try to interpret */
+static int testdev_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+    TestdevCharState *testdev = chr->opaque;
+    int tocopy, eaten, orig_len = len;
+
+    while (len) {
+        /* Complete our buffer as much as possible */
+        tocopy = MIN(len, BUF_SIZE - testdev->in_buf_used);
+
+        memcpy(testdev->in_buf + testdev->in_buf_used, buf, tocopy);
+        testdev->in_buf_used += tocopy;
+        buf += tocopy;
+        len -= tocopy;
+
+        /* Interpret it as much as possible */
+        while (testdev->in_buf_used > 0 &&
+               (eaten = testdev_eat_packet(testdev)) > 0) {
+            memmove(testdev->in_buf, testdev->in_buf + eaten,
+                    testdev->in_buf_used - eaten);
+            testdev->in_buf_used -= eaten;
+        }
+    }
+    return orig_len;
+}
+
+static void testdev_close(struct CharDriverState *chr)
+{
+    TestdevCharState *testdev = chr->opaque;
+
+    g_free(testdev);
+}
+
+CharDriverState *chr_testdev_init(void)
+{
+    TestdevCharState *testdev;
+    CharDriverState *chr;
+
+    testdev = g_malloc0(sizeof(TestdevCharState));
+    testdev->chr = chr = g_malloc0(sizeof(CharDriverState));
+
+    chr->opaque = testdev;
+    chr->chr_write = testdev_write;
+    chr->chr_close = testdev_close;
+
+    return chr;
+}
+
+static void register_types(void)
+{
+    register_char_driver_qapi("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL);
+}
+
+type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index b81a6ff..fbb356e 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -306,6 +306,9 @@ CharDriverState *qemu_char_get_next_serial(void);
 /* msmouse */
 CharDriverState *qemu_chr_open_msmouse(void);
 
+/* testdev.c */
+CharDriverState *chr_testdev_init(void);
+
 /* baum.c */
 CharDriverState *chr_baum_init(void);
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 14b498b..f9e5bbd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2617,6 +2617,7 @@
                                        'mux'    : 'ChardevMux',
                                        'msmouse': 'ChardevDummy',
                                        'braille': 'ChardevDummy',
+                                       'testdev': 'ChardevDummy',
                                        'stdio'  : 'ChardevStdio',
                                        'console': 'ChardevDummy',
                                        'spicevmc' : 'ChardevSpiceChannel',
diff --git a/qemu-char.c b/qemu-char.c
index 17b476e..a64dec5 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2974,6 +2974,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
         strcmp(filename, "pty")     == 0 ||
         strcmp(filename, "msmouse") == 0 ||
         strcmp(filename, "braille") == 0 ||
+        strcmp(filename, "testdev") == 0 ||
         strcmp(filename, "stdio")   == 0) {
         qemu_opt_set(opts, "backend", filename);
         return opts;
@@ -3770,6 +3771,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         chr = chr_baum_init();
         break;
 #endif
+    case CHARDEV_BACKEND_KIND_TESTDEV:
+        chr = chr_testdev_init();
+        break;
     case CHARDEV_BACKEND_KIND_STDIO:
         chr = qemu_chr_open_stdio(backend->stdio);
         break;