All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation
@ 2015-10-12  8:03 Paolo Bonzini
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
                   ` (20 more replies)
  0 siblings, 21 replies; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

This series rewrites chardev creation to use a new ->create
member of the CharDriver struct, and to always signal errors
via Error*.

The advantage is that backend-specific creation functions need
not be exported anymore for qemu-char.c's usage, and hence do not
need stubs anymore.

Paolo Bonzini (21):
  qemu-char: cleanup qmp_chardev_add
  qemu-char: cleanup HAVE_CHARDEV_*
  qemu-char: add create to register_char_driver
  qemu-char: convert file backend to data-driven creation
  qemu-char: convert serial backend to data-driven creation
  qemu-char: convert parallel backend to data-driven creation
  qemu-char: convert pipe backend to data-driven creation
  qemu-char: convert socket backend to data-driven creation
  qemu-char: convert UDP backend to data-driven creation
  qemu-char: convert pty backend to data-driven creation
  qemu-char: convert null backend to data-driven creation
  qemu-char: convert mux backend to data-driven creation
  qemu-char: convert msmouse backend to data-driven creation
  qemu-char: convert braille backend to data-driven creation
  qemu-char: convert testdev backend to data-driven creation
  qemu-char: convert stdio backend to data-driven creation
  qemu-char: convert console backend to data-driven creation
  qemu-char: convert spice backend to data-driven creation
  qemu-char: convert vc backend to data-driven creation
  qemu-char: convert ringbuf backend to data-driven creation
  qemu-char: cleanup after completed conversion to cd->create

 backends/baum.c             |  14 +-
 backends/msmouse.c          |   8 +-
 backends/testdev.c          |   8 +-
 include/sysemu/char.h       |  18 +-
 include/ui/qemu-spice.h     |   2 -
 qemu-char.c                 | 392 ++++++++++++++++++++++++--------------------
 spice-qemu-char.c           |  21 ++-
 stubs/Makefile.objs         |   5 -
 stubs/chr-baum-init.c       |   7 -
 stubs/chr-msmouse.c         |   7 -
 stubs/chr-testdev.c         |   7 -
 stubs/qemu-chr-open-spice.c |  14 --
 stubs/vc-init.c             |   7 -
 ui/console.c                |  11 +-
 ui/gtk.c                    |   2 +-
 15 files changed, 257 insertions(+), 266 deletions(-)
 delete mode 100644 stubs/chr-baum-init.c
 delete mode 100644 stubs/chr-msmouse.c
 delete mode 100644 stubs/chr-testdev.c
 delete mode 100644 stubs/qemu-chr-open-spice.c
 delete mode 100644 stubs/vc-init.c

-- 
2.5.0

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 14:55   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_* Paolo Bonzini
                   ` (19 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Use the usual idioms for error propagation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 56 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 653ea10..f51c0aa 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4214,6 +4214,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
 {
     ChardevReturn *ret = g_new0(ChardevReturn, 1);
     CharDriverState *base, *chr = NULL;
+    Error *local_err = NULL;
 
     chr = qemu_chr_find(id);
     if (chr) {
@@ -4224,22 +4225,22 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
 
     switch (backend->kind) {
     case CHARDEV_BACKEND_KIND_FILE:
-        chr = qmp_chardev_open_file(backend->file, errp);
+        chr = qmp_chardev_open_file(backend->file, &local_err);
         break;
     case CHARDEV_BACKEND_KIND_SERIAL:
-        chr = qmp_chardev_open_serial(backend->serial, errp);
+        chr = qmp_chardev_open_serial(backend->serial, &local_err);
         break;
     case CHARDEV_BACKEND_KIND_PARALLEL:
-        chr = qmp_chardev_open_parallel(backend->parallel, errp);
+        chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
         break;
     case CHARDEV_BACKEND_KIND_PIPE:
         chr = qemu_chr_open_pipe(backend->pipe);
         break;
     case CHARDEV_BACKEND_KIND_SOCKET:
-        chr = qmp_chardev_open_socket(backend->socket, errp);
+        chr = qmp_chardev_open_socket(backend->socket, &local_err);
         break;
     case CHARDEV_BACKEND_KIND_UDP:
-        chr = qmp_chardev_open_udp(backend->udp, errp);
+        chr = qmp_chardev_open_udp(backend->udp, &local_err);
         break;
 #ifdef HAVE_CHARDEV_TTY
     case CHARDEV_BACKEND_KIND_PTY:
@@ -4252,7 +4253,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     case CHARDEV_BACKEND_KIND_MUX:
         base = qemu_chr_find(backend->mux->chardev);
         if (base == NULL) {
-            error_setg(errp, "mux: base chardev %s not found",
+            error_setg(&local_err, "mux: base chardev %s not found",
                        backend->mux->chardev);
             break;
         }
@@ -4290,11 +4291,11 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         break;
     case CHARDEV_BACKEND_KIND_RINGBUF:
     case CHARDEV_BACKEND_KIND_MEMORY:
-        chr = qemu_chr_open_ringbuf(backend->ringbuf, errp);
+        chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
         break;
     default:
         error_setg(errp, "unknown chardev backend (%d)", backend->kind);
-        break;
+        goto out_error;
     }
 
     /*
@@ -4303,25 +4304,30 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
      * error then.
      * TODO full conversion to Error API
      */
-    if (chr == NULL && errp && !*errp) {
-        error_setg(errp, "Failed to create chardev");
-    }
-    if (chr) {
-        chr->label = g_strdup(id);
-        chr->avail_connections =
-            (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
-        if (!chr->filename) {
-            chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
-        }
-        if (!chr->explicit_be_open) {
-            qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+    if (chr == NULL) {
+        if (local_err) {
+            error_propagate(errp, local_err);
+        } else {
+            error_setg(errp, "Failed to create chardev");
         }
-        QTAILQ_INSERT_TAIL(&chardevs, chr, next);
-        return ret;
-    } else {
-        g_free(ret);
-        return NULL;
+        goto out_error;
+    }
+
+    chr->label = g_strdup(id);
+    chr->avail_connections =
+        (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
+    if (!chr->filename) {
+        chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
     }
+    if (!chr->explicit_be_open) {
+        qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+    }
+    QTAILQ_INSERT_TAIL(&chardevs, chr, next);
+    return ret;
+
+out_error:
+    g_free(ret);
+    return NULL;
 }
 
 void qmp_chardev_remove(const char *id, Error **errp)
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_*
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 14:57   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver Paolo Bonzini
                   ` (18 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Move the #ifdef up into qmp_chardev_add, and avoid duplicating
the code that reports unavailable backends.  Split HAVE_CHARDEV_TTY
into HAVE_CHARDEV_SERIAL and HAVE_CHARDEV_PTY.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index f51c0aa..7219d56 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1196,7 +1196,8 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
     || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
     || defined(__GLIBC__)
 
-#define HAVE_CHARDEV_TTY 1
+#define HAVE_CHARDEV_SERIAL 1
+#define HAVE_CHARDEV_PTY 1
 
 typedef struct {
     GIOChannel *fd;
@@ -1832,6 +1833,8 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd)
 
 #else /* _WIN32 */
 
+#define HAVE_CHARDEV_SERIAL 1
+
 typedef struct {
     int max_size;
     HANDLE hcom, hrecv, hsend;
@@ -4069,10 +4072,10 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
     return qemu_chr_open_fd(in, out);
 }
 
+#ifdef HAVE_CHARDEV_SERIAL
 static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
                                                 Error **errp)
 {
-#ifdef HAVE_CHARDEV_TTY
     int fd;
 
     fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -4081,16 +4084,12 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
     }
     qemu_set_nonblock(fd);
     return qemu_chr_open_tty_fd(fd);
-#else
-    error_setg(errp, "character device backend type 'serial' not supported");
-    return NULL;
-#endif
 }
 
+#ifdef HAVE_CHARDEV_PARPORT
 static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
                                                   Error **errp)
 {
-#ifdef HAVE_CHARDEV_PARPORT
     int fd;
 
     fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
@@ -4098,11 +4097,8 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
         return NULL;
     }
     return qemu_chr_open_pp_fd(fd);
-#else
-    error_setg(errp, "character device backend type 'parallel' not supported");
-    return NULL;
-#endif
 }
+#endif
 
 #endif /* WIN32 */
 
@@ -4227,12 +4223,16 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     case CHARDEV_BACKEND_KIND_FILE:
         chr = qmp_chardev_open_file(backend->file, &local_err);
         break;
+#ifdef HAVE_CHARDEV_SERIAL
     case CHARDEV_BACKEND_KIND_SERIAL:
         chr = qmp_chardev_open_serial(backend->serial, &local_err);
         break;
+#endif
+#ifdef HAVE_CHARDEV_PARPORT
     case CHARDEV_BACKEND_KIND_PARALLEL:
         chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
         break;
+#endif
     case CHARDEV_BACKEND_KIND_PIPE:
         chr = qemu_chr_open_pipe(backend->pipe);
         break;
@@ -4242,7 +4242,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     case CHARDEV_BACKEND_KIND_UDP:
         chr = qmp_chardev_open_udp(backend->udp, &local_err);
         break;
-#ifdef HAVE_CHARDEV_TTY
+#ifdef HAVE_CHARDEV_PTY
     case CHARDEV_BACKEND_KIND_PTY:
         chr = qemu_chr_open_pty(id, ret);
         break;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_* Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:04   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation Paolo Bonzini
                   ` (17 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Having creation as a member of the CharDriver struct removes the need
to export functions for qemu-char.c's usage.  After the conversion,
chardev backends implemented outside qemu-char.c will not need a stub
creation function anymore.

Ultimately all drivers will be converted.  For now, support the case
where cd->create == NULL.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/baum.c       |   3 +-
 backends/msmouse.c    |   3 +-
 backends/testdev.c    |   3 +-
 include/sysemu/char.h |   4 +-
 qemu-char.c           | 213 ++++++++++++++++++++++++++++----------------------
 spice-qemu-char.c     |   4 +-
 ui/console.c          |   3 +-
 7 files changed, 133 insertions(+), 100 deletions(-)

diff --git a/backends/baum.c b/backends/baum.c
index a17f625..e86a019 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -629,7 +629,8 @@ fail_handle:
 
 static void register_types(void)
 {
-    register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL);
+    register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL,
+                         NULL);
 }
 
 type_init(register_types);
diff --git a/backends/msmouse.c b/backends/msmouse.c
index 0119110..d50ed47 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -79,7 +79,8 @@ CharDriverState *qemu_chr_open_msmouse(void)
 
 static void register_types(void)
 {
-    register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL);
+    register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL,
+                         NULL);
 }
 
 type_init(register_types);
diff --git a/backends/testdev.c b/backends/testdev.c
index 1429152..43787f6 100644
--- a/backends/testdev.c
+++ b/backends/testdev.c
@@ -125,7 +125,8 @@ CharDriverState *chr_testdev_init(void)
 
 static void register_types(void)
 {
-    register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL);
+    register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL,
+                         NULL);
 }
 
 type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 832b7fe..4b01a8c 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -345,7 +345,9 @@ 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, ChardevBackendKind kind,
-        void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
+        void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp),
+        CharDriverState *(*create)(const char *id, ChardevBackend *backend,
+                                   ChardevReturn *ret, Error **errp));
 
 /* add an eventfd to the qemu devices that are polled */
 CharDriverState *qemu_chr_open_eventfd(int eventfd);
