[Qemu-devel] [PATCH v2 0/5] Convert remaining legacy chardevs to QAPI

[Qemu-devel] [PATCH v2 0/5] Convert remaining legacy chardevs to QAPI

[Peter Maydell]

This patchset converts the two remaining legacy chardevs
('socket' and 'udp') to use the new-style parse/kind
mechanisms, and removes all the no-longer-required
legacy machinery.

Patch 1 was posted to the list back in June
(https://patches.linaro.org/32298/). I've fixed the obvious
bug picked up in code review, and as far as I can tell from
the thread we decided that the blocking/non-blocking
difference between QAPI and legacy wasn't a problem.

Patch 2 fixes a hole in the functionality of QAPI-described
UDP chardevs, to avoid regressing the commandline functionality
when we convert the UDP backend in patch 3.

Patch 4 may be easier to review as an ignore-whitespaces
diff (the de-indentation makes the diff a bit awkward).

Changes v1->v2:
 * fixed the has_* values as suggested by Markus
 * added patch 5 which renames the _qapi() function now
   the legacy version has gone (again, as suggested by
   Markus)

Peter Maydell (5):
  qemu-char: Convert socket backend to QAPI
  util/qemu-sockets.c: Support specifying IPv4 or IPv6 in socket_dgram()
  qemu-char: Convert udp backend to QAPI
  qemu-char: Remove register_char_driver() machinery
  qemu-char: Rename register_char_driver_qapi() to
    register_char_driver()

 backends/baum.c       |   2 +-
 backends/msmouse.c    |   2 +-
 backends/testdev.c    |   2 +-
 include/sysemu/char.h |   3 +-
 qemu-char.c           | 353 ++++++++++++++++++++++++--------------------------
 spice-qemu-char.c     |   8 +-
 ui/console.c          |   3 +-
 util/qemu-sockets.c   |   3 +-
 8 files changed, 180 insertions(+), 196 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/5] qemu-char: Convert socket backend to QAPI

[Peter Maydell]

Convert the socket char backend to the new style QAPI framework;
this allows it to return an Error ** to callers who might not
want it to print directly about socket failures.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 qemu-char.c | 112 ++++++++++++++++++++++++++++++------------------------------
 1 file changed, 56 insertions(+), 56 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index d4f327a..4b99382 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2983,61 +2983,6 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, bool do_nodelay,
     return chr;
 }
 
-static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
-{
-    CharDriverState *chr = NULL;
-    Error *local_err = NULL;
-    int fd = -1;
-
-    bool is_listen      = qemu_opt_get_bool(opts, "server", false);
-    bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
-    bool is_telnet      = qemu_opt_get_bool(opts, "telnet", false);
-    bool do_nodelay     = !qemu_opt_get_bool(opts, "delay", true);
-    bool is_unix        = qemu_opt_get(opts, "path") != NULL;
-
-    if (is_unix) {
-        if (is_listen) {
-            fd = unix_listen_opts(opts, &local_err);
-        } else {
-            fd = unix_connect_opts(opts, &local_err, NULL, NULL);
-        }
-    } else {
-        if (is_listen) {
-            fd = inet_listen_opts(opts, 0, &local_err);
-        } else {
-            fd = inet_connect_opts(opts, &local_err, NULL, NULL);
-        }
-    }
-    if (fd < 0) {
-        goto fail;
-    }
-
-    if (!is_waitconnect)
-        qemu_set_nonblock(fd);
-
-    chr = qemu_chr_open_socket_fd(fd, do_nodelay, is_listen, is_telnet,
-                                  is_waitconnect, &local_err);
-    if (local_err) {
-        goto fail;
-    }
-    return chr;
-
-
- fail:
-    if (local_err) {
-        qerror_report_err(local_err);
-        error_free(local_err);
-    }
-    if (fd >= 0) {
-        closesocket(fd);
-    }
-    if (chr) {
-        g_free(chr->opaque);
-        g_free(chr);
-    }
-    return NULL;
-}
-
 /*********************************************************/
 /* Ring buffer chardev */
 
@@ -3448,6 +3393,60 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
     backend->mux->chardev = g_strdup(chardev);
 }
 
