[Qemu-devel] [PATCH v2 00/10] chardev hotplug patch series

[Qemu-devel] [PATCH v2 00/10] chardev hotplug patch series

[Gerd Hoffmann]

  Hi,

Next round of chardev hotplug support patches, 
Updated according to the review comments from Paolo & Eric.
Simplified QMP syntax a bit, mostly due to to dropping fd
passing support as this can handled via add-fd + magic
"/dev/fdset/..." path.  Improved documentation.

cheers,
  Gerd

Gerd Hoffmann (10):
  chardev: add error reporting for qemu_chr_new_from_opts
  chardev: fix QemuOpts lifecycle
  chardev: reduce chardev ifdef mess a bit
  chardev: add qmp hotplug commands, with null chardev support
  chardev: add hmp hotplug commands
  chardev: add file chardev support to chardev-add (qmp)
  chardev: add serial chardev support to chardev-add (qmp)
  chardev: add parallel chardev support to chardev-add (qmp)
  chardev: add socket chardev support to chardev-add (qmp)
  chardev: add pty chardev support to chardev-add (qmp)

 hmp-commands.hx     |   32 ++++
 hmp.c               |   23 +++
 hmp.h               |    2 +
 include/char/char.h |    4 +-
 qapi-schema.json    |   89 ++++++++++
 qemu-char.c         |  454 +++++++++++++++++++++++++++++++++++++++------------
 qemu-options.hx     |   14 +-
 qmp-commands.hx     |   61 +++++++
 vl.c                |    9 +-
 9 files changed, 575 insertions(+), 113 deletions(-)

[Qemu-devel] [PATCH v2 01/10] chardev: add error reporting for qemu_chr_new_from_opts

[Gerd Hoffmann]

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

diff --git a/include/char/char.h b/include/char/char.h
index baa5d03..1952a10 100644
--- a/include/char/char.h
+++ b/include/char/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/qemu-char.c b/qemu-char.c
index f41788c..91f64e9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2779,19 +2779,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++) {
@@ -2799,15 +2800,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;
     }
 
@@ -2837,6 +2838,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);
@@ -2846,7 +2848,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/vl.c b/vl.c
index 79e5122..b4ea3ae 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 v2 02/10] 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>
---
 include/char/char.h |    1 +
 qemu-char.c         |   20 ++++++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/include/char/char.h b/include/char/char.h
index 1952a10..c91ce3c 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -75,6 +75,7 @@ struct CharDriverState {
     char *filename;
     int opened;
     int avail_connections;
+    QemuOpts *opts;
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
diff --git a/qemu-char.c b/qemu-char.c
index 91f64e9..a29c2bb 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2787,13 +2787,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)
@@ -2802,14 +2802,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)
@@ -2830,7 +2830,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))
@@ -2856,7 +2861,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;
 }
 
@@ -2884,10 +2888,14 @@ void qemu_chr_fe_close(struct CharDriverState *chr)
 void qemu_chr_delete(CharDriverState *chr)
 {
     QTAILQ_REMOVE(&chardevs, chr, next);
-    if (chr->chr_close)
+    if (chr->chr_close) {
         chr->chr_close(chr);
+    }
     g_free(chr->filename);
     g_free(chr->label);
+    if (chr->opts) {
+        qemu_opts_del(chr->opts);
+    }
     g_free(chr);
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 03/10] 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 a29c2bb..c511de3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -856,6 +856,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;
@@ -1244,14 +1246,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;
@@ -1395,6 +1395,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;
@@ -2755,19 +2758,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 v2 04/10] chardev: add qmp hotplug commands, with null chardev support

[Gerd Hoffmann]

Add chardev-add and chardev-remove qmp commands.  Hotplugging
a null chardev is supported for now, more will be added later.

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

diff --git a/qapi-schema.json b/qapi-schema.json
index 5dfa052..53d4b9e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3017,3 +3017,52 @@
 # Since: 1.3.0
 ##
 { 'command': 'nbd-server-stop' }