diff --git a/qemu-char.c b/qemu-char.c
index 7219d56..a6411d6 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3644,12 +3644,16 @@ typedef struct CharDriver {
     const char *name;
     ChardevBackendKind kind;
     void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
+    CharDriverState *(*create)(const char *id, ChardevBackend *backend,
+                               ChardevReturn *ret, Error **errp);
 } CharDriver;
 
 static GSList *backends;
 
 void register_char_driver(const char *name, ChardevBackendKind kind,
-        void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp))
+        void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp),
+        CharDriverState *(*create)(const char *id, ChardevBackend *backend,
+                                   ChardevReturn *ret, Error **errp))
 {
     CharDriver *s;
 
@@ -3657,6 +3661,7 @@ void register_char_driver(const char *name, ChardevBackendKind kind,
     s->name = g_strdup(name);
     s->kind = kind;
     s->parse = parse;
+    s->create = create;
 
     backends = g_slist_append(backends, s);
 }
@@ -4211,6 +4216,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     ChardevReturn *ret = g_new0(ChardevReturn, 1);
     CharDriverState *base, *chr = NULL;
     Error *local_err = NULL;
+    GSList *i;
+    CharDriver *cd;
 
     chr = qemu_chr_find(id);
     if (chr) {
@@ -4219,98 +4226,113 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         return NULL;
     }
 
-    switch (backend->kind) {
-    case CHARDEV_BACKEND_KIND_FILE:
-        chr = qmp_chardev_open_file(backend->file, &local_err);
-        break;
+    for (i = backends; i; i = i->next) {
+        cd = i->data;
+
+        if (cd->kind == backend->kind && cd->create) {
+            chr = cd->create(id, backend, ret, &local_err);
+            if (local_err) {
+                error_propagate(errp, local_err);
+                goto out_error;
+            }
+            break;
+        }
+    }
+
+    if (chr == NULL) {
+        switch (backend->kind) {
+        case CHARDEV_BACKEND_KIND_FILE:
+            chr = qmp_chardev_open_file(backend->file, &local_err);
+            break;
 #ifdef HAVE_CHARDEV_SERIAL
-    case CHARDEV_BACKEND_KIND_SERIAL:
-        chr = qmp_chardev_open_serial(backend->serial, &local_err);
-        break;
+        case CHARDEV_BACKEND_KIND_SERIAL:
+            chr = qmp_chardev_open_serial(backend->serial, &local_err);
+            break;
 #endif
 #ifdef HAVE_CHARDEV_PARPORT
-    case CHARDEV_BACKEND_KIND_PARALLEL:
-        chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
-        break;
+        case CHARDEV_BACKEND_KIND_PARALLEL:
+            chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
+            break;
 #endif
-    case CHARDEV_BACKEND_KIND_PIPE:
-        chr = qemu_chr_open_pipe(backend->pipe);
-        break;
-    case CHARDEV_BACKEND_KIND_SOCKET:
-        chr = qmp_chardev_open_socket(backend->socket, &local_err);
-        break;
-    case CHARDEV_BACKEND_KIND_UDP:
-        chr = qmp_chardev_open_udp(backend->udp, &local_err);
-        break;
+        case CHARDEV_BACKEND_KIND_PIPE:
+            chr = qemu_chr_open_pipe(backend->pipe);
+            break;
+        case CHARDEV_BACKEND_KIND_SOCKET:
+            chr = qmp_chardev_open_socket(backend->socket, &local_err);
+            break;
+        case CHARDEV_BACKEND_KIND_UDP:
+            chr = qmp_chardev_open_udp(backend->udp, &local_err);
+            break;
 #ifdef HAVE_CHARDEV_PTY
-    case CHARDEV_BACKEND_KIND_PTY:
-        chr = qemu_chr_open_pty(id, ret);
-        break;
+        case CHARDEV_BACKEND_KIND_PTY:
+            chr = qemu_chr_open_pty(id, ret);
+            break;
 #endif
-    case CHARDEV_BACKEND_KIND_NULL:
-        chr = qemu_chr_open_null();
-        break;
-    case CHARDEV_BACKEND_KIND_MUX:
-        base = qemu_chr_find(backend->mux->chardev);
-        if (base == NULL) {
-            error_setg(&local_err, "mux: base chardev %s not found",
-                       backend->mux->chardev);
+        case CHARDEV_BACKEND_KIND_NULL:
+            chr = qemu_chr_open_null();
+            break;
+        case CHARDEV_BACKEND_KIND_MUX:
+            base = qemu_chr_find(backend->mux->chardev);
+            if (base == NULL) {
+                error_setg(&local_err, "mux: base chardev %s not found",
+                           backend->mux->chardev);
+                break;
+            }
+            chr = qemu_chr_open_mux(base);
+            break;
+        case CHARDEV_BACKEND_KIND_MSMOUSE:
+            chr = qemu_chr_open_msmouse();
             break;
-        }
-        chr = qemu_chr_open_mux(base);
-        break;
-    case CHARDEV_BACKEND_KIND_MSMOUSE:
-        chr = qemu_chr_open_msmouse();
-        break;
 #ifdef CONFIG_BRLAPI
-    case CHARDEV_BACKEND_KIND_BRAILLE:
-        chr = chr_baum_init();
-        break;
+        case CHARDEV_BACKEND_KIND_BRAILLE:
+            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;
+        case CHARDEV_BACKEND_KIND_TESTDEV:
+            chr = chr_testdev_init();
+            break;
+        case CHARDEV_BACKEND_KIND_STDIO:
+            chr = qemu_chr_open_stdio(backend->stdio);
+            break;
 #ifdef _WIN32
-    case CHARDEV_BACKEND_KIND_CONSOLE:
-        chr = qemu_chr_open_win_con();
-        break;
+        case CHARDEV_BACKEND_KIND_CONSOLE:
+            chr = qemu_chr_open_win_con();
+            break;
 #endif
 #ifdef CONFIG_SPICE
-    case CHARDEV_BACKEND_KIND_SPICEVMC:
-        chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
-        break;
-    case CHARDEV_BACKEND_KIND_SPICEPORT:
-        chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
-        break;
+        case CHARDEV_BACKEND_KIND_SPICEVMC:
+            chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
+            break;
+        case CHARDEV_BACKEND_KIND_SPICEPORT:
+            chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
+            break;
 #endif
-    case CHARDEV_BACKEND_KIND_VC:
-        chr = vc_init(backend->vc);
-        break;
-    case CHARDEV_BACKEND_KIND_RINGBUF:
-    case CHARDEV_BACKEND_KIND_MEMORY:
-        chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
-        break;
-    default:
-        error_setg(errp, "unknown chardev backend (%d)", backend->kind);
-        goto out_error;
-    }
+        case CHARDEV_BACKEND_KIND_VC:
+            chr = vc_init(backend->vc);
+            break;
+        case CHARDEV_BACKEND_KIND_RINGBUF:
+        case CHARDEV_BACKEND_KIND_MEMORY:
+            chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
+            break;
+        default:
+            error_setg(errp, "unknown chardev backend (%d)", backend->kind);
+            goto out_error;
+        }
 
-    /*
-     * Character backend open hasn't been fully converted to the Error
-     * API.  Some opens fail without setting an error.  Set a generic
-     * error then.
-     * TODO full conversion to Error API
-     */
-    if (chr == NULL) {
-        if (local_err) {
-            error_propagate(errp, local_err);
-        } else {
-            error_setg(errp, "Failed to create chardev");
+        /*
+         * Character backend open hasn't been fully converted to the Error
+         * API.  Some opens fail without setting an error.  Set a generic
+         * error then.
+         * TODO full conversion to Error API
+         */
+        if (chr == NULL) {
+            if (local_err) {
+                error_propagate(errp, local_err);
+            } else {
+                error_setg(errp, "Failed to create chardev");
+            }
+            goto out_error;
         }
-        goto out_error;
     }
 
     chr->label = g_strdup(id);
@@ -4349,32 +4371,37 @@ void qmp_chardev_remove(const char *id, Error **errp)
 
 static void register_types(void)
 {
-    register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL);
+    register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, 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);
+                         qemu_chr_parse_socket, NULL);
+    register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
+                         NULL);
     register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
-                         qemu_chr_parse_ringbuf);
+                         qemu_chr_parse_ringbuf, NULL);
     register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
-                         qemu_chr_parse_file_out);
+                         qemu_chr_parse_file_out, NULL);
     register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
-                         qemu_chr_parse_stdio);
+                         qemu_chr_parse_stdio, NULL);
     register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
-                         qemu_chr_parse_serial);
+                         qemu_chr_parse_serial, NULL);
     register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
-                         qemu_chr_parse_serial);
+                         qemu_chr_parse_serial, NULL);
     register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
-                         qemu_chr_parse_parallel);
+                         qemu_chr_parse_parallel, NULL);
     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);
+                         qemu_chr_parse_parallel, NULL);
+    register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
+                         NULL);
+    register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
+                         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);
+                         qemu_chr_parse_pipe, NULL);
+    register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
+                         NULL);
     /* Bug-compatibility: */
     register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