+static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
+                                  Error **errp)
+{
+    bool is_listen      = qemu_opt_get_bool(opts, "server", false);
+    bool is_waitconnect = is_listen && qemu_opt_get_bool(opts, "wait", true);
+    bool is_telnet      = qemu_opt_get_bool(opts, "telnet", false);
+    bool do_nodelay     = !qemu_opt_get_bool(opts, "delay", true);
+    const char *path = qemu_opt_get(opts, "path");
+    const char *host = qemu_opt_get(opts, "host");
+    const char *port = qemu_opt_get(opts, "port");
+    SocketAddress *addr;
+
+    if (!path) {
+        if (!host) {
+            error_setg(errp, "chardev: socket: no host given");
+            return;
+        }
+        if (!port) {
+            error_setg(errp, "chardev: socket: no port given");
+            return;
+        }
+    }
+
+    backend->socket = g_new0(ChardevSocket, 1);
+
+    backend->socket->has_nodelay = true;
+    backend->socket->nodelay = do_nodelay;
+    backend->socket->has_server = true;
+    backend->socket->server = is_listen;
+    backend->socket->has_telnet = true;
+    backend->socket->telnet = is_telnet;
+    backend->socket->has_wait = true;
+    backend->socket->wait = is_waitconnect;
+
+    addr = g_new0(SocketAddress, 1);
+    if (path) {
+        addr->kind = SOCKET_ADDRESS_KIND_UNIX;
+        addr->q_unix = g_new0(UnixSocketAddress, 1);
+        addr->q_unix->path = g_strdup(path);
+    } else {
+        addr->kind = SOCKET_ADDRESS_KIND_INET;
+        addr->inet = g_new0(InetSocketAddress, 1);
+        addr->inet->host = g_strdup(host);
+        addr->inet->port = g_strdup(port);
+        addr->inet->has_to = qemu_opt_get(opts, "to");
+        addr->inet->to = qemu_opt_get_number(opts, "to", 0);
+        addr->inet->has_ipv4 = qemu_opt_get(opts, "ipv4");
+        addr->inet->ipv4 = qemu_opt_get_bool(opts, "ipv4", 0);
+        addr->inet->has_ipv6 = qemu_opt_get(opts, "ipv6");
+        addr->inet->ipv6 = qemu_opt_get_bool(opts, "ipv6", 0);
+    }
+    backend->socket->addr = addr;
+}
+
 typedef struct CharDriver {
     const char *name;
     /* old, pre qapi */
@@ -4136,7 +4135,8 @@ void qmp_chardev_remove(const char *id, Error **errp)
 static void register_types(void)
 {
     register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
-    register_char_driver("socket", qemu_chr_open_socket);
+    register_char_driver_qapi("socket", CHARDEV_BACKEND_KIND_SOCKET,
+                              qemu_chr_parse_socket);
     register_char_driver("udp", qemu_chr_open_udp);
     register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
                               qemu_chr_parse_ringbuf);
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/5] util/qemu-sockets.c: Support specifying IPv4 or IPv6 in socket_dgram()

[Peter Maydell]

Currently you can specify whether you want a UDP chardev backend
to be IPv4 or IPv6 using the ipv4 or ipv6 options if you use the
QemuOpts parsing code in inet_dgram_opts(). However the QMP struct
parsing code in socket_dgram() doesn't provide this flexibility
(which in turn prevents us from converting the UDP backend handling
to the new style QAPI framework).

Use the existing inet_addr_to_opts() function to convert the
remote->inet address to option strings; this handles ipv4 and
ipv6 flags as well as host and port. (It will also convert any
'to' specification, which is harmless as it is ignored in this
context.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 util/qemu-sockets.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 5d38395..4a25585 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -966,8 +966,7 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
     opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
     switch (remote->kind) {
     case SOCKET_ADDRESS_KIND_INET:
-        qemu_opt_set(opts, "host", remote->inet->host);
-        qemu_opt_set(opts, "port", remote->inet->port);
+        inet_addr_to_opts(opts, remote->inet);
         if (local) {
             qemu_opt_set(opts, "localaddr", local->inet->host);
             qemu_opt_set(opts, "localport", local->inet->port);
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/5] qemu-char: Convert udp backend to QAPI

[Peter Maydell]

Convert the udp char backend to the new style QAPI framework.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 qemu-char.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 15 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 4b99382..97a839e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2383,20 +2383,6 @@ static CharDriverState *qemu_chr_open_udp_fd(int fd)
     return chr;
 }
 
-static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
-{
-    Error *local_err = NULL;
-    int fd = -1;
-
-    fd = inet_dgram_opts(opts, &local_err);
-    if (fd < 0) {
-        qerror_report_err(local_err);
-        error_free(local_err);
-        return NULL;
-    }
-    return qemu_chr_open_udp_fd(fd);
-}
-
 /***********************************************************/
 /* TCP Net console */
 
@@ -3447,6 +3433,58 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
     backend->socket->addr = addr;
 }
 
