Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

[Gerd Hoffmann <kraxel@redhat.com>]

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

On 10/17/12 12:09, Gerd Hoffmann wrote:
> This patch adds chardev_add and chardev_remove monitor commands.
> 
> They work similar to the netdev_{add,del} commands.  The hmp version of
> chardev_add accepts like the -chardev command line option does.  The qmp
> version expects the arguments being passed as named parameters.

Trying another approach, see attached patch.  This adds backend-specific
qemu commands to add chardevs, with just the parameters needed for the
specific backend.  Starting with file and tty, both accepting a path.
'file' is nice for testing, 'tty' very useful when hotplugging serial
devices on the host.  Others can be added as needed.  We probably don't
need all of them.  For example hotplugging the 'stdio' chardev doesn't
make much sense.

Advantage #1: Cleaner API.
Advantage #2: No legacy syntax headache when qomifying chardevs.

Comments?

cheers,
  Gerd

[-- Attachment #2: 0001-chardev-add-hotplug-support.patch --]
[-- Type: text/plain, Size: 8216 bytes --]

From 7f80af55530c9a448e4aed5a41c76a8e9514a27c Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 11 Oct 2012 14:53:00 +0200
Subject: [PATCH] chardev: add hotplug support.

This patch adds chardev_add_file, chardev_add_tty and chardev_remove
monitor commands.

chardev_add_file and chardev_add_tty expect an id and a path, they
create a file/tty chardev.

chardev_del just takes an id argument and zaps the chardev specified.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx  |   44 +++++++++++++++++++++++++++++++
 hmp.c            |   29 ++++++++++++++++++++
 hmp.h            |    3 ++
 qapi-schema.json |   43 ++++++++++++++++++++++++++++++
 qemu-char.c      |   41 +++++++++++++++++++++++++++++
 qemu-char.h      |    2 +
 qmp-commands.hx  |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 238 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..ecfc497 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,50 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
     {
+        .name       = "chardev_add_file",
+        .args_type  = "id:s,path:s",
+        .params     = "id path ",
+        .help       = "add file chardev",
+        .mhandler.cmd = hmp_chardev_add_file,
+    },
+
+STEXI
+@item chardev_add_file id path
+@findex chardev_add_file
+
+ETEXI
+
+    {
+        .name       = "chardev_add_tty",
+        .args_type  = "id:s,path:s",
+        .params     = "id path ",
+        .help       = "add tty chardev",
+        .mhandler.cmd = hmp_chardev_add_tty,
+    },
+
+STEXI
+@item chardev_add_tty id path
+@findex chardev_add_tty
+
+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 70bdec2..346dd29 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,32 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+static void hmp_chardev_add_path(Monitor *mon, const QDict *qdict,
+                                 const char *backend)
+{
+    const char *id   = qdict_get_str(qdict, "args");
+    const char *path = qdict_get_str(qdict, "path");
+    Error *local_err = NULL;
+
+    qmp_chardev_add_path(id, path, backend, &local_err);
+    hmp_handle_error(mon, &local_err);
+}
+
+void hmp_chardev_add_file(Monitor *mon, const QDict *qdict)
+{
+    hmp_chardev_add_path(mon, qdict, "file");
+}
+
+void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict)
+{
+    hmp_chardev_add_path(mon, qdict, "tty");
+}
+
+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 71ea384..107941d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,8 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add_file(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..76e765b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,46 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev-add-file:
+#
+# Add a file chardev
+#
+# @id: the chardev's ID, must be unique
+# @path: file path
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-add-file', 'data': {'id'   : 'str',
+                                          'path' : 'str' } }
+
+##
+# @chardev-add-tty:
+#
+# Add a terminal chardev
+#
+# @id: the chardev's ID, must be unique
+# @path: device path
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-add-tty', 'data': {'id'   : 'str',
+                                         'path' : 'str' } }
+
+##
+# @chardev-remove:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index be4ec61..76c3396 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2911,3 +2911,44 @@ CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+void qmp_chardev_add_path(const char *id, const char *path,
+                          const char *backend, Error **errp)
+{
+    QemuOpts *opts;
+
+    opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    qemu_opt_set(opts, "path", path);
+    qemu_opt_set(opts, "backend", backend);
+    qemu_chr_new_from_opts(opts, NULL, errp);
+}
+
+void qmp_chardev_add_file(const char *id, const char *path, Error **errp)
+{
+    qmp_chardev_add_path(id, path, "file", errp);
+}
+
+void qmp_chardev_add_tty(const char *id, const char *path, Error **errp)
+{
+    qmp_chardev_add_path(id, path, "tty", errp);
+}
+
+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\n", id);
+        return;
+    }
+    if (chr->chr_can_read || chr->chr_read ||
+        chr->chr_event || chr->handler_opaque) {
+        error_setg(errp, "Chardev '%s' is busy\n", id);
+        return;
+    }
+    qemu_chr_delete(chr);
+}
diff --git a/qemu-char.h b/qemu-char.h
index 99bc132..ffe4c79 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -238,6 +238,8 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data);
 CharDriverState *qemu_chr_find(const char *name);
 
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
+void qmp_chardev_add_path(const char *id, const char *path,
+                          const char *backend, Error **errp);
 
 /* add an eventfd to the qemu devices that are polled */
 CharDriverState *qemu_chr_open_eventfd(int eventfd);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..73c02ed 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2576,3 +2576,79 @@ EQMP
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_target,
     },
+
+    {
+        .name       = "chardev-add-file",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_add_file,
+    },
+
+SQMP
+chardev-add-file
+----------------
+
+Add a file chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must be unique (json-string)
+- "path": file path (json-string)
+
+Example:
+
+-> { "execute"   : "chardev-add-file",
+     "arguments" : { "id"   : "foo",
+                     "path" : "/tmp/foo" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "chardev-add-tty",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_add_tty,
+    },
+
+SQMP
+chardev-add-tty
+---------------
+
+Add a terminal chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must be unique (json-string)
+- "path": device path (json-string)
+
+Example:
+
+-> { "execute"   : "chardev-add-tty",
+     "arguments" : { "id"   : "serial",
+                     "path" : "/dev/ttyS0" } }
+<- { "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