-                         qemu_chr_parse_ringbuf);
+                         qemu_chr_parse_ringbuf, NULL);
     /* 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 d41bb74..e4353ef 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -379,9 +379,9 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend *backend,
 static void register_types(void)
 {
     register_char_driver("spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
-                         qemu_chr_parse_spice_vmc);
+                         qemu_chr_parse_spice_vmc, NULL);
     register_char_driver("spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
-                         qemu_chr_parse_spice_port);
+                         qemu_chr_parse_spice_port, NULL);
 }
 
 type_init(register_types);
diff --git a/ui/console.c b/ui/console.c
index 75fc492..746b23a 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2030,7 +2030,8 @@ static const TypeInfo qemu_console_info = {
 static void register_types(void)
 {
     type_register_static(&qemu_console_info);
-    register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc);
+    register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc,
+                         NULL);
 }
 
 type_init(register_types);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (2 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:06   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 05/21] qemu-char: convert serial " Paolo Bonzini
                   ` (16 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index a6411d6..13fd394 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4010,8 +4010,12 @@ QemuOptsList qemu_chardev_opts = {
 
 #ifdef _WIN32
 
-static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+static CharDriverState *qmp_chardev_open_file(const char *id,
+                                              ChardevBackend *backend,
+                                              ChardevReturn *ret,
+                                              Error **errp)
 {
+    ChardevFile *file = backend->file;
     HANDLE out;
 
     if (file->has_in) {
@@ -4055,8 +4059,12 @@ static int qmp_chardev_open_file_source(char *src, int flags,
     return fd;
 }
 
-static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+static CharDriverState *qmp_chardev_open_file(const char *id,
+                                              ChardevBackend *backend,
+                                              ChardevReturn *ret,
+                                              Error **errp)
 {
+    ChardevFile *file = backend->file;
     int flags, in = -1, out;
 
     flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
@@ -4242,7 +4250,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     if (chr == NULL) {
         switch (backend->kind) {
         case CHARDEV_BACKEND_KIND_FILE:
-            chr = qmp_chardev_open_file(backend->file, &local_err);
+            abort();
             break;
 #ifdef HAVE_CHARDEV_SERIAL
         case CHARDEV_BACKEND_KIND_SERIAL:
@@ -4380,7 +4388,7 @@ static void register_types(void)
     register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
                          qemu_chr_parse_ringbuf, NULL);
     register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
-                         qemu_chr_parse_file_out, NULL);
+                         qemu_chr_parse_file_out, qmp_chardev_open_file);
     register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
                          qemu_chr_parse_stdio, NULL);
     register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 05/21] qemu-char: convert serial backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (3 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:09   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel " Paolo Bonzini
                   ` (15 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 48 +++++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 13fd394..8567580 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1886,7 +1886,7 @@ static void win_chr_close(CharDriverState *chr)
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
-static int win_chr_init(CharDriverState *chr, const char *filename)
+static int win_chr_init(CharDriverState *chr, const char *filename, Error **errp)
 {
     WinCharState *s = chr->opaque;
     COMMCONFIG comcfg;
@@ -1897,25 +1897,25 @@ static int win_chr_init(CharDriverState *chr, const char *filename)
 
     s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hsend) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        error_setg(errp, "Failed CreateEvent");
         goto fail;
     }
     s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hrecv) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        error_setg(errp, "Failed CreateEvent");
         goto fail;
     }
 
     s->hcom = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, 0, NULL,
                       OPEN_EXISTING, FILE_FLAG_OVERLAPPED, 0);
     if (s->hcom == INVALID_HANDLE_VALUE) {
-        fprintf(stderr, "Failed CreateFile (%lu)\n", GetLastError());
+        error_setg(errp, "Failed CreateFile (%lu)", GetLastError());
         s->hcom = NULL;
         goto fail;
     }
 
     if (!SetupComm(s->hcom, NRECVBUF, NSENDBUF)) {
-        fprintf(stderr, "Failed SetupComm\n");
+        error_setg(errp, "Failed SetupComm");
         goto fail;
     }
 
@@ -1926,23 +1926,23 @@ static int win_chr_init(CharDriverState *chr, const char *filename)
     CommConfigDialog(filename, NULL, &comcfg);
 
     if (!SetCommState(s->hcom, &comcfg.dcb)) {
-        fprintf(stderr, "Failed SetCommState\n");
+        error_setg(errp, "Failed SetCommState");
         goto fail;
     }
 
     if (!SetCommMask(s->hcom, EV_ERR)) {
-        fprintf(stderr, "Failed SetCommMask\n");
+        error_setg(errp, "Failed SetCommMask");
         goto fail;
     }
 
     cto.ReadIntervalTimeout = MAXDWORD;
     if (!SetCommTimeouts(s->hcom, &cto)) {
-        fprintf(stderr, "Failed SetCommTimeouts\n");
+        error_setg(errp, "Failed SetCommTimeouts");
         goto fail;
     }
 
     if (!ClearCommError(s->hcom, &err, &comstat)) {
-        fprintf(stderr, "Failed ClearCommError\n");
+        error_setg(errp, "Failed ClearCommError");
         goto fail;
     }
     qemu_add_polling_cb(win_chr_poll, chr);
@@ -2047,7 +2047,8 @@ static int win_chr_poll(void *opaque)
     return 0;
 }
 
-static CharDriverState *qemu_chr_open_win_path(const char *filename)
+static CharDriverState *qemu_chr_open_win_path(const char *filename,
+                                               Error **errp)
 {
     CharDriverState *chr;
     WinCharState *s;
@@ -2058,7 +2059,7 @@ static CharDriverState *qemu_chr_open_win_path(const char *filename)
     chr->chr_write = win_chr_write;
     chr->chr_close = win_chr_close;
 
-    if (win_chr_init(chr, filename) < 0) {
+    if (win_chr_init(chr, filename, errp) < 0) {
         g_free(s);
         g_free(chr);
         return NULL;
@@ -3465,6 +3466,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
     backend->stdio->signal = qemu_opt_get_bool(opts, "signal", true);
 }
 
+#ifdef HAVE_CHARDEV_SERIAL
 static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
                                   Error **errp)
 {
@@ -3477,6 +3479,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
     backend->serial = g_new0(ChardevHostdev, 1);
     backend->serial->device = g_strdup(device);
 }
+#endif
 
 static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
                                     Error **errp)
@@ -4032,10 +4035,13 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
     return qemu_chr_open_win_file(out);
 }
 
-static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
+static CharDriverState *qmp_chardev_open_serial(const char *id,
+                                                ChardevBackend *backend,
+                                                ChardevReturn *ret,
                                                 Error **errp)
 {
-    return qemu_chr_open_win_path(serial->device);
+    ChardevHostdev *serial = backend->serial;
+    return qemu_chr_open_win_path(serial->device, errp);
 }
 
 static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
@@ -4086,9 +4092,12 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
 }
 
 #ifdef HAVE_CHARDEV_SERIAL
-static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
+static CharDriverState *qmp_chardev_open_serial(const char *id,
+                                                ChardevBackend *backend,
+                                                ChardevReturn *ret,
                                                 Error **errp)
 {
+    ChardevHostdev *serial = backend->serial;
     int fd;
 
     fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -4098,6 +4107,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
     qemu_set_nonblock(fd);
     return qemu_chr_open_tty_fd(fd);
 }
+#endif
 
 #ifdef HAVE_CHARDEV_PARPORT
 static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
@@ -4252,11 +4262,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         case CHARDEV_BACKEND_KIND_FILE:
             abort();
             break;
-#ifdef HAVE_CHARDEV_SERIAL
         case CHARDEV_BACKEND_KIND_SERIAL:
-            chr = qmp_chardev_open_serial(backend->serial, &local_err);
+            abort();
             break;
-#endif
 #ifdef HAVE_CHARDEV_PARPORT
         case CHARDEV_BACKEND_KIND_PARALLEL:
             chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
@@ -4391,10 +4399,12 @@ static void register_types(void)
                          qemu_chr_parse_file_out, qmp_chardev_open_file);
     register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
                          qemu_chr_parse_stdio, NULL);
+#if defined HAVE_CHARDEV_SERIAL
     register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
-                         qemu_chr_parse_serial, NULL);
+                         qemu_chr_parse_serial, qmp_chardev_open_serial);
     register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
-                         qemu_chr_parse_serial, NULL);
+                         qemu_chr_parse_serial, qmp_chardev_open_serial);
+#endif
     register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
                          qemu_chr_parse_parallel, NULL);
     register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (4 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 05/21] qemu-char: convert serial " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:13   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe " Paolo Bonzini
                   ` (14 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Conversion to Error * brings better error messages; before:

    qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: Failed to create chardev

After:

    qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: not a parallel port: Inappropriate ioctl for device

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 8567580..84cb8d0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1753,13 +1753,14 @@ static void pp_close(CharDriverState *chr)
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
 {
     CharDriverState *chr;
     ParallelCharDriver *drv;
 
     if (ioctl(fd, PPCLAIM) < 0) {
         close(fd);
+        error_setg_errno(errp, errno, "not a parallel port");
         return NULL;
     }
 
@@ -1818,7 +1819,7 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
     return 0;
 }
 
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
 {
     CharDriverState *chr;
 
@@ -3481,6 +3482,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
 }
 #endif
 
+#ifdef HAVE_CHARDEV_PARPORT
 static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
                                     Error **errp)
 {
@@ -3493,6 +3495,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
     backend->parallel = g_new0(ChardevHostdev, 1);
     backend->parallel->device = g_strdup(device);
 }
+#endif
 
 static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
                                 Error **errp)
@@ -4044,7 +4047,9 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
     return qemu_chr_open_win_path(serial->device, errp);
 }
 
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
+static CharDriverState *qmp_chardev_open_parallel(const char *id,
+                                                  ChardevBackend *backend,
+                                                  ChardevReturn *ret,
                                                   Error **errp)
 {
     error_setg(errp, "character device backend type 'parallel' not supported");
@@ -4110,16 +4115,19 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
 #endif
 
 #ifdef HAVE_CHARDEV_PARPORT
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
+static CharDriverState *qmp_chardev_open_parallel(const char *id,
+                                                  ChardevBackend *backend,
+                                                  ChardevReturn *ret,
                                                   Error **errp)
 {
+    ChardevHostdev *parallel = backend->parallel;
     int fd;
 
     fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
     if (fd < 0) {
         return NULL;
     }
-    return qemu_chr_open_pp_fd(fd);
+    return qemu_chr_open_pp_fd(fd, errp);
 }
 #endif
 
@@ -4265,11 +4273,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         case CHARDEV_BACKEND_KIND_SERIAL:
             abort();
             break;
-#ifdef HAVE_CHARDEV_PARPORT
         case CHARDEV_BACKEND_KIND_PARALLEL:
-            chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
+            abort();
             break;
-#endif
         case CHARDEV_BACKEND_KIND_PIPE:
             chr = qemu_chr_open_pipe(backend->pipe);
             break;
@@ -4405,10 +4411,12 @@ static void register_types(void)
     register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
                          qemu_chr_parse_serial, qmp_chardev_open_serial);
 #endif
+#ifdef HAVE_CHARDEV_PARPORT
     register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
-                         qemu_chr_parse_parallel, NULL);
+                         qemu_chr_parse_parallel, qmp_chardev_open_parallel);
     register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
-                         qemu_chr_parse_parallel, NULL);
+                         qemu_chr_parse_parallel, qmp_chardev_open_parallel);
+#endif
     register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
                          NULL);
     register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (5 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:16   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 08/21] qemu-char: convert socket " Paolo Bonzini
                   ` (13 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 84cb8d0..3545cd8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1078,18 +1078,17 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
     return chr;
 }
 
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(const char *id,
+                                           ChardevBackend *backend,
+                                           ChardevReturn *ret,
+                                           Error **errp)
 {
+    ChardevHostdev *opts = backend->pipe;
     int fd_in, fd_out;
     char filename_in[CHR_MAX_FILENAME_SIZE];
     char filename_out[CHR_MAX_FILENAME_SIZE];
     const char *filename = opts->device;
 
-    if (filename == NULL) {
-        fprintf(stderr, "chardev: pipe: no filename given\n");
-        return NULL;
-    }
-
     snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
     snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename);
     TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
@@ -1101,6 +1100,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
 	    close(fd_out);
         TFR(fd_in = fd_out = qemu_open(filename, O_RDWR | O_BINARY));
         if (fd_in < 0) {
+            error_setg_file_open(errp, errno, filename);
             return NULL;
         }
     }
@@ -2084,7 +2084,8 @@ static int win_chr_pipe_poll(void *opaque)
     return 0;
 }
 
-static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
+static int win_chr_pipe_init(CharDriverState *chr, const char *filename,
+                             Error **errp)
 {
     WinCharState *s = chr->opaque;
     OVERLAPPED ov;
@@ -2096,12 +2097,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
 
     s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hsend) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        error_setg(errp, "Failed CreateEvent\n");
         goto fail;
     }
     s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hrecv) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        error_setg(errp, "Failed CreateEvent\n");
         goto fail;
     }
 
@@ -2111,7 +2112,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
                               PIPE_WAIT,
                               MAXCONNECT, NSENDBUF, NRECVBUF, NTIMEOUT, NULL);
     if (s->hcom == INVALID_HANDLE_VALUE) {
-        fprintf(stderr, "Failed CreateNamedPipe (%lu)\n", GetLastError());
+        error_setg(errp, "Failed CreateNamedPipe (%lu)\n", GetLastError());
         s->hcom = NULL;
         goto fail;
     }
@@ -2120,13 +2121,13 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
     ov.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
     ret = ConnectNamedPipe(s->hcom, &ov);
     if (ret) {
-        fprintf(stderr, "Failed ConnectNamedPipe\n");
+        error_setg(errp, "Failed ConnectNamedPipe\n");
         goto fail;
     }
 
     ret = GetOverlappedResult(s->hcom, &ov, &size, TRUE);
     if (!ret) {
-        fprintf(stderr, "Failed GetOverlappedResult\n");
+        error_setg(errp, "Failed GetOverlappedResult\n");
         if (ov.hEvent) {
             CloseHandle(ov.hEvent);
             ov.hEvent = NULL;
@@ -2147,8 +2148,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
 }
 
 
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(const char *id,
+                                           ChardevBackend *backend,
+                                           ChardevReturn *ret,
+                                           Error **errp)
 {
+    ChardevHostdev *opts = backend->pipe;
     const char *filename = opts->device;
     CharDriverState *chr;
     WinCharState *s;
@@ -2159,7 +2164,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
     chr->chr_write = win_chr_write;
     chr->chr_close = win_chr_close;
 
-    if (win_chr_pipe_init(chr, filename) < 0) {
+    if (win_chr_pipe_init(chr, filename, errp) < 0) {
         g_free(s);
         g_free(chr);
         return NULL;
@@ -4277,7 +4282,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_PIPE:
-            chr = qemu_chr_open_pipe(backend->pipe);
+            abort();
             break;
         case CHARDEV_BACKEND_KIND_SOCKET:
             chr = qmp_chardev_open_socket(backend->socket, &local_err);
@@ -4422,7 +4427,7 @@ static void register_types(void)
     register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
                          NULL);
     register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
-                         qemu_chr_parse_pipe, NULL);
+                         qemu_chr_parse_pipe, qemu_chr_open_pipe);
     register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
                          NULL);
     /* Bug-compatibility: */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 08/21] qemu-char: convert socket backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (6 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:20   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP " Paolo Bonzini
                   ` (12 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 3545cd8..459ed5c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4163,11 +4163,14 @@ static gboolean socket_reconnect_timeout(gpointer opaque)
     return false;
 }
 
-static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
+static CharDriverState *qmp_chardev_open_socket(const char *id,
+                                                ChardevBackend *backend,
+                                                ChardevReturn *ret,
                                                 Error **errp)
 {
     CharDriverState *chr;
     TCPCharDriver *s;
+    ChardevSocket *sock = backend->socket;
     SocketAddress *addr = sock->addr;
     bool do_nodelay     = sock->has_nodelay ? sock->nodelay : false;
     bool is_listen      = sock->has_server  ? sock->server  : true;
@@ -4285,7 +4288,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_SOCKET:
-            chr = qmp_chardev_open_socket(backend->socket, &local_err);
+            abort();
             break;
         case CHARDEV_BACKEND_KIND_UDP:
             chr = qmp_chardev_open_udp(backend->udp, &local_err);
@@ -4401,7 +4404,7 @@ static void register_types(void)
     register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL,
                          NULL);
     register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
-                         qemu_chr_parse_socket, NULL);
+                         qemu_chr_parse_socket, qmp_chardev_open_socket);
     register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
                          NULL);
     register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (7 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 08/21] qemu-char: convert socket " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:22   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 10/21] qemu-char: convert pty " Paolo Bonzini
                   ` (11 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 459ed5c..79c0c05 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4232,9 +4232,12 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
     return chr;
 }
 
-static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
+static CharDriverState *qmp_chardev_open_udp(const char *id,
+                                             ChardevBackend *backend,
+                                             ChardevReturn *ret,
                                              Error **errp)
 {
+    ChardevUdp *udp = backend->udp;
     int fd;
 
     fd = socket_dgram(udp->remote, udp->local, errp);
@@ -4291,7 +4294,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_UDP:
-            chr = qmp_chardev_open_udp(backend->udp, &local_err);
+            abort();
             break;
 #ifdef HAVE_CHARDEV_PTY
         case CHARDEV_BACKEND_KIND_PTY:
@@ -4406,7 +4409,7 @@ static void register_types(void)
     register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
                          qemu_chr_parse_socket, qmp_chardev_open_socket);
     register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
-                         NULL);
+                         qmp_chardev_open_udp);
     register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
                          qemu_chr_parse_ringbuf, NULL);
     register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 10/21] qemu-char: convert pty backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (8 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:22   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 11/21] qemu-char: convert null " Paolo Bonzini
                   ` (10 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 79c0c05..c36cbf0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1390,7 +1390,9 @@ static void pty_chr_close(struct CharDriverState *chr)
 }
 
 static CharDriverState *qemu_chr_open_pty(const char *id,
-                                          ChardevReturn *ret)
+                                          ChardevBackend *backend,
+                                          ChardevReturn *ret,
+                                          Error **errp)
 {
     CharDriverState *chr;
     PtyCharDriver *s;
@@ -1399,6 +1401,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
 
     master_fd = qemu_openpty_raw(&slave_fd, pty_name);
     if (master_fd < 0) {
+        error_setg_errno(errp, errno, "Failed to create PTY");
         return NULL;
     }
 
@@ -4296,11 +4299,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         case CHARDEV_BACKEND_KIND_UDP:
             abort();
             break;
-#ifdef HAVE_CHARDEV_PTY
         case CHARDEV_BACKEND_KIND_PTY:
-            chr = qemu_chr_open_pty(id, ret);
+            abort();
             break;
-#endif
         case CHARDEV_BACKEND_KIND_NULL:
             chr = qemu_chr_open_null();
             break;
@@ -4428,8 +4429,10 @@ static void register_types(void)
     register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
                          qemu_chr_parse_parallel, qmp_chardev_open_parallel);
 #endif
+#ifdef HAVE_CHARDEV_PTY
     register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
-                         NULL);
+                         qemu_chr_open_pty);
+#endif
     register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
                          NULL);
     register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 11/21] qemu-char: convert null backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (9 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 10/21] qemu-char: convert pty " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:23   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 12/21] qemu-char: convert mux " Paolo Bonzini
                   ` (9 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index c36cbf0..3e48a47 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -383,7 +383,10 @@ static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
     return len;
 }
 
