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

Paolo Bonzini
2014-06-12T10:16:26Z

[-- 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;