+
+##
+# @ChardevBackend:
+#
+# Configuration info for the new chardev backend.
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevDummy', 'data': { } }
+
+{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } }
+
+##
+# @ChardevReturn:
+#
+# Return infos about the chardev backend just created.
+#
+# Since: 1.4
+##
+{ 'type' : 'ChardevReturn', 'data': { } }
+
+##
+# @chardev-add:
+#
+# Add a file chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: backend type and parameters
+#
+# Returns: chardev info.
+#
+# Since: 1.4
+##
+{ 'command': 'chardev-add', 'data': {'id'      : 'str',
+                                     'backend' : 'ChardevBackend' },
+  'returns': 'ChardevReturn' }
+
+##
+# @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 c511de3..62d35b6 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2938,3 +2938,45 @@ CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
+                               Error **errp)
+{
+    ChardevReturn *ret = g_new0(ChardevReturn, 1);
+    CharDriverState *chr = NULL;
+
+    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);
+        break;
+    }
+
+    if (chr == NULL && !error_is_set(errp)) {
+        error_setg(errp, "Failed to create chardev");
+    }
+    if (chr) {
+        chr->label = g_strdup(id);
+        chr->avail_connections = 1;
+        QTAILQ_INSERT_TAIL(&chardevs, chr, next);
+    }
+    return ret;
+}
+
+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..c9ab37c 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  = "id:s,backend:q",
+        .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",
+                     "backend" : { "type" : "null", "data" : {} } } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "chardev-remove",
+        .args_type  = "id:s",
+        .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 v2 05/10] chardev: add hmp hotplug commands

[Gerd Hoffmann]

Add chardev-add and chardev-remove commands to the human monitor.
chardev-add accepts the same syntax as -chardev, chardev-remove
expects a chardev id.

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..67569ef 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 9e9e624..68929b4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1336,3 +1336,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 21f3e05..700fbdc 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 v2 06/10] chardev: add file chardev support to chardev-add (qmp)

[Gerd Hoffmann]

Add support for file chardevs.  Output file is mandatory,
input file is optional.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |   13 ++++++++++-
 qemu-char.c      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |    8 ++++++-
 3 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 53d4b9e..7930139 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3019,6 +3019,16 @@
 { 'command': 'nbd-server-stop' }
 
 ##
+# @ChardevFile:
+#
+# Configuration info for file chardevs.
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevFile', 'data': { '*in' : 'str',
+                                   'out' : 'str' } }
+
+##
 # @ChardevBackend:
 #
 # Configuration info for the new chardev backend.
@@ -3027,7 +3037,8 @@
 ##
 { 'type': 'ChardevDummy', 'data': { } }
 
-{ 'union': 'ChardevBackend', 'data': { 'null' : 'ChardevDummy' } }
+{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+                                       'null' : 'ChardevDummy' } }
 
 ##
 # @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index 62d35b6..fa0a884 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2938,6 +2938,64 @@ 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;
+    }
+
+    out = CreateFile(file->out, 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);
+        return NULL;
+    }
+    return qemu_chr_open_win_file(out);
+}
+
+#else /* WIN32 */
+
+static int qmp_chardev_open_file_source(char *src, int flags,
+                                        Error **errp)
+{
+    int fd = -1;
+
+    TFR(fd = qemu_open(src, flags, 0666));
+    if (fd == -1) {
+        error_setg(errp, "open %s: %s", src, strerror(errno));
+    }
+    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 */
+
 ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
                                Error **errp)
 {
@@ -2945,6 +3003,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     CharDriverState *chr = NULL;
 
     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;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c9ab37c..4d382f4 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2672,13 +2672,19 @@ Arguments:
 - "id": the chardev's ID, must be unique (json-string)
 - "backend": chardev backend type + parameters
 
-Example:
+Examples:
 
 -> { "execute" : "chardev-add",
      "arguments" : { "id" : "foo",
                      "backend" : { "type" : "null", "data" : {} } } }
 <- { "return": {} }
 
+-> { "execute" : "chardev-add",
+     "arguments" : { "id" : "bar",
+                     "backend" : { "type" : "file",
+                                   "data" : { "out" : "/tmp/bar.log" } } } }
+<- { "return": {} }
+
 EQMP
 
     {
-- 
1.7.1

[Qemu-devel] [PATCH v2 07/10] chardev: add serial chardev support to chardev-add (qmp)

[Gerd Hoffmann]

Simliar to file, except that no separate in/out files are supported
because it's pointless for direct device access.  Also the special
tty ioctl hooks (pass through linespeed settings etc) are activated
on Unix.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |   13 +++++++++++
 qemu-char.c      |   62 +++++++++++++++++++++++++++++++++++++++++++++++-------
 qemu-options.hx  |    9 +++----
 3 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 7930139..c328a0f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3029,6 +3029,18 @@
                                    'out' : 'str' } }
 
 ##