-static CharDriverState *qemu_chr_open_null(void)
+static CharDriverState *qemu_chr_open_null(const char *id,
+                                           ChardevBackend *backend,
+                                           ChardevReturn *ret,
+                                           Error **errp)
 {
     CharDriverState *chr;
 
@@ -4303,7 +4306,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_NULL:
-            chr = qemu_chr_open_null();
+            abort();
             break;
         case CHARDEV_BACKEND_KIND_MUX:
             base = qemu_chr_find(backend->mux->chardev);
@@ -4406,7 +4409,7 @@ void qmp_chardev_remove(const char *id, Error **errp)
 static void register_types(void)
 {
     register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL,
-                         NULL);
+                         qemu_chr_open_null);
     register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
                          qemu_chr_parse_socket, qmp_chardev_open_socket);
     register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 12/21] qemu-char: convert mux backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (10 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 11/21] qemu-char: convert null " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:24   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse " Paolo Bonzini
                   ` (8 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 3e48a47..8e3a79a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -682,11 +682,20 @@ static GSource *mux_chr_add_watch(CharDriverState *s, GIOCondition cond)
     return d->drv->chr_add_watch(d->drv, cond);
 }
 
-static CharDriverState *qemu_chr_open_mux(CharDriverState *drv)
+static CharDriverState *qemu_chr_open_mux(const char *id,
+                                          ChardevBackend *backend,
+                                          ChardevReturn *ret, Error **errp)
 {
-    CharDriverState *chr;
+    ChardevMux *mux = backend->mux;
+    CharDriverState *chr, *drv;
     MuxDriver *d;
 
+    drv = qemu_chr_find(mux->chardev);
+    if (drv == NULL) {
+        error_setg(errp, "mux: base chardev %s not found", mux->chardev);
+        return NULL;
+    }
+
     chr = qemu_chr_alloc();
     d = g_new0(MuxDriver, 1);
 
@@ -4257,7 +4266,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
                                Error **errp)
 {
     ChardevReturn *ret = g_new0(ChardevReturn, 1);
-    CharDriverState *base, *chr = NULL;
+    CharDriverState *chr = NULL;
     Error *local_err = NULL;
     GSList *i;
     CharDriver *cd;
@@ -4309,13 +4318,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_MUX:
-            base = qemu_chr_find(backend->mux->chardev);
-            if (base == NULL) {
-                error_setg(&local_err, "mux: base chardev %s not found",
-                           backend->mux->chardev);
-                break;
-            }
-            chr = qemu_chr_open_mux(base);
+            abort();
             break;
         case CHARDEV_BACKEND_KIND_MSMOUSE:
             chr = qemu_chr_open_msmouse();
@@ -4441,7 +4444,7 @@ static void register_types(void)
     register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
                          qemu_chr_parse_pipe, qemu_chr_open_pipe);
     register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
-                         NULL);
+                         qemu_chr_open_mux);
     /* Bug-compatibility: */
     register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
                          qemu_chr_parse_ringbuf, NULL);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (11 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 12/21] qemu-char: convert mux " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:25   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 14/21] qemu-char: convert braille " Paolo Bonzini
                   ` (7 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/msmouse.c    | 7 +++++--
 include/sysemu/char.h | 3 ---
 qemu-char.c           | 2 +-
 stubs/Makefile.objs   | 1 -
 stubs/chr-msmouse.c   | 7 -------
 5 files changed, 6 insertions(+), 14 deletions(-)
 delete mode 100644 stubs/chr-msmouse.c

diff --git a/backends/msmouse.c b/backends/msmouse.c
index d50ed47..0126fa0 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -63,7 +63,10 @@ static void msmouse_chr_close (struct CharDriverState *chr)
     g_free (chr);
 }
 
-CharDriverState *qemu_chr_open_msmouse(void)
+static CharDriverState *qemu_chr_open_msmouse(const char *id,
+                                              ChardevBackend *backend,
+                                              ChardevReturn *ret,
+                                              Error **errp)
 {
     CharDriverState *chr;
 
@@ -80,7 +83,7 @@ CharDriverState *qemu_chr_open_msmouse(void)
 static void register_types(void)
 {
     register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL,
-                         NULL);
+                         qemu_chr_open_msmouse);
 }
 
 type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 4b01a8c..2fe8275 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -356,9 +356,6 @@ extern int term_escape_char;
 
 CharDriverState *qemu_char_get_next_serial(void);
 
-/* msmouse */
-CharDriverState *qemu_chr_open_msmouse(void);
-
 /* testdev.c */
 CharDriverState *chr_testdev_init(void);
 
diff --git a/qemu-char.c b/qemu-char.c
index 8e3a79a..8425891 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4321,7 +4321,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_MSMOUSE:
-            chr = qemu_chr_open_msmouse();
+            abort();
             break;
 #ifdef CONFIG_BRLAPI
         case CHARDEV_BACKEND_KIND_BRAILLE:
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 85e4e81..63988ca 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,7 +1,6 @@
 stub-obj-y += arch-query-cpu-def.o
 stub-obj-y += bdrv-commit-all.o
 stub-obj-y += chr-baum-init.o
-stub-obj-y += chr-msmouse.o
 stub-obj-y += chr-testdev.o
 stub-obj-y += clock-warp.o
 stub-obj-y += cpu-get-clock.o
