[Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Gerd Hoffmann]

  Hi,

Chardev hotplug patch series reloaded.  Not finished yet, commit
messages not finalized yet, totally untested other than building on
linux+windows.

I doubt I manage to finish (and test!) it before xmas.  Nevertheless I
wanna get the bits out of the door for you to see what is coming and for
early reviews.  It goes largely the route suggested by Paolo.

cheers,
  Gerd

Gerd Hoffmann (9):
  chardev: add error reporting for qemu_chr_new_from_opts
  chardev: fix QemuOpts lifecycle
  chardev: reduce chardev ifdef mess a bit
  chardev: hotplug, qmp, null
  chardev: hotplug, hmp
  chardev: hotplug, qmp, file
  chardev: hotplug, qmp, tty
  chardev: hotplug, qmp, serial
  chardev: hotplug, qmp, parallel

 hmp-commands.hx  |   32 ++++++
 hmp.c            |   23 +++++
 hmp.h            |    2 +
 qapi-schema.json |   47 +++++++++
 qemu-char.c      |  280 ++++++++++++++++++++++++++++++++++++++++++++---------
 qemu-char.h      |    4 +-
 qmp-commands.hx  |   50 ++++++++++
 vl.c             |    9 +-
 8 files changed, 395 insertions(+), 52 deletions(-)

[Qemu-devel] [PATCH 1/9] chardev: add error reporting for qemu_chr_new_from_opts

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |   24 +++++++++++++++---------
 qemu-char.h |    3 ++-
 vl.c        |    9 ++++++---
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 9940701..713eae9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2769,19 +2769,20 @@ static const struct {
 };
 
 CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-                                    void (*init)(struct CharDriverState *s))
+                                    void (*init)(struct CharDriverState *s),
+                                    Error **errp)
 {
     CharDriverState *chr;
     int i;
 
     if (qemu_opts_id(opts) == NULL) {
-        fprintf(stderr, "chardev: no id specified\n");
+        error_setg(errp, "chardev: no id specified\n");
         return NULL;
     }
 
     if (qemu_opt_get(opts, "backend") == NULL) {
-        fprintf(stderr, "chardev: \"%s\" missing backend\n",
-                qemu_opts_id(opts));
+        error_setg(errp, "chardev: \"%s\" missing backend\n",
+                   qemu_opts_id(opts));
         return NULL;
     }
     for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
@@ -2789,15 +2790,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
             break;
     }
     if (i == ARRAY_SIZE(backend_table)) {
-        fprintf(stderr, "chardev: backend \"%s\" not found\n",
-                qemu_opt_get(opts, "backend"));
+        error_setg(errp, "chardev: backend \"%s\" not found\n",
+                   qemu_opt_get(opts, "backend"));
         return NULL;
     }
 
     chr = backend_table[i].open(opts);
     if (!chr) {
-        fprintf(stderr, "chardev: opening backend \"%s\" failed\n",
-                qemu_opt_get(opts, "backend"));
+        error_setg(errp, "chardev: opening backend \"%s\" failed\n",
+                   qemu_opt_get(opts, "backend"));
         return NULL;
     }
 
@@ -2827,6 +2828,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     const char *p;
     CharDriverState *chr;
     QemuOpts *opts;
+    Error *err = NULL;
 
     if (strstart(filename, "chardev:", &p)) {
         return qemu_chr_find(p);
@@ -2836,7 +2838,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     if (!opts)
         return NULL;
 
-    chr = qemu_chr_new_from_opts(opts, init);
+    chr = qemu_chr_new_from_opts(opts, init, &err);
+    if (error_is_set(&err)) {
+        fprintf(stderr, "%s\n", error_get_pretty(err));
+        error_free(err);
+    }
     if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
         monitor_init(chr, MONITOR_USE_READLINE);
     }
diff --git a/qemu-char.h b/qemu-char.h
index a121e04..d7eed34 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -89,7 +89,8 @@ struct CharDriverState {
  * Returns: a new character backend
  */
 CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-                                    void (*init)(struct CharDriverState *s));
+                                    void (*init)(struct CharDriverState *s),
+                                    Error **errp);
 
 /**
  * @qemu_chr_new:
diff --git a/vl.c b/vl.c
index 3ebf01f..e96dade 100644
--- a/vl.c
+++ b/vl.c
@@ -2052,11 +2052,14 @@ static int device_init_func(QemuOpts *opts, void *opaque)
 
 static int chardev_init_func(QemuOpts *opts, void *opaque)
 {
-    CharDriverState *chr;
+    Error *local_err = NULL;
 
-    chr = qemu_chr_new_from_opts(opts, NULL);
-    if (!chr)
+    qemu_chr_new_from_opts(opts, NULL, &local_err);
+    if (error_is_set(&local_err)) {
+        fprintf(stderr, "%s\n", error_get_pretty(local_err));
+        error_free(local_err);
         return -1;
+    }
     return 0;
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 2/9] chardev: fix QemuOpts lifecycle

[Gerd Hoffmann]

qemu_chr_new_from_opts handles QemuOpts release now, so callers don't
have to worry.  It will either be saved in CharDriverState, then
released in qemu_chr_delete, or in the error case released instantly.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |   15 ++++++++++-----
 qemu-char.h |    1 +
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 713eae9..9bb3a6b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2777,13 +2777,13 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
 
     if (qemu_opts_id(opts) == NULL) {
         error_setg(errp, "chardev: no id specified\n");
-        return NULL;
+        goto err;
     }
 
     if (qemu_opt_get(opts, "backend") == NULL) {
         error_setg(errp, "chardev: \"%s\" missing backend\n",
                    qemu_opts_id(opts));
-        return NULL;
+        goto err;
     }
     for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
         if (strcmp(backend_table[i].name, qemu_opt_get(opts, "backend")) == 0)
@@ -2792,14 +2792,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
     if (i == ARRAY_SIZE(backend_table)) {
         error_setg(errp, "chardev: backend \"%s\" not found\n",
                    qemu_opt_get(opts, "backend"));
-        return NULL;
+        goto err;
     }
 
     chr = backend_table[i].open(opts);
     if (!chr) {
         error_setg(errp, "chardev: opening backend \"%s\" failed\n",
                    qemu_opt_get(opts, "backend"));
-        return NULL;
+        goto err;
     }
 
     if (!chr->filename)
@@ -2820,7 +2820,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
         chr->avail_connections = 1;
     }
     chr->label = g_strdup(qemu_opts_id(opts));
+    chr->opts = opts;
     return chr;
+
+err:
+    qemu_opts_del(opts);
+    return NULL;
 }
 
 CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s))
@@ -2846,7 +2851,6 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in
     if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
         monitor_init(chr, MONITOR_USE_READLINE);
     }
-    qemu_opts_del(opts);
     return chr;
 }
 
@@ -2878,6 +2882,7 @@ void qemu_chr_delete(CharDriverState *chr)
         chr->chr_close(chr);
     g_free(chr->filename);
     g_free(chr->label);
+    qemu_opts_del(chr->opts);
     g_free(chr);
 }
 
diff --git a/qemu-char.h b/qemu-char.h
index d7eed34..f984071 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -75,6 +75,7 @@ struct CharDriverState {
     char *filename;
     int opened;
     int avail_connections;
+    QemuOpts *opts;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
-- 
1.7.1

[Qemu-devel] [PATCH 3/9] chardev: reduce chardev ifdef mess a bit

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-char.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 9bb3a6b..208c525 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -853,6 +853,8 @@ static void cfmakeraw (struct termios *termios_p)
     || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
     || defined(__GLIBC__)
 
+#define HAVE_CHARDEV_TTY 1
+
 typedef struct {
     int fd;
     int connected;
@@ -1234,14 +1236,12 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
     chr->chr_close = qemu_chr_close_tty;
     return chr;
 }
-#else  /* ! __linux__ && ! __sun__ */
-static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
-{
-    return NULL;
-}
 #endif /* __linux__ || __sun__ */
 
 #if defined(__linux__)