+# @ChardevPort:
+#
+# Configuration info for device chardevs.
+#
+# Since: 1.4
+##
+{ 'enum': 'ChardevPortKind', 'data': [ 'serial' ] }
+
+{ 'type': 'ChardevPort', 'data': { 'device' : 'str',
+                                   'type'   : 'ChardevPortKind'} }
+
+##
 # @ChardevBackend:
 #
 # Configuration info for the new chardev backend.
@@ -3038,6 +3050,7 @@
 { 'type': 'ChardevDummy', 'data': { } }
 
 { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
+                                       'port' : 'ChardevPort',
                                        'null' : 'ChardevDummy' } }
 
 ##
diff --git a/qemu-char.c b/qemu-char.c
index fa0a884..1b9ae2d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1230,21 +1230,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__ */
 
@@ -1666,9 +1672,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;
 
@@ -1687,6 +1692,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;
@@ -2765,6 +2775,7 @@ static const struct {
 #endif
 #ifdef HAVE_CHARDEV_TTY
     { .name = "tty",       .open = qemu_chr_open_tty },
+    { .name = "serial",    .open = qemu_chr_open_tty },
     { .name = "pty",       .open = qemu_chr_open_pty },
 #endif
 #ifdef HAVE_CHARDEV_PARPORT
@@ -2958,6 +2969,17 @@ 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) {
+    case CHARDEV_PORT_KIND_SERIAL:
+        return qemu_chr_open_win_path(port->device);
+    default:
+        error_setg(errp, "unknown chardev port (%d)", port->type);
+        return NULL;
+    }
+}
+
 #else /* WIN32 */
 
 static int qmp_chardev_open_file_source(char *src, int flags,
@@ -2994,6 +3016,27 @@ 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_SERIAL:
+        flags = O_RDWR;
+        fd = qmp_chardev_open_file_source(port->device, flags, errp);
+        if (error_is_set(errp)) {
+            return NULL;
+        }
+        socket_set_nonblock(fd);
+        return qemu_chr_open_tty_fd(fd);
+#endif
+    default:
+        error_setg(errp, "unknown chardev port (%d)", port->type);
+        return NULL;
+    }
+}
+
 #endif /* WIN32 */
 
 ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
@@ -3006,6 +3049,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     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;
diff --git a/qemu-options.hx b/qemu-options.hx
index 9df0cde..17cc1ad 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1742,6 +1742,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
 #endif
 #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \
         || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__)
+    "-chardev serial,id=id,path=path[,mux=on|off]\n"
     "-chardev tty,id=id,path=path[,mux=on|off]\n"
 #endif
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
@@ -1910,8 +1911,8 @@ take any options.
 
 Send traffic from the guest to a serial device on the host.
 
-@option{serial} is
-only available on Windows hosts.
+On Unix hosts serial will actually accept any tty device,
+not only serial lines.
 
 @option{path} specifies the name of the serial device to open.
 
@@ -1937,10 +1938,8 @@ Connect to a local BrlAPI server. @option{braille} does not take any options.
 
 @item -chardev tty ,id=@var{id} ,path=@var{path}
 
-Connect to a local tty device.
-
 @option{tty} is only available on Linux, Sun, FreeBSD, NetBSD, OpenBSD and