diff --git a/stubs/chr-msmouse.c b/stubs/chr-msmouse.c
deleted file mode 100644
index 812f8b0..0000000
--- a/stubs/chr-msmouse.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *qemu_chr_open_msmouse(void)
-{
-    return 0;
-}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (12 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:30   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev " Paolo Bonzini
                   ` (6 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/baum.c       | 13 ++++++++-----
 include/sysemu/char.h |  3 ---
 qemu-char.c           |  4 +---
 stubs/Makefile.objs   |  1 -
 stubs/chr-baum-init.c |  7 -------
 5 files changed, 9 insertions(+), 19 deletions(-)
 delete mode 100644 stubs/chr-baum-init.c

diff --git a/backends/baum.c b/backends/baum.c
index e86a019..281be30 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -561,7 +561,10 @@ static void baum_close(struct CharDriverState *chr)
     g_free(baum);
 }
 
-CharDriverState *chr_baum_init(void)
+static CharDriverState *chr_baum_init(const char *id,
+                                      ChardevBackend *backend,
+                                      ChardevReturn *ret,
+                                      Error **errp)
 {
     BaumDriverState *baum;
     CharDriverState *chr;
@@ -586,14 +589,14 @@ CharDriverState *chr_baum_init(void)
 
     baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL);
     if (baum->brlapi_fd == -1) {
-        brlapi_perror("baum_init: brlapi_openConnection");
+        error_setg(errp, "baum_init: brlapi_openConnection");
         goto fail_handle;
     }
 
     baum->cellCount_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, baum_cellCount_timer_cb, baum);
 
     if (brlapi__getDisplaySize(handle, &baum->x, &baum->y) == -1) {
-        brlapi_perror("baum_init: brlapi_getDisplaySize");
+        error_setg(errp, "baum_init: brlapi_getDisplaySize");
         goto fail;
     }
 
@@ -609,7 +612,7 @@ CharDriverState *chr_baum_init(void)
         tty = BRLAPI_TTY_DEFAULT;
 
     if (brlapi__enterTtyMode(handle, tty, NULL) == -1) {
-        brlapi_perror("baum_init: brlapi_enterTtyMode");
+        error_setg(errp, "baum_init: brlapi_enterTtyMode");
         goto fail;
     }
 
@@ -630,7 +633,7 @@ fail_handle:
 static void register_types(void)
 {
     register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL,
-                         NULL);
+                         chr_baum_init);
 }
 
 type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 2fe8275..77415ec 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -359,9 +359,6 @@ CharDriverState *qemu_char_get_next_serial(void);
 /* testdev.c */
 CharDriverState *chr_testdev_init(void);
 
-/* baum.c */
-CharDriverState *chr_baum_init(void);
-
 /* console.c */
 typedef CharDriverState *(VcHandler)(ChardevVC *vc);
 
diff --git a/qemu-char.c b/qemu-char.c
index 8425891..96f40f3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4323,11 +4323,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         case CHARDEV_BACKEND_KIND_MSMOUSE:
             abort();
             break;
-#ifdef CONFIG_BRLAPI
         case CHARDEV_BACKEND_KIND_BRAILLE:
-            chr = chr_baum_init();
+            abort();
             break;
-#endif
         case CHARDEV_BACKEND_KIND_TESTDEV:
             chr = chr_testdev_init();
             break;
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 63988ca..8cfa5a2 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,6 +1,5 @@
 stub-obj-y += arch-query-cpu-def.o
 stub-obj-y += bdrv-commit-all.o
-stub-obj-y += chr-baum-init.o
 stub-obj-y += chr-testdev.o
 stub-obj-y += clock-warp.o
 stub-obj-y += cpu-get-clock.o
diff --git a/stubs/chr-baum-init.c b/stubs/chr-baum-init.c
deleted file mode 100644
index f5cc6ce..0000000
--- a/stubs/chr-baum-init.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *chr_baum_init(void)
-{
-    return NULL;
-}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (13 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 14/21] qemu-char: convert braille " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:49   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio " Paolo Bonzini
                   ` (5 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 backends/testdev.c    | 7 +++++--
 include/sysemu/char.h | 3 ---
 qemu-char.c           | 2 +-
 stubs/Makefile.objs   | 1 -
 stubs/chr-testdev.c   | 7 -------
 5 files changed, 6 insertions(+), 14 deletions(-)
 delete mode 100644 stubs/chr-testdev.c

diff --git a/backends/testdev.c b/backends/testdev.c
index 43787f6..26d5c73 100644
--- a/backends/testdev.c
+++ b/backends/testdev.c
@@ -108,7 +108,10 @@ static void testdev_close(struct CharDriverState *chr)
     g_free(testdev);
 }
 
-CharDriverState *chr_testdev_init(void)
+static CharDriverState *chr_testdev_init(const char *id,
+                                         ChardevBackend *backend,
+                                         ChardevReturn *ret,
+                                         Error **errp)
 {
     TestdevCharState *testdev;
     CharDriverState *chr;
@@ -126,7 +129,7 @@ CharDriverState *chr_testdev_init(void)
 static void register_types(void)
 {
     register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL,
-                         NULL);
+                         chr_testdev_init);
 }
 
 type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 77415ec..5c28c16 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -356,9 +356,6 @@ extern int term_escape_char;
 
 CharDriverState *qemu_char_get_next_serial(void);
 
-/* testdev.c */
-CharDriverState *chr_testdev_init(void);
-
 /* console.c */
 typedef CharDriverState *(VcHandler)(ChardevVC *vc);
 
diff --git a/qemu-char.c b/qemu-char.c
index 96f40f3..72658d2 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4327,7 +4327,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_TESTDEV:
-            chr = chr_testdev_init();
+            abort();
             break;
         case CHARDEV_BACKEND_KIND_STDIO:
             chr = qemu_chr_open_stdio(backend->stdio);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8cfa5a2..b5322a2 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,6 +1,5 @@
 stub-obj-y += arch-query-cpu-def.o
 stub-obj-y += bdrv-commit-all.o
-stub-obj-y += chr-testdev.o
 stub-obj-y += clock-warp.o
 stub-obj-y += cpu-get-clock.o
 stub-obj-y += cpu-get-icount.o
diff --git a/stubs/chr-testdev.c b/stubs/chr-testdev.c
deleted file mode 100644
index 23112a2..0000000
--- a/stubs/chr-testdev.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *chr_testdev_init(void)
-{
-    return 0;
-}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (14 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:50   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 17/21] qemu-char: convert console " Paolo Bonzini
                   ` (4 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

The backend now always returns errors via the Error* argument.
This avoids a double error message.  Before:

    qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
    qemu-system-x86_64: -chardev stdio,id=base: Failed to create chardev

After:

    qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 57 +++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 18 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 72658d2..74ca66d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1168,19 +1168,23 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr)
     fd_chr_close(chr);
 }
 
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(const char *id,
+                                            ChardevBackend *backend,
+                                            ChardevReturn *ret,
+                                            Error **errp)
 {
+    ChardevStdio *opts = backend->stdio;
     CharDriverState *chr;
     struct sigaction act;
 
     if (is_daemonized()) {
-        error_report("cannot use stdio with -daemonize");
+        error_setg(errp, "cannot use stdio with -daemonize");
         return NULL;
     }
 
     if (stdio_in_use) {
-        error_report("cannot use stdio by multiple character devices");
-        exit(1);
+        error_setg(errp, "cannot use stdio by multiple character devices");
+        return NULL;
     }
 
     stdio_in_use = true;
@@ -2341,7 +2345,10 @@ static void win_stdio_close(CharDriverState *chr)
     g_free(chr);
 }
 
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(const char *id,
+                                            ChardevBackend *backend,
+                                            ChardevReturn *ret,
+                                            Error **errp)
 {
     CharDriverState   *chr;
     WinStdioCharState *stdio;
@@ -2353,8 +2360,8 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
 
     stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE);
     if (stdio->hStdIn == INVALID_HANDLE_VALUE) {
-        fprintf(stderr, "cannot open stdio: invalid handle\n");
-        exit(1);
+        error_setg(errp, "cannot open stdio: invalid handle");
+        return NULL;
     }
 
     is_console = GetConsoleMode(stdio->hStdIn, &dwMode) != 0;
@@ -2366,25 +2373,30 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
     if (is_console) {
         if (qemu_add_wait_object(stdio->hStdIn,
                                  win_stdio_wait_func, chr)) {
-            fprintf(stderr, "qemu_add_wait_object: failed\n");
+            error_setg(errp, "qemu_add_wait_object: failed");
+            goto err1;
         }
     } else {
         DWORD   dwId;
             
         stdio->hInputReadyEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
         stdio->hInputDoneEvent  = CreateEvent(NULL, FALSE, FALSE, NULL);
-        stdio->hInputThread     = CreateThread(NULL, 0, win_stdio_thread,
-                                               chr, 0, &dwId);
-
-        if (stdio->hInputThread == INVALID_HANDLE_VALUE
-            || stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
+        if (stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
             || stdio->hInputDoneEvent == INVALID_HANDLE_VALUE) {
-            fprintf(stderr, "cannot create stdio thread or event\n");
-            exit(1);
+            error_setg(errp, "cannot create event");
+            goto err2;
         }
         if (qemu_add_wait_object(stdio->hInputReadyEvent,
                                  win_stdio_thread_wait_func, chr)) {
-            fprintf(stderr, "qemu_add_wait_object: failed\n");
+            error_setg(errp, "qemu_add_wait_object: failed");
+            goto err2;
+        }
+        stdio->hInputThread     = CreateThread(NULL, 0, win_stdio_thread,
+                                               chr, 0, &dwId);
+
+        if (stdio->hInputThread == INVALID_HANDLE_VALUE) {
+            error_setg(errp, "cannot create stdio thread");
+            goto err3;
         }
     }
 
@@ -2402,6 +2414,15 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
     qemu_chr_fe_set_echo(chr, false);
 
     return chr;
+
+err3:
+    qemu_del_wait_object(stdio->hInputReadyEvent, NULL, NULL);
+err2:
+    CloseHandle(stdio->hInputReadyEvent);
+    CloseHandle(stdio->hInputDoneEvent);
+err1:
+    qemu_del_wait_object(stdio->hStdIn, NULL, NULL);
+    return NULL;
 }
 #endif /* !_WIN32 */
 
@@ -4330,7 +4351,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_STDIO:
-            chr = qemu_chr_open_stdio(backend->stdio);
+            abort();
             break;
 #ifdef _WIN32
         case CHARDEV_BACKEND_KIND_CONSOLE:
@@ -4420,7 +4441,7 @@ static void register_types(void)
     register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
                          qemu_chr_parse_file_out, qmp_chardev_open_file);
     register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
-                         qemu_chr_parse_stdio, NULL);
+                         qemu_chr_parse_stdio, qemu_chr_open_stdio);
 #if defined HAVE_CHARDEV_SERIAL
     register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
                          qemu_chr_parse_serial, qmp_chardev_open_serial);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 17/21] qemu-char: convert console backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (15 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:50   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 18/21] qemu-char: convert spice " Paolo Bonzini
                   ` (3 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 74ca66d..535db73 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2204,7 +2204,10 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
     return chr;
 }
 
-static CharDriverState *qemu_chr_open_win_con(void)
+static CharDriverState *qemu_chr_open_win_con(const char *id,
+                                              ChardevBackend *backend,
+                                              ChardevReturn *ret,
+                                              Error **errp)
 {
     return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE));
 }
@@ -4353,11 +4356,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         case CHARDEV_BACKEND_KIND_STDIO:
             abort();
             break;
-#ifdef _WIN32
         case CHARDEV_BACKEND_KIND_CONSOLE:
-            chr = qemu_chr_open_win_con();
+            abort();
             break;
-#endif
 #ifdef CONFIG_SPICE
         case CHARDEV_BACKEND_KIND_SPICEVMC:
             chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
@@ -4458,8 +4459,10 @@ static void register_types(void)
     register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
                          qemu_chr_open_pty);
 #endif
+#ifdef _WIN32
     register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