+static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
+                               Error **errp)
+{
+    const char *host = qemu_opt_get(opts, "host");
+    const char *port = qemu_opt_get(opts, "port");
+    const char *localaddr = qemu_opt_get(opts, "localaddr");
+    const char *localport = qemu_opt_get(opts, "localport");
+    bool has_local = false;
+    SocketAddress *addr;
+
+    if (host == NULL || strlen(host) == 0) {
+        host = "localhost";
+    }
+    if (port == NULL || strlen(port) == 0) {
+        error_setg(errp, "chardev: udp: remote port not specified");
+        return;
+    }
+    if (localport == NULL || strlen(localport) == 0) {
+        localport = "0";
+    } else {
+        has_local = true;
+    }
+    if (localaddr == NULL || strlen(localaddr) == 0) {
+        localaddr = "";
+    } else {
+        has_local = true;
+    }
+
+    backend->udp = g_new0(ChardevUdp, 1);
+
+    addr = g_new0(SocketAddress, 1);
+    addr->kind = SOCKET_ADDRESS_KIND_INET;
+    addr->inet = g_new0(InetSocketAddress, 1);
+    addr->inet->host = g_strdup(host);
+    addr->inet->port = g_strdup(port);
+    addr->inet->has_ipv4 = qemu_opt_get(opts, "ipv4");
+    addr->inet->ipv4 = qemu_opt_get_bool(opts, "ipv4", 0);
+    addr->inet->has_ipv6 = qemu_opt_get(opts, "ipv6");
+    addr->inet->ipv6 = qemu_opt_get_bool(opts, "ipv6", 0);
+    backend->udp->remote = addr;
+
+    if (has_local) {
+        backend->udp->has_local = true;
+        addr = g_new0(SocketAddress, 1);
+        addr->kind = SOCKET_ADDRESS_KIND_INET;
+        addr->inet = g_new0(InetSocketAddress, 1);
+        addr->inet->host = g_strdup(localaddr);
+        addr->inet->port = g_strdup(localport);
+        backend->udp->local = addr;
+    }
+}
+
 typedef struct CharDriver {
     const char *name;
     /* old, pre qapi */
@@ -4137,7 +4175,8 @@ static void register_types(void)
     register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
     register_char_driver_qapi("socket", CHARDEV_BACKEND_KIND_SOCKET,
                               qemu_chr_parse_socket);
-    register_char_driver("udp", qemu_chr_open_udp);
+    register_char_driver_qapi("udp", CHARDEV_BACKEND_KIND_UDP,
+                              qemu_chr_parse_udp);
     register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
                               qemu_chr_parse_ringbuf);
     register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/5] qemu-char: Remove register_char_driver() machinery

[Peter Maydell]

Now that all the char backends have been converted to the QAPI
framework we can remove the machinery for handling old style
backends.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/sysemu/char.h |   1 -
 qemu-char.c           | 126 +++++++++++++++-----------------------------------
 2 files changed, 38 insertions(+), 89 deletions(-)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 98cd4c9..dbfd4e6 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -344,7 +344,6 @@ bool chr_is_ringbuf(const CharDriverState *chr);
 
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
 
-void register_char_driver(const char *name, CharDriverState *(*open)(QemuOpts *));
 void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
         void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
 
diff --git a/qemu-char.c b/qemu-char.c
index 97a839e..33d613f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3487,26 +3487,12 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
 
 typedef struct CharDriver {
     const char *name;
-    /* old, pre qapi */
-    CharDriverState *(*open)(QemuOpts *opts);
-    /* new, qapi-based */
     ChardevBackendKind kind;
     void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
 } CharDriver;
 
 static GSList *backends;
 