-DragonFlyBSD hosts.
+DragonFlyBSD hosts.  It is an alias for -serial.
 
 @option{path} specifies the path to the tty. @option{path} is required.
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 08/10] chardev: add parallel chardev support to chardev-add (qmp)

[Gerd Hoffmann]

Also alias the old parport name to parallel for -chardev.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    3 ++-
 qemu-char.c      |   44 ++++++++++++++++++++++++++++----------------
 qemu-options.hx  |    5 ++++-
 3 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index c328a0f..2b23c25 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3035,7 +3035,8 @@
 #
 # Since: 1.4
 ##
-{ 'enum': 'ChardevPortKind', 'data': [ 'serial' ] }
+{ 'enum': 'ChardevPortKind', 'data': [ 'serial',
+                                       'parallel' ] }
 
 { 'type': 'ChardevPort', 'data': { 'device' : 'str',
                                    'type'   : 'ChardevPortKind'} }
diff --git a/qemu-char.c b/qemu-char.c
index 1b9ae2d..1cf07be 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1367,17 +1367,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);
@@ -1441,16 +1434,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;
@@ -2750,6 +2736,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);
@@ -2779,6 +2781,7 @@ static const struct {
     { .name = "pty",       .open = qemu_chr_open_pty },
 #endif
 #ifdef HAVE_CHARDEV_PARPORT
+    { .name = "parallel",  .open = qemu_chr_open_pp },
     { .name = "parport",   .open = qemu_chr_open_pp },
 #endif
 #ifdef CONFIG_SPICE
@@ -3031,6 +3034,15 @@ static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
         socket_set_nonblock(fd);
         return qemu_chr_open_tty_fd(fd);
 #endif
+#ifdef HAVE_CHARDEV_PARPORT
+    case CHARDEV_PORT_KIND_PARALLEL:
+        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;
diff --git a/qemu-options.hx b/qemu-options.hx
index 17cc1ad..40cd683 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1746,6 +1746,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev,
     "-chardev tty,id=id,path=path[,mux=on|off]\n"
 #endif
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__)
+    "-chardev parallel,id=id,path=path[,mux=on|off]\n"
     "-chardev parport,id=id,path=path[,mux=on|off]\n"
 #endif
 #if defined(CONFIG_SPICE)
@@ -1776,6 +1777,7 @@ Backend is one of:
 @option{stdio},
 @option{braille},
 @option{tty},
+@option{parallel},
 @option{parport},
 @option{spicevmc}.
 @option{spiceport}.
@@ -1943,9 +1945,10 @@ DragonFlyBSD hosts.  It is an alias for -serial.
 
 @option{path} specifies the path to the tty. @option{path} is required.
 
+@item -chardev parallel ,id=@var{id} ,path=@var{path}
 @item -chardev parport ,id=@var{id} ,path=@var{path}
 
-@option{parport} is only available on Linux, FreeBSD and DragonFlyBSD hosts.
+@option{parallel} is only available on Linux, FreeBSD and DragonFlyBSD hosts.
 
 Connect to a local parallel port.
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 09/10] chardev: add socket chardev support to chardev-add (qmp)

[Gerd Hoffmann]

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 |   20 ++++++-
 qemu-char.c      |  168 ++++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 131 insertions(+), 57 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 2b23c25..a654096 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3042,6 +3042,19 @@
                                    'type'   : 'ChardevPortKind'} }
 
 ##
+# @ChardevSocket:
+#
+# Configuration info for socket chardevs.
+#
+# Since: 1.4
+##
+{ 'type': 'ChardevSocket', 'data': { 'addr'    : 'SocketAddress',
+                                     '*server' : 'bool',
+                                     '*wait'   : 'bool',
+                                     '*delay'  : 'bool',
+                                     '*telnet' : 'bool' } }
+
+##
 # @ChardevBackend:
 #
 # Configuration info for the new chardev backend.
@@ -3050,9 +3063,10 @@
 ##
 { 'type': 'ChardevDummy', 'data': { } }
 