-                         NULL);
+                         qemu_chr_open_win_con);
+#endif
     register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
                          qemu_chr_parse_pipe, qemu_chr_open_pipe);
     register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 18/21] qemu-char: convert spice backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (16 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 17/21] qemu-char: convert console " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:51   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 19/21] qemu-char: convert vc " Paolo Bonzini
                   ` (2 subsequent siblings)
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/ui/qemu-spice.h     |  2 --
 qemu-char.c                 |  6 ++----
 spice-qemu-char.c           | 21 ++++++++++++---------
 stubs/Makefile.objs         |  1 -
 stubs/qemu-chr-open-spice.c | 14 --------------
 5 files changed, 14 insertions(+), 30 deletions(-)
 delete mode 100644 stubs/qemu-chr-open-spice.c

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 0dff422..f9ce357 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -43,9 +43,7 @@ int qemu_spice_set_pw_expire(time_t expires);
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
                             const char *subject);
 
-CharDriverState *qemu_chr_open_spice_vmc(const char *type);
 #if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name);
 void qemu_spice_register_ports(void);
 #else
 static inline CharDriverState *qemu_chr_open_spice_port(const char *name)
diff --git a/qemu-char.c b/qemu-char.c
index 535db73..43205ae 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4359,14 +4359,12 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
         case CHARDEV_BACKEND_KIND_CONSOLE:
             abort();
             break;
-#ifdef CONFIG_SPICE
         case CHARDEV_BACKEND_KIND_SPICEVMC:
-            chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
+            abort();
             break;
         case CHARDEV_BACKEND_KIND_SPICEPORT:
-            chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
+            abort();
             break;
-#endif
         case CHARDEV_BACKEND_KIND_VC:
             chr = vc_init(backend->vc);
             break;
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index e4353ef..a20fb5c 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -296,15 +296,14 @@ static CharDriverState *chr_open(const char *subtype,
     return chr;
 }
 
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
+static CharDriverState *qemu_chr_open_spice_vmc(const char *id,
+                                                ChardevBackend *backend,
+                                                ChardevReturn *ret,
+                                                Error **errp)
 {
+    const char *type = backend->spicevmc->type;
     const char **psubtype = spice_server_char_device_recognized_subtypes();
 
-    if (type == NULL) {
-        fprintf(stderr, "spice-qemu-char: missing name parameter\n");
-        print_allowed_subtypes();
-        return NULL;
-    }
     for (; *psubtype != NULL; ++psubtype) {
         if (strcmp(type, *psubtype) == 0) {
             break;
@@ -320,8 +319,12 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
 }
 
 #if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name)
+static CharDriverState *qemu_chr_open_spice_port(const char *id,
+                                                 ChardevBackend *backend,
+                                                 ChardevReturn *ret,
+                                                 Error **errp)
 {
+    const char *name = backend->spiceport->fqdn;
     CharDriverState *chr;
     SpiceCharDriver *s;
 
@@ -379,9 +382,9 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend *backend,
 static void register_types(void)
 {
     register_char_driver("spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
-                         qemu_chr_parse_spice_vmc, NULL);
+                         qemu_chr_parse_spice_vmc, qemu_chr_open_spice_vmc);
     register_char_driver("spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
-                         qemu_chr_parse_spice_port, NULL);
+                         qemu_chr_parse_spice_port, qemu_chr_open_spice_port);
 }
 
 type_init(register_types);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index b5322a2..6d4363d 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -20,7 +20,6 @@ stub-obj-y += mon-is-qmp.o
 stub-obj-y += mon-printf.o
 stub-obj-y += monitor-init.o
 stub-obj-y += notify-event.o
-stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
 stub-obj-y += qtest.o
 stub-obj-y += reset.o
 stub-obj-y += runstate-check.o
diff --git a/stubs/qemu-chr-open-spice.c b/stubs/qemu-chr-open-spice.c
deleted file mode 100644
index f1c4849..0000000
--- a/stubs/qemu-chr-open-spice.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include "qemu-common.h"
-#include "ui/qemu-spice.h"
-
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
-{
-    return NULL;
-}
-
-#if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name)
-{
-    return NULL;
-}
-#endif
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 19/21] qemu-char: convert vc backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (17 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 18/21] qemu-char: convert spice " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:53   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf " Paolo Bonzini
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create Paolo Bonzini
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/sysemu/char.h |  5 ++---
 qemu-char.c           |  2 +-
 stubs/Makefile.objs   |  1 -
 stubs/vc-init.c       |  7 -------
 ui/console.c          | 10 ++++++----
 ui/gtk.c              |  2 +-
 6 files changed, 10 insertions(+), 17 deletions(-)
 delete mode 100644 stubs/vc-init.c

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5c28c16..edf7669 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -357,8 +357,7 @@ extern int term_escape_char;
 CharDriverState *qemu_char_get_next_serial(void);
 
 /* console.c */
-typedef CharDriverState *(VcHandler)(ChardevVC *vc);
-
+typedef CharDriverState *(VcHandler)(ChardevVC *vc, Error **errp);
 void register_vc_handler(VcHandler *handler);
-CharDriverState *vc_init(ChardevVC *vc);
+
 #endif
diff --git a/qemu-char.c b/qemu-char.c
index 43205ae..7ef1293 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4366,7 +4366,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             abort();
             break;
         case CHARDEV_BACKEND_KIND_VC:
-            chr = vc_init(backend->vc);
+            abort();
             break;
         case CHARDEV_BACKEND_KIND_RINGBUF:
         case CHARDEV_BACKEND_KIND_MEMORY:
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 6d4363d..1862f84 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -27,7 +27,6 @@ stub-obj-y += set-fd-handler.o
 stub-obj-y += slirp.o
 stub-obj-y += sysbus.o
 stub-obj-y += uuid.o
-stub-obj-y += vc-init.o
 stub-obj-y += vm-stop.o
 stub-obj-y += vmstate.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
diff --git a/stubs/vc-init.c b/stubs/vc-init.c
deleted file mode 100644
index 308dfa0..0000000
--- a/stubs/vc-init.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *vc_init(ChardevVC *vc)
-{
-    return 0;
-}
diff --git a/ui/console.c b/ui/console.c
index 746b23a..5e94b0f 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1899,7 +1899,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
         chr->init(chr);
 }
 
-static CharDriverState *text_console_init(ChardevVC *vc)
+static CharDriverState *text_console_init(ChardevVC *vc, Error **errp)
 {
     CharDriverState *chr;
     QemuConsole *s;
@@ -1930,6 +1930,7 @@ static CharDriverState *text_console_init(ChardevVC *vc)
 
     if (!s) {
         g_free(chr);
+        error_setg(errp, "cannot create text console");
         return NULL;
     }
 
@@ -1949,9 +1950,10 @@ static CharDriverState *text_console_init(ChardevVC *vc)
 
 static VcHandler *vc_handler = text_console_init;
 
-CharDriverState *vc_init(ChardevVC *vc)
+static CharDriverState *vc_init(const char *id, ChardevBackend *backend,
+                                ChardevReturn *ret, Error **errp)
 {
-    return vc_handler(vc);
+    return vc_handler(backend->vc, errp);
 }
 
 void register_vc_handler(VcHandler *handler)
@@ -2031,7 +2033,7 @@ static void register_types(void)
 {
     type_register_static(&qemu_console_info);
     register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc,
-                         NULL);
+                         vc_init);
 }
 
 type_init(register_types);
diff --git a/ui/gtk.c b/ui/gtk.c
index 3057cdc..9731761 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1522,7 +1522,7 @@ static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
 static int nb_vcs;
 static CharDriverState *vcs[MAX_VCS];
 
-static CharDriverState *gd_vc_handler(ChardevVC *unused)
+static CharDriverState *gd_vc_handler(ChardevVC *unused, Error **errp)
 {
     CharDriverState *chr;
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf backend to data-driven creation
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (18 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 19/21] qemu-char: convert vc " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:53   ` Eric Blake
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create Paolo Bonzini
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 7ef1293..e0faeb7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3222,9 +3222,12 @@ static void ringbuf_chr_close(struct CharDriverState *chr)
     chr->opaque = NULL;
 }
 
-static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts,
+static CharDriverState *qemu_chr_open_ringbuf(const char *id,
+                                              ChardevBackend *backend,
+                                              ChardevReturn *ret,
                                               Error **errp)
 {
+    ChardevRingbuf *opts = backend->ringbuf;
     CharDriverState *chr;
     RingBufCharDriver *d;
 
@@ -4370,7 +4373,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
             break;
         case CHARDEV_BACKEND_KIND_RINGBUF:
         case CHARDEV_BACKEND_KIND_MEMORY:
-            chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
+            abort();
             break;
         default:
             error_setg(errp, "unknown chardev backend (%d)", backend->kind);
@@ -4436,7 +4439,7 @@ static void register_types(void)
     register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
                          qmp_chardev_open_udp);
     register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
-                         qemu_chr_parse_ringbuf, NULL);
+                         qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf);
     register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
                          qemu_chr_parse_file_out, qmp_chardev_open_file);
     register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
@@ -4467,7 +4470,7 @@ static void register_types(void)
                          qemu_chr_open_mux);
     /* Bug-compatibility: */
     register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