+
+#define HAVE_CHARDEV_PARPORT 1
+
 typedef struct {
     int fd;
     int mode;
@@ -1385,6 +1385,9 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
 #endif /* __linux__ */
 
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
+
+#define HAVE_CHARDEV_PARPORT 1
+
 static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
 {
     int fd = (int)(intptr_t)chr->opaque;
@@ -2745,19 +2748,16 @@ static const struct {
 #else
     { .name = "file",      .open = qemu_chr_open_file_out },
     { .name = "pipe",      .open = qemu_chr_open_pipe },
-    { .name = "pty",       .open = qemu_chr_open_pty },
     { .name = "stdio",     .open = qemu_chr_open_stdio },
 #endif
 #ifdef CONFIG_BRLAPI
     { .name = "braille",   .open = chr_baum_init },
 #endif
-#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
-    || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
-    || defined(__FreeBSD_kernel__)
+#ifdef HAVE_CHARDEV_TTY
     { .name = "tty",       .open = qemu_chr_open_tty },
+    { .name = "pty",       .open = qemu_chr_open_pty },
 #endif
-#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__) \
-    || defined(__FreeBSD_kernel__)
+#ifdef HAVE_CHARDEV_PARPORT
     { .name = "parport",   .open = qemu_chr_open_pp },
 #endif
 #ifdef CONFIG_SPICE
-- 
1.7.1

[Qemu-devel] [PATCH 4/9] chardev: hotplug, qmp, null

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |   32 ++++++++++++++++++++++++++++++++
 qemu-char.c      |   34 ++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 116 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..e3f0d44 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3017,3 +3017,35 @@
 # Since: 1.3.0
 ##
 { 'command': 'nbd-server-stop' }
+
+##
+# @chardev-add:
+#
+# Add a file chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: backend type and parameters
+#
+# Returns: Nothing on success
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevDummy', 'data': { } }
+
+{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } }
+
+{ 'command': 'chardev-add', 'data': {'id'      : 'str',
+                                     'backend' : 'ChardevBackend' } }
+
+##
+# @chardev-remove:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.4
+##
+{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index 208c525..d2f0852 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2925,3 +2925,37 @@ CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
+{
+    CharDriverState *chr;
+
+    switch (backend->kind) {
+    case CHARDEV_BACKEND_KIND_NULL:
+        chr = qemu_chr_open_null(NULL);
+        break;
+    default:
+        error_setg(errp, "unknown chardev backend (%d)", backend->kind);
+        return;
+    }
+
+    if (chr == NULL) {
+        error_setg(errp, "Failed to create chardev");
+    }
+}
+
+void qmp_chardev_remove(const char *id, Error **errp)
+{
+    CharDriverState *chr;
+
+    chr = qemu_chr_find(id);
+    if (NULL == chr) {
+        error_setg(errp, "Chardev '%s' not found", id);
+        return;
+    }
+    if (chr->chr_can_read || chr->chr_read ||
+        chr->chr_event || chr->handler_opaque) {
+        error_setg(errp, "Chardev '%s' is busy", id);
+        return;
+    }
+    qemu_chr_delete(chr);
+}
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5c692d0..10408e9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2654,3 +2654,53 @@ EQMP
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_target,
     },
+
+    {
+        .name       = "chardev-add",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_add,
+    },
+
+SQMP
+chardev-add
+----------------
+
+Add a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must be unique (json-string)
+- "backend": chardev backend type + parameters
+
+Example:
+
+-> { "execute"   : "chardev-add",
+     "arguments" : { "id"   : "foo",
+                     FIXME } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "chardev-remove",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_remove,
+    },
+
+
+SQMP
+chardev-remove
+--------------
+
+Remove a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must exist and not be in use (json-string)
+
+Example:
+
+-> { "execute": "chardev-remove", "arguments": { "id" : "foo" } }
+<- { "return": {} }
+
+EQMP
-- 
1.7.1

[Qemu-devel] [PATCH 5/9] chardev: hotplug, hmp

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx |   32 ++++++++++++++++++++++++++++++++
 hmp.c           |   23 +++++++++++++++++++++++
 hmp.h           |    2 ++
 3 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..183e231 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1485,6 +1485,38 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
     {
+        .name       = "chardev_add",
+        .args_type  = "args:s",
+        .params     = "args",
+        .help       = "add chardev",
+        .mhandler.cmd = hmp_chardev_add,
+    },
+
+STEXI
+@item chardev_add args
+@findex chardev_add
+
+chardev_add accepts the same parameters as the -chardev command line switch.
+
+ETEXI
+
+    {
+        .name       = "chardev_remove",
+        .args_type  = "id:s",
+        .params     = "id",
+        .help       = "remove chardev",
+        .mhandler.cmd = hmp_chardev_remove,
+    },
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 180ba2b..7f3ddf5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1335,3 +1335,26 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
     qmp_nbd_server_stop(&errp);
     hmp_handle_error(mon, &errp);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+    const char *args = qdict_get_str(qdict, "args");
+    Error *err = NULL;
+    QemuOpts *opts;
+
+    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
+    if (opts == NULL) {
+        error_setg(&err, "Parsing chardev args failed\n");
+    } else {
+        qemu_chr_new_from_opts(opts, NULL, &err);
+    }
+    hmp_handle_error(mon, &err);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+    Error *local_err = NULL;
+
+    qmp_chardev_remove(qdict_get_str(qdict, "id"), &local_err);
+    hmp_handle_error(mon, &local_err);
+}
diff --git a/hmp.h b/hmp.h
index 0ab03be..e67e482 100644
--- a/hmp.h
+++ b/hmp.h
@@ -80,5 +80,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif
-- 
1.7.1

[Qemu-devel] [PATCH 6/9] chardev: hotplug, qmp, file

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    9 +++++-
 qemu-char.c      |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index e3f0d44..8904d36 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3030,9 +3030,16 @@
 #
 # Since: 1.4
 ##
+{ 'union': 'ChardevFileSource', 'data': { 'path' : 'str',
+                                          'fd'   : 'str' } }
+
+{ 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource',
+                                   'out' : 'ChardevFileSource' } }
+
 { 'type': 'ChardevDummy', 'data': { } }
 
-{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } }
+{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+                                       'null' : 'ChardevDummy' } }
 
 { 'command': 'chardev-add', 'data': {'id'      : 'str',
                                      'backend' : 'ChardevBackend' } }