-{ 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile',
-                                       'port' : 'ChardevPort',
-                                       'null' : 'ChardevDummy' } }
+{ 'union': 'ChardevBackend', 'data': { 'file'   : 'ChardevFile',
+                                       'port'   : 'ChardevPort',
+                                       'socket' : 'ChardevSocket',
+                                       'null'   : 'ChardevDummy' } }
 
 ##
 # @ChardevReturn:
diff --git a/qemu-char.c b/qemu-char.c
index 1cf07be..eb6b153 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2438,10 +2438,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;
@@ -2458,10 +2536,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 {
@@ -2481,56 +2556,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);
@@ -2539,8 +2572,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;
 }
 
@@ -3051,6 +3086,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);
+}
+
 ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
                                Error **errp)
 {
@@ -3064,6 +3121,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     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

[Qemu-devel] [PATCH v2 10/10] chardev: add pty chardev support to chardev-add (qmp)

[Gerd Hoffmann]

The ptsname is returned directly, so there is no need to
use query-chardev to figure the pty device path.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qapi-schema.json |    3 ++-
 qemu-char.c      |   13 +++++++++++++
 qmp-commands.hx  |    5 +++++
 3 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index a654096..23b6862 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3066,6 +3066,7 @@
 { 'union': 'ChardevBackend', 'data': { 'file'   : 'ChardevFile',
                                        'port'   : 'ChardevPort',
                                        'socket' : 'ChardevSocket',
+                                       'pty'    : 'ChardevDummy',
                                        'null'   : 'ChardevDummy' } }
 
 ##
@@ -3075,7 +3076,7 @@
 #
 # Since: 1.4
 ##
-{ 'type' : 'ChardevReturn', 'data': { } }
+{ 'type' : 'ChardevReturn', 'data': { '*pty' : 'str' } }
 
 ##
 # @chardev-add:
diff --git a/qemu-char.c b/qemu-char.c
index eb6b153..530d582 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3124,6 +3124,19 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
     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->pty = g_strdup(qemu_opt_get(opts, "path"));
+        ret->has_pty = true;
+        qemu_opts_del(opts);
+        break;
+    }
+#endif
     case CHARDEV_BACKEND_KIND_NULL:
         chr = qemu_chr_open_null(NULL);
         break;
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4d382f4..cbf1280 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2685,6 +2685,11 @@ Examples:
                                    "data" : { "out" : "/tmp/bar.log" } } } }
 <- { "return": {} }
 
+-> { "execute" : "chardev-add",
+     "arguments" : { "id" : "baz",
+                     "backend" : { "type" : "pty", "data" : {} } } }
+<- { "return": { "pty" : "/dev/pty/42" } }
+
 EQMP
 
     {
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2 01/10] chardev: add error reporting for qemu_chr_new_from_opts

[Eric Blake]

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

On 01/10/2013 07:22 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/char/char.h |    3 ++-
>  qemu-char.c         |   24 +++++++++++++++---------
>  vl.c                |    9 ++++++---
>  3 files changed, 23 insertions(+), 13 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: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 02/10] chardev: fix QemuOpts lifecycle

[Eric Blake]

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

On 01/10/2013 07:22 AM, Gerd Hoffmann wrote:
> 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>
> ---
>  include/char/char.h |    1 +
>  qemu-char.c         |   20 ++++++++++++++------
>  2 files changed, 15 insertions(+), 6 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: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 03/10] chardev: reduce chardev ifdef mess a bit

[Eric Blake]

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

On 01/10/2013 07:22 AM, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qemu-char.c |   22 +++++++++++-----------
>  1 files changed, 11 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: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 04/10] chardev: add qmp hotplug commands, with null chardev support

[Eric Blake]

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

On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> Add chardev-add and chardev-remove qmp commands.  Hotplugging
> a null chardev is supported for now, more will be added later.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---

> +
> +##
> +# @ChardevReturn:
> +#
> +# Return infos about the chardev backend just created.

s/infos/info/ or maybe s/infos/details/