-                         qemu_chr_parse_ringbuf, NULL);
+                         qemu_chr_parse_ringbuf, qemu_chr_open_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
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create
  2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
                   ` (19 preceding siblings ...)
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf " Paolo Bonzini
@ 2015-10-12  8:03 ` Paolo Bonzini
  2015-10-12 15:55   ` Eric Blake
  20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12  8:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: armbru

All backends now return errors through Error*, so the "Failed to
create chardev" placeholder error can only be reached if the backend
is not available (and only from the chardev-add QMP command; instead,
the -chardev command line option fails earlier).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-char.c | 80 ++++---------------------------------------------------------
 1 file changed, 4 insertions(+), 76 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index e0faeb7..8d48e35 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4308,7 +4308,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     for (i = backends; i; i = i->next) {
         cd = i->data;
 
-        if (cd->kind == backend->kind && cd->create) {
+        if (cd->kind == backend->kind) {
             chr = cd->create(id, backend, ret, &local_err);
             if (local_err) {
                 error_propagate(errp, local_err);
@@ -4319,81 +4319,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     }
 
     if (chr == NULL) {
-        switch (backend->kind) {
-        case CHARDEV_BACKEND_KIND_FILE:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_SERIAL:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_PARALLEL:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_PIPE:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_SOCKET:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_UDP:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_PTY:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_NULL:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_MUX:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_MSMOUSE:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_BRAILLE:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_TESTDEV:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_STDIO:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_CONSOLE:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_SPICEVMC:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_SPICEPORT:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_VC:
-            abort();
-            break;
-        case CHARDEV_BACKEND_KIND_RINGBUF:
-        case CHARDEV_BACKEND_KIND_MEMORY:
-            abort();
-            break;
-        default:
-            error_setg(errp, "unknown chardev backend (%d)", backend->kind);
-            goto out_error;
-        }
-
-        /*
-         * Character backend open hasn't been fully converted to the Error
-         * API.  Some opens fail without setting an error.  Set a generic
-         * error then.
-         * TODO full conversion to Error API
-         */
-        if (chr == NULL) {
-            if (local_err) {
-                error_propagate(errp, local_err);
-            } else {
-                error_setg(errp, "Failed to create chardev");
-            }
-            goto out_error;
-        }
+        assert(!i);
+        error_setg(errp, "chardev backend not available");
+        goto out_error;
     }
 
     chr->label = g_strdup(id);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
@ 2015-10-12 14:55   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 14:55 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Use the usual idioms for error propagation.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 56 +++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 31 insertions(+), 25 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 653ea10..f51c0aa 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4214,6 +4214,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>  {
>      ChardevReturn *ret = g_new0(ChardevReturn, 1);
>      CharDriverState *base, *chr = NULL;
> +    Error *local_err = NULL;

It's a toss-up on whether 'local_err' or 'err' is the preferred name for
the local variable. I'm fine with either.


> @@ -4303,25 +4304,30 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>       * error then.
>       * TODO full conversion to Error API
>       */
> -    if (chr == NULL && errp && !*errp) {
> -        error_setg(errp, "Failed to create chardev");
> -    }
> -    if (chr) {
> -        chr->label = g_strdup(id);
> -        chr->avail_connections =
> -            (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
> -        if (!chr->filename) {
> -            chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
> -        }
> -        if (!chr->explicit_be_open) {
> -            qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> +    if (chr == NULL) {

I might write this as if (!chr), but that's cosmetic.

Conversion looks sane.
Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_*
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_* Paolo Bonzini
@ 2015-10-12 14:57   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 14:57 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Move the #ifdef up into qmp_chardev_add, and avoid duplicating
> the code that reports unavailable backends.  Split HAVE_CHARDEV_TTY
> into HAVE_CHARDEV_SERIAL and HAVE_CHARDEV_PTY.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index f51c0aa..7219d56 100644
> --- a/qemu-char.c

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver Paolo Bonzini
@ 2015-10-12 15:04   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:04 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Having creation as a member of the CharDriver struct removes the need
> to export functions for qemu-char.c's usage.  After the conversion,
> chardev backends implemented outside qemu-char.c will not need a stub
> creation function anymore.
> 
> Ultimately all drivers will be converted.  For now, support the case
> where cd->create == NULL.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  backends/baum.c       |   3 +-
>  backends/msmouse.c    |   3 +-
>  backends/testdev.c    |   3 +-
>  include/sysemu/char.h |   4 +-
>  qemu-char.c           | 213 ++++++++++++++++++++++++++++----------------------
>  spice-qemu-char.c     |   4 +-
>  ui/console.c          |   3 +-
>  7 files changed, 133 insertions(+), 100 deletions(-)

The diff is a bit ugly because of reindentation, but seems sane.


>  
>  void register_char_driver(const char *name, ChardevBackendKind kind,
> -        void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp))
> +        void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp),
> +        CharDriverState *(*create)(const char *id, ChardevBackend *backend,
> +                                   ChardevReturn *ret, Error **errp))

A typedef (or two) for the function pointer(s) would make this a bit
easier to write.

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation Paolo Bonzini
@ 2015-10-12 15:06   ` Eric Blake
  2015-10-12 15:17     ` Paolo Bonzini
  0 siblings, 1 reply; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:06 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 

> @@ -4242,7 +4250,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>      if (chr == NULL) {
>          switch (backend->kind) {
>          case CHARDEV_BACKEND_KIND_FILE:
> -            chr = qmp_chardev_open_file(backend->file, &local_err);
> +            abort();
>              break;

The break is now dead code.

>  #ifdef HAVE_CHARDEV_SERIAL
>          case CHARDEV_BACKEND_KIND_SERIAL:
> @@ -4380,7 +4388,7 @@ static void register_types(void)
>      register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
>                           qemu_chr_parse_ringbuf, NULL);
>      register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
> -                         qemu_chr_parse_file_out, NULL);
> +                         qemu_chr_parse_file_out, qmp_chardev_open_file);

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 05/21] qemu-char: convert serial backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 05/21] qemu-char: convert serial " Paolo Bonzini
@ 2015-10-12 15:09   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:09 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 48 +++++++++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 19 deletions(-)
> 

> @@ -4252,11 +4262,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>          case CHARDEV_BACKEND_KIND_FILE:
>              abort();
>              break;
> -#ifdef HAVE_CHARDEV_SERIAL
>          case CHARDEV_BACKEND_KIND_SERIAL:
> -            chr = qmp_chardev_open_serial(backend->serial, &local_err);
> +            abort();
>              break;
> -#endif

Another dead break after abort().

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel " Paolo Bonzini
@ 2015-10-12 15:13   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:13 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Conversion to Error * brings better error messages; before:
> 
>     qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: Failed to create chardev
> 
> After:
> 
>     qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: not a parallel port: Inappropriate ioctl for device
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 

> +static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
>  {
>      CharDriverState *chr;
>      ParallelCharDriver *drv;
>  
>      if (ioctl(fd, PPCLAIM) < 0) {
>          close(fd);
> +        error_setg_errno(errp, errno, "not a parallel port");

Swap this line with the close(), since close() can clobber errno.


> @@ -4265,11 +4273,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>          case CHARDEV_BACKEND_KIND_SERIAL:
>              abort();
>              break;
> -#ifdef HAVE_CHARDEV_PARPORT
>          case CHARDEV_BACKEND_KIND_PARALLEL:
> -            chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
> +            abort();
>              break;

Another dead break.

With the error reporting fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe " Paolo Bonzini
@ 2015-10-12 15:16   ` Eric Blake
  2015-10-12 15:18     ` Paolo Bonzini
  0 siblings, 1 reply; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:16 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 37 +++++++++++++++++++++----------------
>  1 file changed, 21 insertions(+), 16 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 84cb8d0..3545cd8 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1078,18 +1078,17 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
>      return chr;
>  }
>  
> -static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
> +static CharDriverState *qemu_chr_open_pipe(const char *id,
> +                                           ChardevBackend *backend,
> +                                           ChardevReturn *ret,
> +                                           Error **errp)
>  {
> +    ChardevHostdev *opts = backend->pipe;
>      int fd_in, fd_out;
>      char filename_in[CHR_MAX_FILENAME_SIZE];
>      char filename_out[CHR_MAX_FILENAME_SIZE];
>      const char *filename = opts->device;
>  
> -    if (filename == NULL) {
> -        fprintf(stderr, "chardev: pipe: no filename given\n");
> -        return NULL;
> -    }
> -
>      snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);

Do we need assert(filename) here?


> @@ -2096,12 +2097,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
>  
>      s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
>      if (!s->hsend) {
> -        fprintf(stderr, "Failed CreateEvent\n");
> +        error_setg(errp, "Failed CreateEvent\n");
>          goto fail;
>      }
>      s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
>      if (!s->hrecv) {
> -        fprintf(stderr, "Failed CreateEvent\n");
> +        error_setg(errp, "Failed CreateEvent\n");
>          goto fail;

Drop the trailing \n, here and throughout this file

> @@ -4277,7 +4282,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>              abort();
>              break;
>          case CHARDEV_BACKEND_KIND_PIPE:
> -            chr = qemu_chr_open_pipe(backend->pipe);
> +            abort();
>              break;

another dead break


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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation
  2015-10-12 15:06   ` Eric Blake
@ 2015-10-12 15:17     ` Paolo Bonzini
  0 siblings, 0 replies; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 15:17 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: armbru



On 12/10/2015 17:06, Eric Blake wrote:
>> >          case CHARDEV_BACKEND_KIND_FILE:
>> > -            chr = qmp_chardev_open_file(backend->file, &local_err);
>> > +            abort();
>> >              break;
> The break is now dead code.
> 

They all go away in patch 21. :)

Paolo

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
  2015-10-12 15:16   ` Eric Blake
@ 2015-10-12 15:18     ` Paolo Bonzini
  2015-10-12 15:21       ` Eric Blake
  0 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 15:18 UTC (permalink / raw)
  To: Eric Blake, qemu-devel; +Cc: armbru



On 12/10/2015 17:16, Eric Blake wrote:
>> rp)
>> >  {
>> > +    ChardevHostdev *opts = backend->pipe;
>> >      int fd_in, fd_out;
>> >      char filename_in[CHR_MAX_FILENAME_SIZE];
>> >      char filename_out[CHR_MAX_FILENAME_SIZE];
>> >      const char *filename = opts->device;
>> >  
>> > -    if (filename == NULL) {
>> > -        fprintf(stderr, "chardev: pipe: no filename given\n");
>> > -        return NULL;
>> > -    }
>> > -
>> >      snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
> Do we need assert(filename) here?
> 
> 

No, "device" is not optional in the definition of ChardevHostdev.

Paolo

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 08/21] qemu-char: convert socket backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 08/21] qemu-char: convert socket " Paolo Bonzini
@ 2015-10-12 15:20   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:20 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
  2015-10-12 15:18     ` Paolo Bonzini
@ 2015-10-12 15:21       ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:21 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 09:18 AM, Paolo Bonzini wrote:
> 
> 
> On 12/10/2015 17:16, Eric Blake wrote:
>>> rp)
>>>>  {
>>>> +    ChardevHostdev *opts = backend->pipe;
>>>>      int fd_in, fd_out;
>>>>      char filename_in[CHR_MAX_FILENAME_SIZE];
>>>>      char filename_out[CHR_MAX_FILENAME_SIZE];
>>>>      const char *filename = opts->device;
>>>>  
>>>> -    if (filename == NULL) {
>>>> -        fprintf(stderr, "chardev: pipe: no filename given\n");
>>>> -        return NULL;
>>>> -    }
>>>> -
>>>>      snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
>> Do we need assert(filename) here?
>>
>>
> 
> No, "device" is not optional in the definition of ChardevHostdev.

Okay, then with the \n gone,
Reviewed-by: Eric Blake <eblake@redhat.com>

(and I'll quit complaining about dead break)

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP " Paolo Bonzini
@ 2015-10-12 15:22   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:22 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 10/21] qemu-char: convert pty backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 10/21] qemu-char: convert pty " Paolo Bonzini
@ 2015-10-12 15:22   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:22 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 11/21] qemu-char: convert null backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 11/21] qemu-char: convert null " Paolo Bonzini
@ 2015-10-12 15:23   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:23 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 12/21] qemu-char: convert mux backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 12/21] qemu-char: convert mux " Paolo Bonzini
@ 2015-10-12 15:24   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:24 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse " Paolo Bonzini
@ 2015-10-12 15:25   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:25 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  backends/msmouse.c    | 7 +++++--
>  include/sysemu/char.h | 3 ---
>  qemu-char.c           | 2 +-
>  stubs/Makefile.objs   | 1 -
>  stubs/chr-msmouse.c   | 7 -------
>  5 files changed, 6 insertions(+), 14 deletions(-)
>  delete mode 100644 stubs/chr-msmouse.c
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 14/21] qemu-char: convert braille " Paolo Bonzini
@ 2015-10-12 15:30   ` Eric Blake
  2015-10-12 15:41     ` Samuel Thibault
  0 siblings, 1 reply; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:30 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  backends/baum.c       | 13 ++++++++-----
>  include/sysemu/char.h |  3 ---
>  qemu-char.c           |  4 +---
>  stubs/Makefile.objs   |  1 -
>  stubs/chr-baum-init.c |  7 -------
>  5 files changed, 9 insertions(+), 19 deletions(-)
>  delete mode 100644 stubs/chr-baum-init.c
> 

> @@ -586,14 +589,14 @@ CharDriverState *chr_baum_init(void)
>  
>      baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL);
>      if (baum->brlapi_fd == -1) {
> -        brlapi_perror("baum_init: brlapi_openConnection");
> +        error_setg(errp, "baum_init: brlapi_openConnection");

error_setg() already tracks function name, so you could drop 'baum_init:
'.  Also, I assume that brlapi_perror() adds additional information to
the error message it prints, such as conversion of a brlapi-specific
error message in the same manner in which perror() converts errno and in
which error_setg_errno() would be used.  So I don't know if this
conversion is the best.  But I'm unfamiliar with brlapi_* in general, to
know if there is anything better to use, so I'd rather get a second
opinion on this.
'
Other than the possibility of the incorrect conversion of (several)
error messages, the rest of the patch looks fine to me.

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
  2015-10-12 15:30   ` Eric Blake
@ 2015-10-12 15:41     ` Samuel Thibault
  2015-10-14 15:51       ` Paolo Bonzini
  0 siblings, 1 reply; 51+ messages in thread
From: Samuel Thibault @ 2015-10-12 15:41 UTC (permalink / raw)
  To: Eric Blake; +Cc: Paolo Bonzini, qemu-devel, armbru

Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
>  Also, I assume that brlapi_perror() adds additional information to
> the error message it prints, such as conversion of a brlapi-specific
> error message in the same manner in which perror() converts errno and in
> which error_setg_errno() would be used.

Yes. Such additional information is really useful to debug brlapi
issues.

> So I don't know if this
> conversion is the best.  But I'm unfamiliar with brlapi_* in general, to
> know if there is anything better to use,

brlapi_error_t * brlapi_error_location(void);
const char * brlapi_strerror(const brlapi_error_t *error);

So brlapi_strerror(brlapi_error_location()) will return what you want,
i.e. the string that brlapi_error() would have printed.

> so I'd rather get a second opinion on this.

Cc-ing me would have been useful, "braille" just happened to catch my
eye by luck while looking over qemu mails :)