diff --git a/qemu-char.c b/qemu-char.c
index d2f0852..47d55ea 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2925,11 +2925,86 @@ CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+#ifdef _WIN32
+
+static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+{
+    HANDLE out;
+
+    if (file->in) {
+        error_setg(errp, "input file not supported");
+        return NULL;
+    }
+    if (file->out->kind != CHARDEV_FILE_SOURCE_KIND_PATH) {
+        error_setg(errp, "only file paths supported");
+        return NULL;
+    }
+
+    out = CreateFile(file->out->path, GENERIC_WRITE, FILE_SHARE_READ, NULL,
+                     OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+    if (out == INVALID_HANDLE_VALUE) {
+        error_setg(errp, "open %s failed", file->out->path);
+        return NULL;
+    }
+    return qemu_chr_open_win_file(out);
+}
+
+#else /* WIN32 */
+
+static int qmp_chardev_open_file_source(ChardevFileSource *src, int flags,
+                                        Error **errp)
+{
+    int fd = -1;
+
+    switch (src->kind) {
+    case CHARDEV_FILE_SOURCE_KIND_PATH:
+        TFR(fd = qemu_open(src->path, flags, 0666));
+        if (fd == -1) {
+            error_setg(errp, "open %s: %s", src->path, strerror(errno));
+        }
+        break;
+    case CHARDEV_FILE_SOURCE_KIND_FD:
+        fd = monitor_get_fd(cur_mon, src->fd, errp);
+        break;
+    default:
+        error_setg(errp, "unknown chardev file source (%d)", src->kind);
+        break;
+    }
+    return fd;
+}
+
+static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+{
+    int flags, in = -1, out = -1;
+
+    flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
+    out = qmp_chardev_open_file_source(file->out, flags, errp);
+    if (error_is_set(errp)) {
+        return NULL;
+    }
+
+    if (file->in) {
+        flags = O_RDONLY;
+        in = qmp_chardev_open_file_source(file->in, flags, errp);
+        if (error_is_set(errp)) {
+            qemu_close(out);
+            return NULL;
+        }
+    }
+
+    return qemu_chr_open_fd(in, out);
+}
+
+#endif /* WIN32 */
+
 void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
 {
     CharDriverState *chr;
 
     switch (backend->kind) {
+    case CHARDEV_BACKEND_KIND_FILE:
+        chr = qmp_chardev_open_file(backend->file, errp);
+        break;
     case CHARDEV_BACKEND_KIND_NULL:
         chr = qemu_chr_open_null(NULL);
         break;
@@ -2938,7 +3013,7 @@ void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
         return;
     }
 
-    if (chr == NULL) {
+    if (chr == NULL && !error_is_set(errp)) {
         error_setg(errp, "Failed to create chardev");
     }
 }
-- 
1.7.1

[Qemu-devel] [PATCH 7/9] chardev: hotplug, qmp, tty

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    6 ++++++
 qemu-char.c      |   53 +++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 8904d36..7e5c8c2 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3036,9 +3036,15 @@
 { 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource',
                                    'out' : 'ChardevFileSource' } }
 
+{ 'enum': 'ChardevPortKind', 'data': [ 'tty' ] }
+
+{ 'type': 'ChardevPort', 'data': { 'device' : 'ChardevFileSource',
+                                   'type'   : 'ChardevPortKind'} }
+
 { 'type': 'ChardevDummy', 'data': { } }
 
 { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+                                       'port' : 'ChardevPort',
                                        'null' : 'ChardevDummy' } }
 
 { 'command': 'chardev-add', 'data': {'id'      : 'str',
diff --git a/qemu-char.c b/qemu-char.c
index 47d55ea..28e4991 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1220,21 +1220,27 @@ static void qemu_chr_close_tty(CharDriverState *chr)
     }
 }
 
+static CharDriverState *qemu_chr_open_tty_fd(int fd)
+{
+    CharDriverState *chr;
+
+    tty_serial_init(fd, 115200, 'N', 8, 1);
+    chr = qemu_chr_open_fd(fd, fd);
+    chr->chr_ioctl = tty_serial_ioctl;
+    chr->chr_close = qemu_chr_close_tty;
+    return chr;
+}
+
 static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
 {
     const char *filename = qemu_opt_get(opts, "path");
-    CharDriverState *chr;
     int fd;
 
     TFR(fd = qemu_open(filename, O_RDWR | O_NONBLOCK));
     if (fd < 0) {
         return NULL;
     }
-    tty_serial_init(fd, 115200, 'N', 8, 1);
-    chr = qemu_chr_open_fd(fd, fd);
-    chr->chr_ioctl = tty_serial_ioctl;
-    chr->chr_close = qemu_chr_close_tty;
-    return chr;
+    return qemu_chr_open_tty_fd(fd);
 }
 #endif /* __linux__ || __sun__ */
 
@@ -2949,6 +2955,15 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
     return qemu_chr_open_win_file(out);
 }
 
+static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
+{
+    switch (port->type) {
+    default:
+        error_setg(errp, "unknown chardev port (%d)", port->type);
+        return NULL;
+    }
+}
+
 #else /* WIN32 */
 
 static int qmp_chardev_open_file_source(ChardevFileSource *src, int flags,
@@ -2965,6 +2980,9 @@ static int qmp_chardev_open_file_source(ChardevFileSource *src, int flags,
         break;
     case CHARDEV_FILE_SOURCE_KIND_FD:
         fd = monitor_get_fd(cur_mon, src->fd, errp);
+        if (flags & O_NONBLOCK) {
+            socket_set_nonblock(fd);
+        }
         break;
     default:
         error_setg(errp, "unknown chardev file source (%d)", src->kind);
@@ -2995,6 +3013,26 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
     return qemu_chr_open_fd(in, out);
 }
 
+static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
+{
+    int flags, fd;
+
+    switch (port->type) {
+#ifdef HAVE_CHARDEV_TTY
+    case CHARDEV_PORT_KIND_TTY:
+        flags = O_RDWR | O_NONBLOCK;
+        fd = qmp_chardev_open_file_source(port->device, flags, errp);
+        if (error_is_set(errp)) {
+            return NULL;
+        }
+        return qemu_chr_open_tty_fd(fd);
+#endif
+    default:
+        error_setg(errp, "unknown chardev port (%d)", port->type);
+        return NULL;
+    }
+}
+
 #endif /* WIN32 */
 
 void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
@@ -3005,6 +3043,9 @@ void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
     case CHARDEV_BACKEND_KIND_FILE:
         chr = qmp_chardev_open_file(backend->file, errp);
         break;
+    case CHARDEV_BACKEND_KIND_PORT:
+        chr = qmp_chardev_open_port(backend->port, errp);
+        break;
     case CHARDEV_BACKEND_KIND_NULL:
         chr = qemu_chr_open_null(NULL);
         break;
-- 
1.7.1

[Qemu-devel] [PATCH 8/9] chardev: hotplug, qmp, serial

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    3 ++-
 qemu-char.c      |   16 ++++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 7e5c8c2..d833385 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3036,7 +3036,8 @@
 { 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource',
                                    'out' : 'ChardevFileSource' } }
 
-{ 'enum': 'ChardevPortKind', 'data': [ 'tty' ] }
+{ 'enum': 'ChardevPortKind', 'data': [ 'tty',
+                                       'serial' ] }
 
 { 'type': 'ChardevPort', 'data': { 'device' : 'ChardevFileSource',
                                    'type'   : 'ChardevPortKind'} }
diff --git a/qemu-char.c b/qemu-char.c
index 28e4991..9f23c25 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1662,9 +1662,8 @@ static int win_chr_poll(void *opaque)
     return 0;
 }
 
-static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_win_path(const char *filename)
 {
-    const char *filename = qemu_opt_get(opts, "path");
     CharDriverState *chr;
     WinCharState *s;
 
@@ -1683,6 +1682,11 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
     return chr;
 }
 