> +ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> +                               Error **errp)
> +{
> +    ChardevReturn *ret = g_new0(ChardevReturn, 1);
> +    CharDriverState *chr = NULL;
> +

Should you add:

    chr = qemu_chr_find(id);
    if (chr) {
        error_setg(errp, "Chardev '%s' already exists", id);
        return NULL;
    }

> +    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);
> +        break;
> +    }
> +
> +    if (chr == NULL && !error_is_set(errp)) {
> +        error_setg(errp, "Failed to create chardev");
> +    }
> +    if (chr) {
> +        chr->label = g_strdup(id);
> +        chr->avail_connections = 1;
> +        QTAILQ_INSERT_TAIL(&chardevs, chr, next);
> +    }
> +    return ret;

Do you really want to be returning a successful object even when the
error is set, or should you start with ret = NULL, and only do the
g_new0(ChardevReturn, 1) on success?

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

Re: [Qemu-devel] [PATCH v2 05/10] chardev: add hmp hotplug commands

[Eric Blake]

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

On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> Add chardev-add and chardev-remove commands to the human monitor.
> chardev-add accepts the same syntax as -chardev, chardev-remove
> expects a chardev id.
> 
> 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(-)
> 

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

Re: [Qemu-devel] [PATCH v2 06/10] chardev: add file chardev support to chardev-add (qmp)

[Eric Blake]

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

On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> Add support for file chardevs.  Output file is mandatory,
> input file is optional.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi-schema.json |   13 ++++++++++-
>  qemu-char.c      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |    8 ++++++-
>  3 files changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 53d4b9e..7930139 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3019,6 +3019,16 @@
>  { 'command': 'nbd-server-stop' }
>  
>  ##
> +# @ChardevFile:

Should you mention '@in: #optional' and '@out:' in any further detail?

> +#
> +# Configuration info for file chardevs.
> +#
> +# Since: 1.4
> +##
> +{ 'type': 'ChardevFile', 'data': { '*in' : 'str',
> +                                   'out' : 'str' } }

Hmm; here you document ChardevFile as a separate type, but you didn't
document ChardevDummy in patch 4/10.


> +#ifdef _WIN32

> +
> +#else /* WIN32 */

Wouldn't this be /* !_WIN32 */?

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

Re: [Qemu-devel] [PATCH v2 07/10] chardev: add serial chardev support to chardev-add (qmp)

[Eric Blake]

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

On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> Simliar to file, except that no separate in/out files are supported

s/Simliar/Similar/

> because it's pointless for direct device access.  Also the special
> tty ioctl hooks (pass through linespeed settings etc) are activated
> on Unix.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi-schema.json |   13 +++++++++++
>  qemu-char.c      |   62 +++++++++++++++++++++++++++++++++++++++++++++++-------
>  qemu-options.hx  |    9 +++----
>  3 files changed, 71 insertions(+), 13 deletions(-)
> 