-void register_char_driver(const char *name, CharDriverState *(*open)(QemuOpts *))
-{
-    CharDriver *s;
-
-    s = g_malloc0(sizeof(*s));
-    s->name = g_strdup(name);
-    s->open = open;
-
-    backends = g_slist_append(backends, s);
-}
-
 void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
         void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp))
 {
@@ -3528,8 +3514,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
     CharDriver *cd;
     CharDriverState *chr;
     GSList *i;
+    ChardevReturn *ret = NULL;
+    ChardevBackend *backend;
+    const char *id = qemu_opts_id(opts);
+    char *bid = NULL;
 
-    if (qemu_opts_id(opts) == NULL) {
+    if (id == NULL) {
         error_setg(errp, "chardev: no id specified");
         goto err;
     }
@@ -3552,89 +3542,49 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         goto err;
     }
 
-    if (!cd->open) {
-        /* using new, qapi init */
-        ChardevBackend *backend = g_new0(ChardevBackend, 1);
-        ChardevReturn *ret = NULL;
-        const char *id = qemu_opts_id(opts);
-        char *bid = NULL;
+    backend = g_new0(ChardevBackend, 1);
 
-        if (qemu_opt_get_bool(opts, "mux", 0)) {
-            bid = g_strdup_printf("%s-base", id);
-        }
+    if (qemu_opt_get_bool(opts, "mux", 0)) {
+        bid = g_strdup_printf("%s-base", id);
+    }
 
-        chr = NULL;
-        backend->kind = cd->kind;
-        if (cd->parse) {
-            cd->parse(opts, backend, &local_err);
-            if (local_err) {
-                error_propagate(errp, local_err);
-                goto qapi_out;
-            }
-        }
-        ret = qmp_chardev_add(bid ? bid : id, backend, errp);
-        if (!ret) {
+    chr = NULL;
+    backend->kind = cd->kind;
+    if (cd->parse) {
+        cd->parse(opts, backend, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
             goto qapi_out;
         }
-
-        if (bid) {
-            qapi_free_ChardevBackend(backend);
-            qapi_free_ChardevReturn(ret);
-            backend = g_new0(ChardevBackend, 1);
-            backend->mux = g_new0(ChardevMux, 1);
-            backend->kind = CHARDEV_BACKEND_KIND_MUX;
-            backend->mux->chardev = g_strdup(bid);
-            ret = qmp_chardev_add(id, backend, errp);
-            if (!ret) {
-                chr = qemu_chr_find(bid);
-                qemu_chr_delete(chr);
-                chr = NULL;
-                goto qapi_out;
-            }
-        }
-
-        chr = qemu_chr_find(id);
-        chr->opts = opts;
-
-    qapi_out:
-        qapi_free_ChardevBackend(backend);
-        qapi_free_ChardevReturn(ret);
-        g_free(bid);
-        return chr;
     }
-
-    chr = cd->open(opts);
-    if (!chr) {
-        error_setg(errp, "chardev: opening backend \"%s\" failed",
-                   qemu_opt_get(opts, "backend"));
-        goto err;
+    ret = qmp_chardev_add(bid ? bid : id, backend, errp);
+    if (!ret) {
+        goto qapi_out;
     }
 
-    if (!chr->filename)
-        chr->filename = g_strdup(qemu_opt_get(opts, "backend"));
-    chr->init = init;
-    /* if we didn't create the chardev via qmp_chardev_add, we
-     * need to send the OPENED event here
-     */
-    if (!chr->explicit_be_open) {
-        qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+    if (bid) {
+        qapi_free_ChardevBackend(backend);
+        qapi_free_ChardevReturn(ret);
+        backend = g_new0(ChardevBackend, 1);
+        backend->mux = g_new0(ChardevMux, 1);
+        backend->kind = CHARDEV_BACKEND_KIND_MUX;
+        backend->mux->chardev = g_strdup(bid);
+        ret = qmp_chardev_add(id, backend, errp);
+        if (!ret) {
+            chr = qemu_chr_find(bid);
+            qemu_chr_delete(chr);
+            chr = NULL;
+            goto qapi_out;
+        }
     }
-    QTAILQ_INSERT_TAIL(&chardevs, chr, next);
 
-    if (qemu_opt_get_bool(opts, "mux", 0)) {
-        CharDriverState *base = chr;
-        int len = strlen(qemu_opts_id(opts)) + 6;
-        base->label = g_malloc(len);
-        snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
-        chr = qemu_chr_open_mux(base);
-        chr->filename = base->filename;
-        chr->avail_connections = MAX_MUX;
-        QTAILQ_INSERT_TAIL(&chardevs, chr, next);
-    } else {
-        chr->avail_connections = 1;
-    }
-    chr->label = g_strdup(qemu_opts_id(opts));
+    chr = qemu_chr_find(id);
     chr->opts = opts;
+
+qapi_out:
+    qapi_free_ChardevBackend(backend);
+    qapi_free_ChardevReturn(ret);
+    g_free(bid);
     return chr;
 
 err:
-- 
1.9.1

[Qemu-devel] [PATCH v2 5/5] qemu-char: Rename register_char_driver_qapi() to register_char_driver()

[Peter Maydell]

Now we have removed the legacy register_char_driver() we can
rename register_char_driver_qapi() to the more obvious and
shorter name.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 backends/baum.c       |  2 +-
 backends/msmouse.c    |  2 +-
 backends/testdev.c    |  2 +-
 include/sysemu/char.h |  2 +-
 qemu-char.c           | 54 +++++++++++++++++++++++++--------------------------
 spice-qemu-char.c     |  8 ++++----
 ui/console.c          |  3 +--
 7 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/backends/baum.c b/backends/baum.c
index 796512d..a69aaff 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -629,7 +629,7 @@ fail_handle:
 
 static void register_types(void)
 {
-    register_char_driver_qapi("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL);
+    register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL);
 }
 
 type_init(register_types);
diff --git a/backends/msmouse.c b/backends/msmouse.c
index 650a531..0119110 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -79,7 +79,7 @@ CharDriverState *qemu_chr_open_msmouse(void)
 
 static void register_types(void)
 {
-    register_char_driver_qapi("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL);
+    register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL);
 }
 
 type_init(register_types);
diff --git a/backends/testdev.c b/backends/testdev.c
index 70d63b3..eba396a 100644
--- a/backends/testdev.c
+++ b/backends/testdev.c
@@ -125,7 +125,7 @@ CharDriverState *chr_testdev_init(void)
 
 static void register_types(void)
 {
-    register_char_driver_qapi("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL);
+    register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL);
 }
 
 type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index dbfd4e6..832b7fe 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -344,7 +344,7 @@ bool chr_is_ringbuf(const CharDriverState *chr);
 
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
 
-void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
+void register_char_driver(const char *name, ChardevBackendKind kind,
         void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
 
 /* add an eventfd to the qemu devices that are polled */
diff --git a/qemu-char.c b/qemu-char.c
index 33d613f..82c8d09 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3493,7 +3493,7 @@ typedef struct CharDriver {
 
 static GSList *backends;
 
-void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
+void register_char_driver(const char *name, ChardevBackendKind kind,
         void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp))
 {
     CharDriver *s;
@@ -4122,34 +4122,32 @@ void qmp_chardev_remove(const char *id, Error **errp)
 
 static void register_types(void)
 {
-    register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
-    register_char_driver_qapi("socket", CHARDEV_BACKEND_KIND_SOCKET,
-                              qemu_chr_parse_socket);
-    register_char_driver_qapi("udp", CHARDEV_BACKEND_KIND_UDP,
-                              qemu_chr_parse_udp);
-    register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
-                              qemu_chr_parse_ringbuf);
-    register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,
-                              qemu_chr_parse_file_out);
-    register_char_driver_qapi("stdio", CHARDEV_BACKEND_KIND_STDIO,
-                              qemu_chr_parse_stdio);
-    register_char_driver_qapi("serial", CHARDEV_BACKEND_KIND_SERIAL,
-                              qemu_chr_parse_serial);
-    register_char_driver_qapi("tty", CHARDEV_BACKEND_KIND_SERIAL,
-                              qemu_chr_parse_serial);
-    register_char_driver_qapi("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
-                              qemu_chr_parse_parallel);
-    register_char_driver_qapi("parport", CHARDEV_BACKEND_KIND_PARALLEL,
-                              qemu_chr_parse_parallel);
-    register_char_driver_qapi("pty", CHARDEV_BACKEND_KIND_PTY, NULL);
-    register_char_driver_qapi("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL);
-    register_char_driver_qapi("pipe", CHARDEV_BACKEND_KIND_PIPE,
-                              qemu_chr_parse_pipe);
-    register_char_driver_qapi("mux", CHARDEV_BACKEND_KIND_MUX,
-                              qemu_chr_parse_mux);
+    register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL);
+    register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
+                         qemu_chr_parse_socket);
+    register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp);
+    register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
+                         qemu_chr_parse_ringbuf);
+    register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
+                         qemu_chr_parse_file_out);
+    register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
+                         qemu_chr_parse_stdio);
+    register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
+                         qemu_chr_parse_serial);
+    register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
+                         qemu_chr_parse_serial);
+    register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
+                         qemu_chr_parse_parallel);
+    register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
+                         qemu_chr_parse_parallel);
+    register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL);
+    register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL);
+    register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
+                         qemu_chr_parse_pipe);
+    register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux);
     /* Bug-compatibility: */