+static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
+{
+    return qemu_chr_open_win_path(qemu_opt_get(opts, "path"));
+}
+
 static int win_chr_pipe_poll(void *opaque)
 {
     CharDriverState *chr = opaque;
@@ -2957,7 +2961,14 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
 
 static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
 {
+    if (port->device->kind != CHARDEV_FILE_SOURCE_KIND_PATH) {
+        error_setg(errp, "only file paths supported");
+        return NULL;
+    }
+
     switch (port->type) {
+    case CHARDEV_PORT_KIND_SERIAL:
+        return qemu_chr_open_win_path(port->device->path);
     default:
         error_setg(errp, "unknown chardev port (%d)", port->type);
         return NULL;
@@ -3020,6 +3031,7 @@ static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
     switch (port->type) {
 #ifdef HAVE_CHARDEV_TTY
     case CHARDEV_PORT_KIND_TTY:
+    case CHARDEV_PORT_KIND_SERIAL:
         flags = O_RDWR | O_NONBLOCK;
         fd = qmp_chardev_open_file_source(port->device, flags, errp);
         if (error_is_set(errp)) {
-- 
1.7.1

[Qemu-devel] [PATCH 9/9] chardev: hotplug, qmp, parallel

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    3 ++-
 qemu-char.c      |   43 +++++++++++++++++++++++++++----------------
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index d833385..0922823 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3037,7 +3037,8 @@
                                    'out' : 'ChardevFileSource' } }
 
 { 'enum': 'ChardevPortKind', 'data': [ 'tty',
-                                       'serial' ] }
+                                       'serial',
+                                       'parport' ] }
 
 { 'type': 'ChardevPort', 'data': { 'device' : 'ChardevFileSource',
                                    'type'   : 'ChardevPortKind'} }
diff --git a/qemu-char.c b/qemu-char.c
index 9f23c25..5dedacc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1357,17 +1357,10 @@ static void pp_close(CharDriverState *chr)
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
-static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_pp_fd(int fd)
 {
-    const char *filename = qemu_opt_get(opts, "path");
     CharDriverState *chr;
     ParallelCharDriver *drv;
-    int fd;
-
-    TFR(fd = qemu_open(filename, O_RDWR));
-    if (fd < 0) {
-        return NULL;
-    }
 
     if (ioctl(fd, PPCLAIM) < 0) {
         close(fd);
@@ -1431,16 +1424,9 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
     return 0;
 }
 
-static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_pp_fd(int fd)
 {
-    const char *filename = qemu_opt_get(opts, "path");
     CharDriverState *chr;
-    int fd;
-
-    fd = qemu_open(filename, O_RDWR);
-    if (fd < 0) {
-        return NULL;
-    }
 
     chr = g_malloc0(sizeof(CharDriverState));
     chr->opaque = (void *)(intptr_t)fd;
@@ -2740,6 +2726,22 @@ fail:
     return NULL;
 }
 
+#ifdef HAVE_CHARDEV_PARPORT
+
+static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
+{
+    const char *filename = qemu_opt_get(opts, "path");
+    int fd;
+
+    fd = qemu_open(filename, O_RDWR);
+    if (fd < 0) {
+        return NULL;
+    }
+    return qemu_chr_open_pp_fd(fd);
+}
+
+#endif
+
 static const struct {
     const char *name;
     CharDriverState *(*open)(QemuOpts *opts);
@@ -3039,6 +3041,15 @@ static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
         }
         return qemu_chr_open_tty_fd(fd);
 #endif
+#ifdef HAVE_CHARDEV_PARPORT
+    case CHARDEV_PORT_KIND_PARPORT:
+        flags = O_RDWR;
+        fd = qmp_chardev_open_file_source(port->device, flags, errp);
+        if (error_is_set(errp)) {
+            return NULL;
+        }
+        return qemu_chr_open_pp_fd(fd);
+#endif
     default:
         error_setg(errp, "unknown chardev port (%d)", port->type);
         return NULL;
-- 
1.7.1

Re: [Qemu-devel] [PATCH 8/9] chardev: hotplug, qmp, serial

[Paolo Bonzini]

Il 19/12/2012 16:59, Gerd Hoffmann ha scritto:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi-schema.json |    3 ++-
>  qemu-char.c      |   16 ++++++++++++++--
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7e5c8c2..d833385 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3036,7 +3036,8 @@
>  { 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource',
>                                     'out' : 'ChardevFileSource' } }
>  
> -{ 'enum': 'ChardevPortKind', 'data': [ 'tty' ] }
> +{ 'enum': 'ChardevPortKind', 'data': [ 'tty',
> +                                       'serial' ] }

I think 'tty' and 'serial' are really the same thing, just one for
Windows and one for Linux.

We could make one a synonym of the other, even for -chardev.

Paolo

>  { 'type': 'ChardevPort', 'data': { 'device' : 'ChardevFileSource',
>                                     'type'   : 'ChardevPortKind'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index 28e4991..9f23c25 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1662,9 +1662,8 @@ static int win_chr_poll(void *opaque)
>      return 0;
>  }
>  
> -static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
> +static CharDriverState *qemu_chr_open_win_path(const char *filename)
>  {
> -    const char *filename = qemu_opt_get(opts, "path");
>      CharDriverState *chr;
>      WinCharState *s;
>  
> @@ -1683,6 +1682,11 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
>      return chr;
>  }
>  
> +static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
> +{
> +    return qemu_chr_open_win_path(qemu_opt_get(opts, "path"));
> +}
> +
>  static int win_chr_pipe_poll(void *opaque)
>  {
>      CharDriverState *chr = opaque;
> @@ -2957,7 +2961,14 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
>  
>  static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
>  {
> +    if (port->device->kind != CHARDEV_FILE_SOURCE_KIND_PATH) {
> +        error_setg(errp, "only file paths supported");
> +        return NULL;
> +    }
> +
>      switch (port->type) {
> +    case CHARDEV_PORT_KIND_SERIAL:
> +        return qemu_chr_open_win_path(port->device->path);
>      default:
>          error_setg(errp, "unknown chardev port (%d)", port->type);
>          return NULL;
> @@ -3020,6 +3031,7 @@ static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
>      switch (port->type) {
>  #ifdef HAVE_CHARDEV_TTY
>      case CHARDEV_PORT_KIND_TTY:
> +    case CHARDEV_PORT_KIND_SERIAL:
>          flags = O_RDWR | O_NONBLOCK;
>          fd = qmp_chardev_open_file_source(port->device, flags, errp);
>          if (error_is_set(errp)) {
> 

Re: [Qemu-devel] [PATCH 3/9] chardev: reduce chardev ifdef mess a bit

[Blue Swirl]

On Wed, Dec 19, 2012 at 3:59 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qemu-char.c |   22 +++++++++++-----------
>  1 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 9bb3a6b..208c525 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -853,6 +853,8 @@ static void cfmakeraw (struct termios *termios_p)
>      || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
>      || defined(__GLIBC__)
>
> +#define HAVE_CHARDEV_TTY 1

Perhaps the next step should be configure generating these #defines.

> +
>  typedef struct {
>      int fd;
>      int connected;
> @@ -1234,14 +1236,12 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
>      chr->chr_close = qemu_chr_close_tty;
>      return chr;
>  }
> -#else  /* ! __linux__ && ! __sun__ */
> -static CharDriverState *qemu_chr_open_pty(QemuOpts *opts)
> -{
> -    return NULL;
> -}
>  #endif /* __linux__ || __sun__ */
>
>  #if defined(__linux__)
> +
> +#define HAVE_CHARDEV_PARPORT 1
> +
>  typedef struct {
>      int fd;
>      int mode;
> @@ -1385,6 +1385,9 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts *opts)
>  #endif /* __linux__ */
>
>  #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__DragonFly__)
> +
> +#define HAVE_CHARDEV_PARPORT 1
> +
>  static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
>  {
>      int fd = (int)(intptr_t)chr->opaque;
> @@ -2745,19 +2748,16 @@ static const struct {
>  #else
>      { .name = "file",      .open = qemu_chr_open_file_out },
>      { .name = "pipe",      .open = qemu_chr_open_pipe },
> -    { .name = "pty",       .open = qemu_chr_open_pty },
>      { .name = "stdio",     .open = qemu_chr_open_stdio },
>  #endif
>  #ifdef CONFIG_BRLAPI
>      { .name = "braille",   .open = chr_baum_init },
>  #endif
> -#if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
> -    || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
> -    || defined(__FreeBSD_kernel__)
> +#ifdef HAVE_CHARDEV_TTY
>      { .name = "tty",       .open = qemu_chr_open_tty },
> +    { .name = "pty",       .open = qemu_chr_open_pty },
>  #endif
> -#if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__) \
> -    || defined(__FreeBSD_kernel__)
> +#ifdef HAVE_CHARDEV_PARPORT
>      { .name = "parport",   .open = qemu_chr_open_pp },
>  #endif
>  #ifdef CONFIG_SPICE
> --
> 1.7.1
>
>

Re: [Qemu-devel] [PATCH 8/9] chardev: hotplug, qmp, serial

[Gerd Hoffmann]

On 12/19/12 17:21, Paolo Bonzini wrote:
> Il 19/12/2012 16:59, Gerd Hoffmann ha scritto:
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>>  qapi-schema.json |    3 ++-
>>  qemu-char.c      |   16 ++++++++++++++--
>>  2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 7e5c8c2..d833385 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3036,7 +3036,8 @@
>>  { 'type': 'ChardevFile', 'data': { '*in' : 'ChardevFileSource',
>>                                     'out' : 'ChardevFileSource' } }
>>  
>> -{ 'enum': 'ChardevPortKind', 'data': [ 'tty' ] }
>> +{ 'enum': 'ChardevPortKind', 'data': [ 'tty',
>> +                                       'serial' ] }
> 
> I think 'tty' and 'serial' are really the same thing, just one for
> Windows and one for Linux.

Partly disagree.

tty is a very unix-ish concept, having that on windows would be
confusing I think.

serial lines happen to be a subset of tty on linux.  The patch aliases
serial to tty on unix because of that.

> We could make one a synonym of the other, even for -chardev.

Makes sense to be consistent here.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Michal Privoznik]

On 19.12.2012 16:58, Gerd Hoffmann wrote:
>   Hi,
> 
> Chardev hotplug patch series reloaded.  Not finished yet, commit
> messages not finalized yet, totally untested other than building on
> linux+windows.
> 
> I doubt I manage to finish (and test!) it before xmas.  Nevertheless I
> wanna get the bits out of the door for you to see what is coming and for
> early reviews.  It goes largely the route suggested by Paolo.
> 
> cheers,
>   Gerd
> 
> Gerd Hoffmann (9):
>   chardev: add error reporting for qemu_chr_new_from_opts
>   chardev: fix QemuOpts lifecycle
>   chardev: reduce chardev ifdef mess a bit
>   chardev: hotplug, qmp, null
>   chardev: hotplug, hmp
>   chardev: hotplug, qmp, file
>   chardev: hotplug, qmp, tty
>   chardev: hotplug, qmp, serial
>   chardev: hotplug, qmp, parallel
> 
>  hmp-commands.hx  |   32 ++++++
>  hmp.c            |   23 +++++
>  hmp.h            |    2 +
>  qapi-schema.json |   47 +++++++++
>  qemu-char.c      |  280 ++++++++++++++++++++++++++++++++++++++++++++---------
>  qemu-char.h      |    4 +-
>  qmp-commands.hx  |   50 ++++++++++
>  vl.c             |    9 +-
>  8 files changed, 395 insertions(+), 52 deletions(-)
> 

Okay, the QMP interface seems sane to me (from libvirt POV). However,
what about other chardev types like pipe and vc? And I guess pty can be
covered by tty, right?

Michal

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Daniel P. Berrange]

On Thu, Dec 20, 2012 at 11:49:29AM +0100, Michal Privoznik wrote:
> On 19.12.2012 16:58, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > Chardev hotplug patch series reloaded.  Not finished yet, commit
> > messages not finalized yet, totally untested other than building on
> > linux+windows.
> > 
> > I doubt I manage to finish (and test!) it before xmas.  Nevertheless I
> > wanna get the bits out of the door for you to see what is coming and for
> > early reviews.  It goes largely the route suggested by Paolo.
> > 
> > cheers,
> >   Gerd
> > 
> > Gerd Hoffmann (9):
> >   chardev: add error reporting for qemu_chr_new_from_opts
> >   chardev: fix QemuOpts lifecycle
> >   chardev: reduce chardev ifdef mess a bit
> >   chardev: hotplug, qmp, null
> >   chardev: hotplug, hmp
> >   chardev: hotplug, qmp, file
> >   chardev: hotplug, qmp, tty
> >   chardev: hotplug, qmp, serial
> >   chardev: hotplug, qmp, parallel
> > 
> >  hmp-commands.hx  |   32 ++++++
> >  hmp.c            |   23 +++++
> >  hmp.h            |    2 +
> >  qapi-schema.json |   47 +++++++++
> >  qemu-char.c      |  280 ++++++++++++++++++++++++++++++++++++++++++++---------
> >  qemu-char.h      |    4 +-
> >  qmp-commands.hx  |   50 ++++++++++
> >  vl.c             |    9 +-
> >  8 files changed, 395 insertions(+), 52 deletions(-)
> > 
> 
> Okay, the QMP interface seems sane to me (from libvirt POV). However,
> what about other chardev types like pipe and vc? And I guess pty can be
> covered by tty, right?

No, QEMU's  pty & tty types are different - the latter is about accessing
host serial devices

>From libvirt's POV, I think the most important chardev types are pty,
unix and tcp. The other types are pretty rarely used AFAICT.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Paolo Bonzini]

Il 20/12/2012 11:56, Daniel P. Berrange ha scritto:
> On Thu, Dec 20, 2012 at 11:49:29AM +0100, Michal Privoznik wrote:
>> On 19.12.2012 16:58, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>> Chardev hotplug patch series reloaded.  Not finished yet, commit
>>> messages not finalized yet, totally untested other than building on
>>> linux+windows.
>>>
>>> I doubt I manage to finish (and test!) it before xmas.
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

>> Okay, the QMP interface seems sane to me (from libvirt POV). However,
>> what about other chardev types like pipe and vc? And I guess pty can be
>> covered by tty, right?

I think that is the missing part.

Paolo

> No, QEMU's  pty & tty types are different - the latter is about accessing
> host serial devices
> 
> From libvirt's POV, I think the most important chardev types are pty,
> unix and tcp. The other types are pretty rarely used AFAICT.
> 
> 
> Daniel
> 

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Gerd Hoffmann]

>>>> I doubt I manage to finish (and test!) it before xmas.
>     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
>>> Okay, the QMP interface seems sane to me (from libvirt POV). However,
>>> what about other chardev types like pipe and vc? And I guess pty can be
>>> covered by tty, right?
> 
> I think that is the missing part.

Exactly.

/me wades through the socket code (unix+tcp) right now, which needs some
refactoring to make it fly.

>> From libvirt's POV, I think the most important chardev types are pty,
>> unix and tcp. The other types are pretty rarely used AFAICT.

pty looks like another non-trivial challenge.  How does libvirt gather
the pty device today?  IIRC there is some stderr parsing?  Or was it
info chardev?  With QMP we probably want switch to a more sane model
here ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Gerd Hoffmann]

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

  Hi,

> /me wades through the socket code (unix+tcp) right now, which needs some
> refactoring to make it fly.

Sneak preview attached.  Goes on top of the series.
Compile tested only so far.

enjoy,
  Gerd

[-- Attachment #2: 0001-chardev-hotplug-qmp-socket.patch --]
[-- Type: text/plain, Size: 9258 bytes --]

From 2c5568c20d97d497b43c9af1c1c1388db089c1e9 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 20 Dec 2012 13:53:12 +0100
Subject: [PATCH] chardev: hotplug, qmp, socket

qemu_chr_open_socket is splitted into two functions.  All initialization
after creating the socket file handler is splitted away into the new
qemu_chr_open_socket_fd function.

chr->filename doesn't get filled from QemuOpts any more.  Qemu gathers
the information using getsockname and getnameinfo instead.  This way it
will also work correctly for file handles passed via file descriptor
passing.

Finally qmp_chardev_open_socket() is the actual qmp hotplug
implementation which basically just calls socket_listen or
socket_connect and the new qemu_chr_open_socket_fd function.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |   13 +++-
 qemu-char.c      |  168 ++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 124 insertions(+), 57 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 0922823..39e0ab4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3043,11 +3043,18 @@
 { 'type': 'ChardevPort', 'data': { 'device' : 'ChardevFileSource',
                                    'type'   : 'ChardevPortKind'} }
 
+{ 'type': 'ChardevSocket', 'data': { 'addr'    : 'SocketAddress',
+                                     '*server' : 'bool',
+                                     '*wait'   : 'bool',
+                                     '*delay'  : 'bool',
+                                     '*telnet' : 'bool' } }
+
 { 'type': 'ChardevDummy', 'data': { } }
 
-{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
-                                       'port' : 'ChardevPort',
-                                       'null' : 'ChardevDummy' } }
+{ 'union': 'ChardevBackend', 'data': { 'file'   : 'ChardevFile',
+                                       'port'   : 'ChardevPort',
+                                       'socket' : 'ChardevSocket',
+                                       'null'   : 'ChardevDummy' } }
 
 { 'command': 'chardev-add', 'data': {'id'      : 'str',
                                      'backend' : 'ChardevBackend' } }
diff --git a/qemu-char.c b/qemu-char.c
index 5a6d3c5..15f2ae2 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2427,10 +2427,88 @@ static void tcp_chr_close(CharDriverState *chr)
     qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
 }
 
-static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
+static CharDriverState *qemu_chr_open_socket_fd(int fd, int do_nodelay,
+                                                int is_listen, int is_telnet,
+                                                int is_waitconnect,
+                                                Error **errp)
 {
     CharDriverState *chr = NULL;
     TCPCharDriver *s = NULL;
+    char host[NI_MAXHOST], serv[NI_MAXSERV];
+    const char *left = "", *right = "";
+    struct sockaddr_storage ss;
+    socklen_t ss_len = sizeof(ss);
+
+    memset(&ss, 0, ss_len);
+    if (getsockname(fd, (struct sockaddr *) &ss, &ss_len) != 0) {
+        error_setg(errp, "getsockname: %s", strerror(errno));
+        return NULL;
+    }
+
+    chr = g_malloc0(sizeof(CharDriverState));
+    s = g_malloc0(sizeof(TCPCharDriver));
+
+    s->connected = 0;
+    s->fd = -1;
+    s->listen_fd = -1;
+    s->msgfd = -1;
+
+    chr->filename = g_malloc(256);
+    switch (ss.ss_family) {
+#ifndef _WIN32
+    case AF_UNIX:
+        s->is_unix = 1;
+        snprintf(chr->filename, 256, "unix:%s%s",
+                 ((struct sockaddr_un *)(&ss))->sun_path,
+                 is_listen ? ",server" : "");
+        break;
+#endif
+    case AF_INET6:
+        left  = "[";
+        right = "]";
+        /* fall through */
+    case AF_INET:
+        s->do_nodelay = do_nodelay;
+        getnameinfo((struct sockaddr *) &ss, ss_len, host, sizeof(host),
+                    serv, sizeof(serv), NI_NUMERICHOST | NI_NUMERICSERV);
+        snprintf(chr->filename, 256, "%s:%s:%s%s%s%s",
+                 is_telnet ? "telnet" : "tcp",
+                 left, host, right, serv,
+                 is_listen ? ",server" : "");
+        break;
+    }
+
+    chr->opaque = s;
+    chr->chr_write = tcp_chr_write;
+    chr->chr_close = tcp_chr_close;
+    chr->get_msgfd = tcp_get_msgfd;
+    chr->chr_add_client = tcp_chr_add_client;
+
+    if (is_listen) {
+        s->listen_fd = fd;
+        qemu_set_fd_handler2(s->listen_fd, NULL, tcp_chr_accept, NULL, chr);
+        if (is_telnet) {
+            s->do_telnetopt = 1;
+        }
+    } else {
+        s->connected = 1;
+        s->fd = fd;
+        socket_set_nodelay(fd);
+        tcp_chr_connect(chr);
+    }
+
+    if (is_listen && is_waitconnect) {
+        printf("QEMU waiting for connection on: %s\n",
+               chr->filename);
+        tcp_chr_accept(chr);
+        socket_set_nonblock(s->listen_fd);
+    }
+    return chr;
+}
+
+static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
+{
+    CharDriverState *chr = NULL;
     Error *local_err = NULL;
     int fd = -1;
     int is_listen;
@@ -2447,10 +2525,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     if (!is_listen)
         is_waitconnect = 0;
 
-    chr = g_malloc0(sizeof(CharDriverState));
-    s = g_malloc0(sizeof(TCPCharDriver));
-
-    if (is_unix) {
+     if (is_unix) {
         if (is_listen) {
             fd = unix_listen_opts(opts, &local_err);
         } else {
@@ -2470,56 +2545,14 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     if (!is_waitconnect)
         socket_set_nonblock(fd);
 
-    s->connected = 0;
-    s->fd = -1;
-    s->listen_fd = -1;
-    s->msgfd = -1;
-    s->is_unix = is_unix;
-    s->do_nodelay = do_nodelay && !is_unix;
-
-    chr->opaque = s;
-    chr->chr_write = tcp_chr_write;
-    chr->chr_close = tcp_chr_close;
-    chr->get_msgfd = tcp_get_msgfd;
-    chr->chr_add_client = tcp_chr_add_client;
-
-    if (is_listen) {
-        s->listen_fd = fd;
-        qemu_set_fd_handler2(s->listen_fd, NULL, tcp_chr_accept, NULL, chr);
-        if (is_telnet)
-            s->do_telnetopt = 1;
-
-    } else {
-        s->connected = 1;
-        s->fd = fd;
-        socket_set_nodelay(fd);
-        tcp_chr_connect(chr);
-    }
-
-    /* for "info chardev" monitor command */
-    chr->filename = g_malloc(256);
-    if (is_unix) {
-        snprintf(chr->filename, 256, "unix:%s%s",
-                 qemu_opt_get(opts, "path"),
-                 qemu_opt_get_bool(opts, "server", 0) ? ",server" : "");
-    } else if (is_telnet) {
-        snprintf(chr->filename, 256, "telnet:%s:%s%s",
-                 qemu_opt_get(opts, "host"), qemu_opt_get(opts, "port"),
-                 qemu_opt_get_bool(opts, "server", 0) ? ",server" : "");
-    } else {
-        snprintf(chr->filename, 256, "tcp:%s:%s%s",
-                 qemu_opt_get(opts, "host"), qemu_opt_get(opts, "port"),
-                 qemu_opt_get_bool(opts, "server", 0) ? ",server" : "");
-    }
-
-    if (is_listen && is_waitconnect) {
-        printf("QEMU waiting for connection on: %s\n",
-               chr->filename);
-        tcp_chr_accept(chr);
-        socket_set_nonblock(s->listen_fd);
+    chr = qemu_chr_open_socket_fd(fd, do_nodelay, is_listen, is_telnet,
+                                  is_waitconnect, &local_err);
+    if (error_is_set(&local_err)) {
+        goto fail;
     }
     return chr;
 
+
  fail:
     if (local_err) {
         qerror_report_err(local_err);
@@ -2528,8 +2561,10 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts)
     if (fd >= 0) {
         closesocket(fd);
     }
-    g_free(s);
-    g_free(chr);
+    if (chr) {
+        g_free(chr->opaque);
+        g_free(chr);
+    }
     return NULL;
 }
 
@@ -3057,6 +3092,28 @@ static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
 
 #endif /* WIN32 */
 
+static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
+                                                Error **errp)
+{
+    SocketAddress *addr = sock->addr;
+    bool do_nodelay     = sock->has_delay  ? !sock->delay : true;
+    bool is_listen      = sock->has_server ? sock->server : true;
+    bool is_telnet      = sock->has_telnet ? sock->telnet : false;
+    bool is_waitconnect = sock->has_wait   ? sock->wait   : false;
+    int fd;
+
+    if (is_listen) {
+        fd = socket_listen(addr, errp);
+    } else {
+        fd = socket_connect(addr, errp, NULL, NULL);
+    }
+    if (error_is_set(errp)) {
+        return NULL;
+    }
+    return qemu_chr_open_socket_fd(fd, do_nodelay, is_listen,
+                                   is_telnet, is_waitconnect, errp);
+}
+
 void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
 {
     CharDriverState *chr;
@@ -3068,6 +3125,9 @@ void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
     case CHARDEV_BACKEND_KIND_PORT:
         chr = qmp_chardev_open_port(backend->port, errp);
         break;
+    case CHARDEV_BACKEND_KIND_SOCKET:
+        chr = qmp_chardev_open_socket(backend->socket, errp);
+        break;
     case CHARDEV_BACKEND_KIND_NULL:
         chr = qemu_chr_open_null(NULL);
         break;
-- 
1.7.1

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Daniel P. Berrange]

On Thu, Dec 20, 2012 at 12:45:33PM +0100, Gerd Hoffmann wrote:
> >>>> I doubt I manage to finish (and test!) it before xmas.
> >     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> >>> Okay, the QMP interface seems sane to me (from libvirt POV). However,
> >>> what about other chardev types like pipe and vc? And I guess pty can be
> >>> covered by tty, right?
> > 
> > I think that is the missing part.
> 
> Exactly.
> 
> /me wades through the socket code (unix+tcp) right now, which needs some
> refactoring to make it fly.
> 
> >> From libvirt's POV, I think the most important chardev types are pty,
> >> unix and tcp. The other types are pretty rarely used AFAICT.
> 
> pty looks like another non-trivial challenge.  How does libvirt gather
> the pty device today?  IIRC there is some stderr parsing?  Or was it
> info chardev?  With QMP we probably want switch to a more sane model
> here ...

Yes, these days we use info chardev, or query-chardev as appropriate
for the monitor mode.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Gerd Hoffmann]

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

  Hi,

>> pty looks like another non-trivial challenge.  How does libvirt gather
>> the pty device today?  IIRC there is some stderr parsing?  Or was it
>> info chardev?  With QMP we probably want switch to a more sane model
>> here ...
> 
> Yes, these days we use info chardev, or query-chardev as appropriate
> for the monitor mode.

So do you want continue using query-chardev, or do you prefer to get the
path returned directly, i.e. something like the attached patch?

cheers,
  Gerd


[-- Attachment #2: 0001-chardev-hotplug-qmp-pty.patch --]
[-- Type: text/plain, Size: 3013 bytes --]

From a616ae73d545a80b3a545fdc66bf141a9b3ba004 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 20 Dec 2012 14:39:13 +0100
Subject: [PATCH] chardev: hotplug, qmp, pty

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    7 ++++++-
 qemu-char.c      |   24 +++++++++++++++++++++---
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 39e0ab4..63c61c4 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3054,10 +3054,15 @@
 { 'union': 'ChardevBackend', 'data': { 'file'   : 'ChardevFile',
                                        'port'   : 'ChardevPort',
                                        'socket' : 'ChardevSocket',
+                                       'pty'    : 'ChardevDummy',
                                        'null'   : 'ChardevDummy' } }
 
+{ 'union': 'ChardevReturn', 'data': { 'pty'    : 'str',
+                                      'nodata' : 'ChardevDummy' } }
+
 { 'command': 'chardev-add', 'data': {'id'      : 'str',
-                                     'backend' : 'ChardevBackend' } }
+                                     'backend' : 'ChardevBackend' },
+  'returns' : 'ChardevReturn' }
 
 ##
 # @chardev-remove:
diff --git a/qemu-char.c b/qemu-char.c
index 911649a..7f65d40 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3115,9 +3115,13 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
                                    is_telnet, is_waitconnect, errp);
 }
 
-void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
+ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
+                               Error **errp)
 {
-    CharDriverState *chr;
+    ChardevReturn *ret = g_new0(ChardevReturn, 1);
+    CharDriverState *chr = NULL;
+
+    ret->kind = CHARDEV_RETURN_KIND_NODATA;
 
     switch (backend->kind) {
     case CHARDEV_BACKEND_KIND_FILE:
@@ -3129,17 +3133,31 @@ void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
     case CHARDEV_BACKEND_KIND_SOCKET:
         chr = qmp_chardev_open_socket(backend->socket, errp);
         break;
+#ifdef HAVE_CHARDEV_TTY
+    case CHARDEV_BACKEND_KIND_PTY:
+    {
+        /* qemu_chr_open_pty sets "path" in opts */
+        QemuOpts *opts;
+        opts = qemu_opts_create_nofail(qemu_find_opts("chardev"));
+        chr = qemu_chr_open_pty(opts);
+        ret->kind = CHARDEV_RETURN_KIND_PTY;
+        ret->pty = g_strdup(qemu_opt_get(opts, "path"));
+        qemu_opts_del(opts);
+        break;
+    }
+#endif
     case CHARDEV_BACKEND_KIND_NULL:
         chr = qemu_chr_open_null(NULL);
         break;
     default:
         error_setg(errp, "unknown chardev backend (%d)", backend->kind);
-        return;
+        break;
     }
 
     if (chr == NULL && !error_is_set(errp)) {
         error_setg(errp, "Failed to create chardev");
     }
+    return ret;
 }
 
 void qmp_chardev_remove(const char *id, Error **errp)
-- 
1.7.1

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Daniel P. Berrange]