Samuel

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev " Paolo Bonzini
@ 2015-10-12 15:49   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:49 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  backends/testdev.c    | 7 +++++--
>  include/sysemu/char.h | 3 ---
>  qemu-char.c           | 2 +-
>  stubs/Makefile.objs   | 1 -
>  stubs/chr-testdev.c   | 7 -------
>  5 files changed, 6 insertions(+), 14 deletions(-)
>  delete mode 100644 stubs/chr-testdev.c
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio " Paolo Bonzini
@ 2015-10-12 15:50   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:50 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> The backend now always returns errors via the Error* argument.
> This avoids a double error message.  Before:
> 
>     qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
>     qemu-system-x86_64: -chardev stdio,id=base: Failed to create chardev
> 
> After:
> 
>     qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 57 +++++++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 18 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 17/21] qemu-char: convert console backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 17/21] qemu-char: convert console " Paolo Bonzini
@ 2015-10-12 15:50   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:50 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 18/21] qemu-char: convert spice backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 18/21] qemu-char: convert spice " Paolo Bonzini
@ 2015-10-12 15:51   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:51 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/ui/qemu-spice.h     |  2 --
>  qemu-char.c                 |  6 ++----
>  spice-qemu-char.c           | 21 ++++++++++++---------
>  stubs/Makefile.objs         |  1 -
>  stubs/qemu-chr-open-spice.c | 14 --------------
>  5 files changed, 14 insertions(+), 30 deletions(-)
>  delete mode 100644 stubs/qemu-chr-open-spice.c
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 19/21] qemu-char: convert vc backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 19/21] qemu-char: convert vc " Paolo Bonzini
@ 2015-10-12 15:53   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:53 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/sysemu/char.h |  5 ++---
>  qemu-char.c           |  2 +-
>  stubs/Makefile.objs   |  1 -
>  stubs/vc-init.c       |  7 -------
>  ui/console.c          | 10 ++++++----
>  ui/gtk.c              |  2 +-
>  6 files changed, 10 insertions(+), 17 deletions(-)
>  delete mode 100644 stubs/vc-init.c
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf backend to data-driven creation
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf " Paolo Bonzini
@ 2015-10-12 15:53   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:53 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create
  2015-10-12  8:03 ` [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create Paolo Bonzini
@ 2015-10-12 15:55   ` Eric Blake
  0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: armbru

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

On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> All backends now return errors through Error*, so the "Failed to
> create chardev" placeholder error can only be reached if the backend
> is not available (and only from the chardev-add QMP command; instead,
> the -chardev command line option fails earlier).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qemu-char.c | 80 ++++---------------------------------------------------------
>  1 file changed, 4 insertions(+), 76 deletions(-)
> 

Nice diffstat ratio!
Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
  2015-10-12 15:41     ` Samuel Thibault
@ 2015-10-14 15:51       ` Paolo Bonzini
  2015-10-14 15:55         ` Eric Blake
  2015-10-14 16:34         ` Samuel Thibault
  0 siblings, 2 replies; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-14 15:51 UTC (permalink / raw)
  To: Samuel Thibault, Eric Blake; +Cc: qemu-devel, armbru



On 12/10/2015 17:41, Samuel Thibault wrote:
> Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
>>  Also, I assume that brlapi_perror() adds additional information to
>> the error message it prints, such as conversion of a brlapi-specific
>> error message in the same manner in which perror() converts errno and in
>> which error_setg_errno() would be used.
> 
> Yes. Such additional information is really useful to debug brlapi
> issues.
> 
>> So I don't know if this
>> conversion is the best.  But I'm unfamiliar with brlapi_* in general, to
>> know if there is anything better to use,
> 
> brlapi_error_t * brlapi_error_location(void);
> const char * brlapi_strerror(const brlapi_error_t *error);
> 
> So brlapi_strerror(brlapi_error_location()) will return what you want,
> i.e. the string that brlapi_error() would have printed.

Is it okay to squash this?

diff --git a/backends/baum.c b/backends/baum.c
index 281be30..723c658 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -589,14 +589,16 @@ static CharDriverState *chr_baum_init(const char *id,
 
     baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL);
     if (baum->brlapi_fd == -1) {
-        error_setg(errp, "baum_init: brlapi_openConnection");
+        error_setg(errp, "brlapi__openConnection: %s",
+                   brlapi_strerror(brlapi_error_location()));
         goto fail_handle;
     }
 
     baum->cellCount_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, baum_cellCount_timer_cb, baum);
 
     if (brlapi__getDisplaySize(handle, &baum->x, &baum->y) == -1) {
-        error_setg(errp, "baum_init: brlapi_getDisplaySize");
+        error_setg(errp, "brlapi__getDisplaySize: %s",
+                   brlapi_strerror(brlapi_error_location()));
         goto fail;
     }
 
@@ -612,7 +614,8 @@ static CharDriverState *chr_baum_init(const char *id,
         tty = BRLAPI_TTY_DEFAULT;
 
     if (brlapi__enterTtyMode(handle, tty, NULL) == -1) {
-        error_setg(errp, "baum_init: brlapi_enterTtyMode");
+        error_setg(errp, "brlapi__enterTtyMode: %s",
+                   brlapi_strerror(brlapi_error_location()));
         goto fail;
     }
 

Thanks for the help,

Paolo

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
  2015-10-14 15:51       ` Paolo Bonzini
@ 2015-10-14 15:55         ` Eric Blake
  2015-10-14 16:34         ` Samuel Thibault
  1 sibling, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-14 15:55 UTC (permalink / raw)
  To: Paolo Bonzini, Samuel Thibault; +Cc: qemu-devel, armbru

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

On 10/14/2015 09:51 AM, Paolo Bonzini wrote:
> 
> 
> On 12/10/2015 17:41, Samuel Thibault wrote:
>> Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
>>>  Also, I assume that brlapi_perror() adds additional information to
>>> the error message it prints, such as conversion of a brlapi-specific
>>> error message in the same manner in which perror() converts errno and in
>>> which error_setg_errno() would be used.
>>
>> Yes. Such additional information is really useful to debug brlapi
>> issues.
>>
>>> So I don't know if this
>>> conversion is the best.  But I'm unfamiliar with brlapi_* in general, to
>>> know if there is anything better to use,
>>
>> brlapi_error_t * brlapi_error_location(void);
>> const char * brlapi_strerror(const brlapi_error_t *error);
>>
>> So brlapi_strerror(brlapi_error_location()) will return what you want,
>> i.e. the string that brlapi_error() would have printed.
> 
> Is it okay to squash this?

I'd still like Samuel's confirmation, but from my point of view, that
looks like enough to add:
Reviewed-by: Eric Blake <eblake@redhat.com>

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

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
  2015-10-14 15:51       ` Paolo Bonzini
  2015-10-14 15:55         ` Eric Blake
@ 2015-10-14 16:34         ` Samuel Thibault
  2015-10-14 16:51           ` Paolo Bonzini
  1 sibling, 1 reply; 51+ messages in thread
From: Samuel Thibault @ 2015-10-14 16:34 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, armbru

Paolo Bonzini, le Wed 14 Oct 2015 17:51:51 +0200, a écrit :
> 
> 
> On 12/10/2015 17:41, Samuel Thibault wrote:
> > Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
> >>  Also, I assume that brlapi_perror() adds additional information to
> >> the error message it prints, such as conversion of a brlapi-specific
> >> error message in the same manner in which perror() converts errno and in
> >> which error_setg_errno() would be used.
> > 
> > Yes. Such additional information is really useful to debug brlapi
> > issues.
> > 
> >> So I don't know if this
> >> conversion is the best.  But I'm unfamiliar with brlapi_* in general, to
> >> know if there is anything better to use,
> > 
> > brlapi_error_t * brlapi_error_location(void);
> > const char * brlapi_strerror(const brlapi_error_t *error);
> > 
> > So brlapi_strerror(brlapi_error_location()) will return what you want,
> > i.e. the string that brlapi_error() would have printed.
> 
> Is it okay to squash this?

Yes, this looks right. Building with brlapi-dev installed would confirm
for sure that it builds, of course :)

Samuel

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
  2015-10-14 16:34         ` Samuel Thibault
@ 2015-10-14 16:51           ` Paolo Bonzini
  0 siblings, 0 replies; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-14 16:51 UTC (permalink / raw)
  To: Samuel Thibault; +Cc: qemu-devel, armbru



On 14/10/2015 18:34, Samuel Thibault wrote:
> Paolo Bonzini, le Wed 14 Oct 2015 17:51:51 +0200, a écrit :
>>
>>
>> On 12/10/2015 17:41, Samuel Thibault wrote:
>>> Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
>>>>  Also, I assume that brlapi_perror() adds additional information to
>>>> the error message it prints, such as conversion of a brlapi-specific
>>>> error message in the same manner in which perror() converts errno and in
>>>> which error_setg_errno() would be used.
>>>
>>> Yes. Such additional information is really useful to debug brlapi
>>> issues.
>>>
>>>> So I don't know if this
>>>> conversion is the best.  But I'm unfamiliar with brlapi_* in general, to
>>>> know if there is anything better to use,
>>>
>>> brlapi_error_t * brlapi_error_location(void);
>>> const char * brlapi_strerror(const brlapi_error_t *error);
>>>
>>> So brlapi_strerror(brlapi_error_location()) will return what you want,
>>> i.e. the string that brlapi_error() would have printed.
>>
>> Is it okay to squash this?
> 
> Yes, this looks right. Building with brlapi-dev installed would confirm
> for sure that it builds, of course :)

That I checked. :)

Paolo

^ permalink raw reply	[flat|nested] 51+ messages in thread

end of thread, other threads:[~2015-10-14 16:51 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-12  8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
2015-10-12  8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
2015-10-12 14:55   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_* Paolo Bonzini
2015-10-12 14:57   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver Paolo Bonzini
2015-10-12 15:04   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation Paolo Bonzini
2015-10-12 15:06   ` Eric Blake
2015-10-12 15:17     ` Paolo Bonzini
2015-10-12  8:03 ` [Qemu-devel] [PATCH 05/21] qemu-char: convert serial " Paolo Bonzini
2015-10-12 15:09   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel " Paolo Bonzini
2015-10-12 15:13   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe " Paolo Bonzini
2015-10-12 15:16   ` Eric Blake
2015-10-12 15:18     ` Paolo Bonzini
2015-10-12 15:21       ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 08/21] qemu-char: convert socket " Paolo Bonzini
2015-10-12 15:20   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP " Paolo Bonzini
2015-10-12 15:22   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 10/21] qemu-char: convert pty " Paolo Bonzini
2015-10-12 15:22   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 11/21] qemu-char: convert null " Paolo Bonzini
2015-10-12 15:23   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 12/21] qemu-char: convert mux " Paolo Bonzini
2015-10-12 15:24   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse " Paolo Bonzini
2015-10-12 15:25   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 14/21] qemu-char: convert braille " Paolo Bonzini
2015-10-12 15:30   ` Eric Blake
2015-10-12 15:41     ` Samuel Thibault
2015-10-14 15:51       ` Paolo Bonzini
2015-10-14 15:55         ` Eric Blake
2015-10-14 16:34         ` Samuel Thibault
2015-10-14 16:51           ` Paolo Bonzini
2015-10-12  8:03 ` [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev " Paolo Bonzini
2015-10-12 15:49   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio " Paolo Bonzini
2015-10-12 15:50   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 17/21] qemu-char: convert console " Paolo Bonzini
2015-10-12 15:50   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 18/21] qemu-char: convert spice " Paolo Bonzini
2015-10-12 15:51   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 19/21] qemu-char: convert vc " Paolo Bonzini
2015-10-12 15:53   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf " Paolo Bonzini
2015-10-12 15:53   ` Eric Blake
2015-10-12  8:03 ` [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create Paolo Bonzini
2015-10-12 15:55   ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.