> +static CharDriverState *qmp_chardev_open_port(ChardevPort *port, Error **errp)
> +{
> +    int flags, fd;
> +
> +    switch (port->type) {
> +#ifdef HAVE_CHARDEV_TTY
> +    case CHARDEV_PORT_KIND_SERIAL:
> +        flags = O_RDWR;
> +        fd = qmp_chardev_open_file_source(port->device, flags, errp);
> +        if (error_is_set(errp)) {
> +            return NULL;
> +        }
> +        socket_set_nonblock(fd);

Can this fail?  And if so, should you react to failure?

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

Re: [Qemu-devel] [PATCH v2 08/10] chardev: add parallel chardev support to chardev-add (qmp)

[Eric Blake]

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

On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> Also alias the old parport name to parallel for -chardev.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi-schema.json |    3 ++-
>  qemu-char.c      |   44 ++++++++++++++++++++++++++++----------------
>  qemu-options.hx  |    5 ++++-
>  3 files changed, 34 insertions(+), 18 deletions(-)
> 

>  ##
> -{ 'enum': 'ChardevPortKind', 'data': [ 'serial' ] }
> +{ 'enum': 'ChardevPortKind', 'data': [ 'serial',
> +                                       'parallel' ] }
>  
>  { 'type': 'ChardevPort', 'data': { 'device' : 'str',
>                                     'type'   : 'ChardevPortKind'} }

In patch 7, should the enum and type have separate documentation?

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

Re: [Qemu-devel] [PATCH v2 09/10] chardev: add socket chardev support to chardev-add (qmp)

[Eric Blake]

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

On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> qemu_chr_open_socket is splitted into two functions.  All initialization

s/splitted/split/

> after creating the socket file handler is splitted away into the new

and again

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

> +# @ChardevSocket:
> +#
> +# Configuration info for socket chardevs.

Do you need any further doc details for individual fields?

> +#
> +# Since: 1.4
> +##
> +{ 'type': 'ChardevSocket', 'data': { 'addr'    : 'SocketAddress',
> +                                     '*server' : 'bool',
> +                                     '*wait'   : 'bool',
> +                                     '*delay'  : 'bool',
> +                                     '*telnet' : 'bool' } }
> +
>  
> -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)

You didn't pick up on my v1 comment of converting these parameters to bool.

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

Re: [Qemu-devel] [PATCH v2 10/10] chardev: add pty chardev support to chardev-add (qmp)

[Eric Blake]

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

On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
> The ptsname is returned directly, so there is no need to
> use query-chardev to figure the pty device path.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  qapi-schema.json |    3 ++-
>  qemu-char.c      |   13 +++++++++++++
>  qmp-commands.hx  |    5 +++++
>  3 files changed, 20 insertions(+), 1 deletions(-)

> +++ b/qmp-commands.hx
> @@ -2685,6 +2685,11 @@ Examples:
>                                     "data" : { "out" : "/tmp/bar.log" } } } }
>  <- { "return": {} }
>  
> +-> { "execute" : "chardev-add",
> +     "arguments" : { "id" : "baz",
> +                     "backend" : { "type" : "pty", "data" : {} } } }
> +<- { "return": { "pty" : "/dev/pty/42" } }

I like it.  The QMP API is looking saner now, although I still pointed
out enough questions that you might need a v3.

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

Re: [Qemu-devel] [PATCH v2 06/10] chardev: add file chardev support to chardev-add (qmp)

[Gerd Hoffmann]

>> ## +# @ChardevFile:
> 
> Should you mention '@in: #optional' and '@out:' in any further
> detail?

Done.

> Hmm; here you document ChardevFile as a separate type, but you
> didn't document ChardevDummy in patch 4/10.

As the name says it is just a dummy passed when the backend in
question doesn't need any parameters.  Therefore it is empty.  Does it
really need its own documentation stanza?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 07/10] chardev: add serial chardev support to chardev-add (qmp)

[Gerd Hoffmann]

On 01/10/13 20:39, Eric Blake wrote:
> On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
>> Simliar to file, except that no separate in/out files are
>> supported
> 
> s/Simliar/Similar/

Fixed.

>> +        socket_set_nonblock(fd);
> 
> Can this fail?

No.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 08/10] chardev: add parallel chardev support to chardev-add (qmp)

[Gerd Hoffmann]

On 01/10/13 20:41, Eric Blake wrote:
> On 01/10/2013 07:23 AM, Gerd Hoffmann wrote:
>> Also alias the old parport name to parallel for -chardev.
>> 
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- 
>> qapi-schema.json |    3 ++- qemu-char.c      |   44
>> ++++++++++++++++++++++++++++---------------- qemu-options.hx  |
>> 5 ++++- 3 files changed, 34 insertions(+), 18 deletions(-)
>> 
> 
>> ## -{ 'enum': 'ChardevPortKind', 'data': [ 'serial' ] } +{
>> 'enum': 'ChardevPortKind', 'data': [ 'serial', +
>> 'parallel' ] }
>> 
>> { 'type': 'ChardevPort', 'data': { 'device' : 'str', 'type'   :
>> 'ChardevPortKind'} }
> 
> In patch 7, should the enum and type have separate documentation?

IMHO no, they belong together as ChardevPortKind is only used by
ChardevPort.

cheers,
  Gerd