On Fri, Dec 21, 2012 at 10:01:46AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> >> pty looks like another non-trivial challenge.  How does libvirt gather
> >> the pty device today?  IIRC there is some stderr parsing?  Or was it
> >> info chardev?  With QMP we probably want switch to a more sane model
> >> here ...
> > 
> > Yes, these days we use info chardev, or query-chardev as appropriate
> > for the monitor mode.
> 
> So do you want continue using query-chardev, or do you prefer to get the
> path returned directly, i.e. something like the attached patch?

If you return the path directly, that'd certainly make life easier

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Gerd Hoffmann]

On 12/20/12 14:02, Gerd Hoffmann wrote:
>   Hi,
> 
>> /me wades through the socket code (unix+tcp) right now, which needs some
>> refactoring to make it fly.
> 
> Sneak preview attached.  Goes on top of the series.
> Compile tested only so far.

Now that it comes to testing: how does the union look (in josn) at the wire?

thanks,
  Gerd

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Paolo Bonzini]

Il 21/12/2012 12:45, Gerd Hoffmann ha scritto:
> On 12/20/12 14:02, Gerd Hoffmann wrote:
>>   Hi,
>>
>>> /me wades through the socket code (unix+tcp) right now, which needs some
>>> refactoring to make it fly.
>>
>> Sneak preview attached.  Goes on top of the series.
>> Compile tested only so far.
> 
> Now that it comes to testing: how does the union look (in josn) at the wire?