-    register_char_driver_qapi("memory", CHARDEV_BACKEND_KIND_MEMORY,
-                              qemu_chr_parse_ringbuf);
+    register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
+                         qemu_chr_parse_ringbuf);
     /* this must be done after machine init, since we register FEs with muxes
      * as part of realize functions like serial_isa_realizefn when -nographic
      * is specified
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 4518a4d..8106e06 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -368,10 +368,10 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend *backend,
 
 static void register_types(void)
 {
-    register_char_driver_qapi("spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
-                              qemu_chr_parse_spice_vmc);
-    register_char_driver_qapi("spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
-                              qemu_chr_parse_spice_port);
+    register_char_driver("spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
+                         qemu_chr_parse_spice_vmc);
+    register_char_driver("spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
+                         qemu_chr_parse_spice_port);
 }
 
 type_init(register_types);
diff --git a/ui/console.c b/ui/console.c
index ab84549..3248d60 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2066,8 +2066,7 @@ static const TypeInfo qemu_console_info = {
 static void register_types(void)
 {
     type_register_static(&qemu_console_info);
-    register_char_driver_qapi("vc", CHARDEV_BACKEND_KIND_VC,
-                              qemu_chr_parse_vc);
+    register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc);
 }
 
 type_init(register_types);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 3/5] qemu-char: Convert udp backend to QAPI

[Eric Blake]

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

On 09/02/2014 04:24 AM, Peter Maydell wrote:
> Convert the udp char backend to the new style QAPI framework.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  qemu-char.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 54 insertions(+), 15 deletions(-)
> 

> +
> +    if (host == NULL || strlen(host) == 0) {
> +        host = "localhost";
> +    }
> +    if (port == NULL || strlen(port) == 0) {
> +        error_setg(errp, "chardev: udp: remote port not specified");

In the common case of these strings being non-empty, you end up having
to hunt for the end of the string only to then throw that information
away.  Rather than 'strlen(foo) == 0)', it's slightly faster to check
'*foo' for being a non-NUL byte.

-- 
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: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 3/5] qemu-char: Convert udp backend to QAPI

[Peter Maydell]

On 2 September 2014 21:04, Eric Blake <eblake@redhat.com> wrote:
> On 09/02/2014 04:24 AM, Peter Maydell wrote:
>> +
>> +    if (host == NULL || strlen(host) == 0) {
>> +        host = "localhost";
>> +    }
>> +    if (port == NULL || strlen(port) == 0) {
>> +        error_setg(errp, "chardev: udp: remote port not specified");
>
> In the common case of these strings being non-empty, you end up having
> to hunt for the end of the string only to then throw that information
> away.  Rather than 'strlen(foo) == 0)', it's slightly faster to check
> '*foo' for being a non-NUL byte.

I think that's a lot less clear to read, and in fact the
compiler is entirely capable of turning "strlen(x) == 0"
into "check whether *x is 0":

mnementh$ cat /tmp/zz9.c
#include <string.h>

int is_empty_string(const char *s)
{
    return (strlen(s) == 0);
}
mnementh$ gcc -g -Wall -O2 -o /tmp/zz9.o -c /tmp/zz9.c
mnementh$ objdump --disassemble /tmp/zz9.o

/tmp/zz9.o:     file format elf32-i386


Disassembly of section .text:

00000000 <is_empty_string>:
   0:    8b 44 24 04              mov    0x4(%esp),%eax
   4:    80 38 00                 cmpb   $0x0,(%eax)
   7:    0f 94 c0                 sete   %al
   a:    0f b6 c0                 movzbl %al,%eax
   d:    c3                       ret

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/5] Convert remaining legacy chardevs to QAPI

[Paolo Bonzini]

Il 02/09/2014 12:24, Peter Maydell ha scritto:
> This patchset converts the two remaining legacy chardevs
> ('socket' and 'udp') to use the new-style parse/kind
> mechanisms, and removes all the no-longer-required
> legacy machinery.
> 
> Patch 1 was posted to the list back in June
> (https://patches.linaro.org/32298/). I've fixed the obvious
> bug picked up in code review, and as far as I can tell from
> the thread we decided that the blocking/non-blocking
> difference between QAPI and legacy wasn't a problem.
> 
> Patch 2 fixes a hole in the functionality of QAPI-described
> UDP chardevs, to avoid regressing the commandline functionality
> when we convert the UDP backend in patch 3.
> 
> Patch 4 may be easier to review as an ignore-whitespaces
> diff (the de-indentation makes the diff a bit awkward).
> 
> Changes v1->v2:
>  * fixed the has_* values as suggested by Markus
>  * added patch 5 which renames the _qapi() function now
>    the legacy version has gone (again, as suggested by
>    Markus)
> 
> Peter Maydell (5):
>   qemu-char: Convert socket backend to QAPI
>   util/qemu-sockets.c: Support specifying IPv4 or IPv6 in socket_dgram()
>   qemu-char: Convert udp backend to QAPI
>   qemu-char: Remove register_char_driver() machinery
>   qemu-char: Rename register_char_driver_qapi() to
>     register_char_driver()
> 
>  backends/baum.c       |   2 +-
>  backends/msmouse.c    |   2 +-
>  backends/testdev.c    |   2 +-
>  include/sysemu/char.h |   3 +-
>  qemu-char.c           | 353 ++++++++++++++++++++++++--------------------------
>  spice-qemu-char.c     |   8 +-
>  ui/console.c          |   3 +-
>  util/qemu-sockets.c   |   3 +-
>  8 files changed, 180 insertions(+), 196 deletions(-)
> 

Hi Peter, are you going to apply these directly?

Paolo

Re: [Qemu-devel] [PATCH v2 0/5] Convert remaining legacy chardevs to QAPI

[Peter Maydell]

On 9 September 2014 07:47, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 02/09/2014 12:24, Peter Maydell ha scritto:
>> This patchset converts the two remaining legacy chardevs
>> ('socket' and 'udp') to use the new-style parse/kind
>> mechanisms, and removes all the no-longer-required
>> legacy machinery.

>> Peter Maydell (5):
>>   qemu-char: Convert socket backend to QAPI
>>   util/qemu-sockets.c: Support specifying IPv4 or IPv6 in socket_dgram()
>>   qemu-char: Convert udp backend to QAPI
>>   qemu-char: Remove register_char_driver() machinery
>>   qemu-char: Rename register_char_driver_qapi() to
>>     register_char_driver()
>>
>>  backends/baum.c       |   2 +-
>>  backends/msmouse.c    |   2 +-
>>  backends/testdev.c    |   2 +-
>>  include/sysemu/char.h |   3 +-
>>  qemu-char.c           | 353 ++++++++++++++++++++++++--------------------------
>>  spice-qemu-char.c     |   8 +-
>>  ui/console.c          |   3 +-
>>  util/qemu-sockets.c   |   3 +-
>>  8 files changed, 180 insertions(+), 196 deletions(-)
>>
>
> Hi Peter, are you going to apply these directly?

I guess I could; I was hoping for some review first though :-)

-- PMM

Re: [Qemu-devel] [PATCH v2 0/5] Convert remaining legacy chardevs to QAPI

[Peter Maydell]

On 9 September 2014 01:11, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 September 2014 07:47, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 02/09/2014 12:24, Peter Maydell ha scritto:
>>> This patchset converts the two remaining legacy chardevs
>>> ('socket' and 'udp') to use the new-style parse/kind
>>> mechanisms, and removes all the no-longer-required
>>> legacy machinery.
>
>>> Peter Maydell (5):
>>>   qemu-char: Convert socket backend to QAPI
>>>   util/qemu-sockets.c: Support specifying IPv4 or IPv6 in socket_dgram()
>>>   qemu-char: Convert udp backend to QAPI
>>>   qemu-char: Remove register_char_driver() machinery
>>>   qemu-char: Rename register_char_driver_qapi() to
>>>     register_char_driver()

>> Hi Peter, are you going to apply these directly?
>
> I guess I could; I was hoping for some review first though :-)

Well, no more review, so I've applied these to master.

-- PMM