Tests are your friends! (Especially qapi-schema-test.json and
test-qmp-input-visitor.c).  Something like this:

  { 'type': 'UserDefA',
    'data': { 'boolean': 'bool' } }

  { 'type': 'UserDefB',
    'data': { 'integer': 'int' } }

  { 'union': 'UserDefUnion',
    'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }

looks like this:

  { 'type': 'b', 'data' : { 'integer': 42 } }

Paolo

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Gerd Hoffmann]

On 12/21/12 12:53, Paolo Bonzini wrote:
> Il 21/12/2012 12:45, Gerd Hoffmann ha scritto:
>> On 12/20/12 14:02, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>>> /me wades through the socket code (unix+tcp) right now, which needs some
>>>> refactoring to make it fly.
>>>
>>> Sneak preview attached.  Goes on top of the series.
>>> Compile tested only so far.
>>
>> Now that it comes to testing: how does the union look (in josn) at the wire?
> 
> Tests are your friends! (Especially qapi-schema-test.json and
> test-qmp-input-visitor.c).  Something like this:
> 
>   { 'type': 'UserDefA',
>     'data': { 'boolean': 'bool' } }
> 
>   { 'type': 'UserDefB',
>     'data': { 'integer': 'int' } }
> 
>   { 'union': 'UserDefUnion',
>     'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
> 
> looks like this:
> 
>   { 'type': 'b', 'data' : { 'integer': 42 } }

So ...

{ 'type': 'ChardevDummy', 'data': { } }

{ 'union': 'ChardevBackend', 'data': { 'file'   : 'ChardevFile',
                                       'port'   : 'ChardevPort',
                                       'socket' : 'ChardevSocket',
                                       'pty'    : 'ChardevDummy',
                                       'null'   : 'ChardevDummy' } }

{ 'command': 'chardev-add', 'data': {'id'      : 'str',
                                     'backend' : 'ChardevBackend' },

... should accept ...

{'id': 'test-null', 'backend': {'data': {}, 'type': 'null'}}

... as parameters for chardev-add, no?  But I get back ...

 {u'error': {u'class': u'GenericError', u'desc': u"Invalid parameter
'backend'"}}

... and can't see what is wrong there ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Paolo Bonzini]

Il 21/12/2012 13:17, Gerd Hoffmann ha scritto:
> { 'type': 'ChardevDummy', 'data': { } }
> 
> { 'union': 'ChardevBackend', 'data': { 'file'   : 'ChardevFile',
>                                        'port'   : 'ChardevPort',
>                                        'socket' : 'ChardevSocket',
>                                        'pty'    : 'ChardevDummy',
>                                        'null'   : 'ChardevDummy' } }
> 
> { 'command': 'chardev-add', 'data': {'id'      : 'str',
>                                      'backend' : 'ChardevBackend' },
> 
> ... should accept ...
> 
> {'id': 'test-null', 'backend': {'data': {}, 'type': 'null'}}
> 
> ... as parameters for chardev-add, no?  But I get back ...
> 
>  {u'error': {u'class': u'GenericError', u'desc': u"Invalid parameter
> 'backend'"}}
> 
> ... and can't see what is wrong there ...

Me neither, but this quick test seems to work here:

$ tests/test-qmp-commands
/gerd: test-null 1
OK

diff --git a/qapi-schema-test.json b/qapi-schema-test.json
index 9eae350..c2e9316 100644
--- a/qapi-schema-test.json
+++ b/qapi-schema-test.json
@@ -36,3 +36,12 @@
 { 'command': 'user_def_cmd', 'data': {} }
 { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
 { 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' }
+
+
+{ 'type': 'ChardevDummy', 'data': { } }
+
+{ 'union': 'ChardevBackend', 'data': { 'pty'    : 'ChardevDummy',
+                                       'null'   : 'ChardevDummy' } }
+
+{ 'command': 'chardev-add', 'data': {'id'      : 'str',
+                                     'backend' : 'ChardevBackend' } }
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 5a3e82a..96396db 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -55,6 +55,11 @@ static void test_dispatch_cmd(void)
     QDECREF(req);
 }
 
+void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp)
+{
+	printf("%s %d\n", id, backend->kind);
+}
+
 /* test commands that return an error due to invalid parameters */
 static void test_dispatch_cmd_error(void)
 {
@@ -71,6 +76,20 @@ static void test_dispatch_cmd_error(void)
     QDECREF(req);
 }
 
+static void test_dispatch_gerd(void)
+{
+    QObject *req = qobject_from_json(
+	    "{'execute': 'chardev-add', "
+	    " 'arguments': {'id': 'test-null', "
+	    "               'backend': {'data': {}, 'type': 'null'}}} ");
+    QObject *resp = qmp_dispatch(req);
+    assert(resp != NULL);
+    assert(!qdict_haskey(qobject_to_qdict(resp), "error"));
+
+    qobject_decref(resp);
+    qobject_decref(req);
+}
+
 /* test commands that involve both input parameters and return values */
 static void test_dispatch_cmd_io(void)
 {
@@ -174,6 +193,7 @@ int main(int argc, char **argv)
     g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
     g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
     g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
+    g_test_add_func("/gerd", test_dispatch_gerd);
 
     module_call_init(MODULE_INIT_QAPI);
     g_test_run();

Paolo

Re: [Qemu-devel] [PATCH RfC 0/9] chardev hotplug

[Gerd Hoffmann]

  Hi,

>> ... and can't see what is wrong there ...
> 
> Me neither, but this quick test seems to work here:

Test works for me too.

Bug turned out to be elsewhere: args_type in qmp-commands.hx not set
correctly ...

cheers,
  Gerd