Patchwork [7/7] chardev: add hotplug support.

login
register
mail settings
Submitter Gerd Hoffmann
Date Oct. 12, 2012, 9:26 a.m.
Message ID <1350033962-16665-8-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/191080/
State New
Headers show

Comments

Gerd Hoffmann - Oct. 12, 2012, 9:26 a.m.
This patch adds chardev_add and chardev_del monitor commands.

chardev_del is pretty straight forward, it just takes an id argument and
zaps the chardev specified.

chardev_add is more tricky as there are tons of arguments for the
different backends.  The hmp version limited to the most common use
cases, especially when it comes to sockets:  You can only specify port
(tcp) or path (unix) and qemu will create a listening socket.  For
example this ...

   (qemu) chardev_add foo socket 42

... will do the same as ...

   -chardev socket,id=foo,port=42,server,nowait

on the qemu command line.

The qmp version has full support for everything the -chardev command
line switch can handle.  The implementation is pretty straight
forward: It just puts all arguments it got into a QemuOpts, then goes
call qemu_chr_new_from_opts().

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx  |   49 +++++++++++++++++++++++++++++++++++
 hmp.c            |   39 ++++++++++++++++++++++++++++
 hmp.h            |    2 +
 qapi-schema.json |   46 +++++++++++++++++++++++++++++++++
 qemu-char.c      |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 271 insertions(+), 0 deletions(-)
Paolo Bonzini - Oct. 12, 2012, 9:40 a.m.
Il 12/10/2012 11:26, Gerd Hoffmann ha scritto:
> This patch adds chardev_add and chardev_del monitor commands.
> 
> chardev_del is pretty straight forward, it just takes an id argument and
> zaps the chardev specified.
> 
> chardev_add is more tricky as there are tons of arguments for the
> different backends.  The hmp version limited to the most common use
> cases, especially when it comes to sockets:  You can only specify port
> (tcp) or path (unix) and qemu will create a listening socket.  For
> example this ...
> 
>    (qemu) chardev_add foo socket 42
> 
> ... will do the same as ...
> 
>    -chardev socket,id=foo,port=42,server,nowait

Why not

chardev_add socket,id=foo,port=42,server,nowait

?

> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     'path'    : 'str',
> +                                     'name'    : 'str',
> +                                     'host'    : 'str',
> +                                     'port'    : 'str',

You cannot pass NULLs via QMP, so these need to be optional.

I suggest that you implement the commands in a similar way as netdev_add.

Paolo

> +                                     'server'  : 'bool',
> +                                     'wait'    : 'bool',
> +                                     'ipv4'    : 'bool',
> +                                     'ipv6'    : 'bool',
> +                                     'telnet'  : 'bool' } }
> +
> +##
> +# @chardev_del:
> +#
> +# 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_del', 'data': {'id': 'str'} }
> diff --git a/qemu-char.c b/qemu-char.c
> index b082bae..2f9d860 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2900,3 +2900,77 @@ CharDriverState *qemu_char_get_next_serial(void)
>      return serial_hds[next_serial++];
>  }
>  
> +void qmp_chardev_add(const char *id, const char *backend,
> +                     const char *path, const char *name,
> +                     const char *host, const char *port,
> +                     bool server, bool wait,
> +                     bool ipv4, bool ipv6,
> +                     bool telnet, Error **errp)
> +{
> +    CharDriverState *chr;
> +    QemuOpts *opts;
> +
> +    chr = qemu_chr_find(id);
> +    if (NULL != chr) {
> +        error_setg(errp, "Chardev id '%s' exists already\n", id);
> +        return;
> +    }
> +
> +    opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
> +    qemu_opt_set(opts, "backend", backend);
> +    if (path) {
> +        qemu_opt_set(opts, "path", path);
> +    }
> +    if (name) {
> +        qemu_opt_set(opts, "name", name);
> +    }
> +    if (host) {
> +        qemu_opt_set(opts, "host", host);
> +    }
> +    if (port) {
> +        qemu_opt_set(opts, "port", port);
> +    }
> +    if (server) {
> +        qemu_opt_set(opts, "server", "on");
> +    }
> +    if (!wait) {
> +        qemu_opt_set(opts, "wait", "off");
> +    }
> +    if (ipv4) {
> +        qemu_opt_set(opts, "ipv4", "on");
> +    }
> +    if (ipv6) {
> +        qemu_opt_set(opts, "ipv6", "on");
> +    }
> +    if (telnet) {
> +        qemu_opt_set(opts, "telnet", "on");
> +    }
> +
> +    chr = qemu_chr_new_from_opts(opts, NULL);
> +    qemu_opts_del(opts);
> +
> +    if (chr == NULL) {
> +        error_setg(errp, "Creating chardev failed\n");
> +        return;
> +    }
> +}
> +
> +void qmp_chardev_del(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/qmp-commands.hx b/qmp-commands.hx
> index 2f8477e..b904df2 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2576,3 +2576,64 @@ 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": the chardev backend: "file", "socket", ... (json-string)
> +- "path": file / device / unix socket path (json-string, optional)
> +- "name": spice channel name (json-string, optional)
> +- "host": host name (json-string, optional)
> +- "port": port number (json-string, optional)
> +- "server": create socket in server mode (json-bool, optional)
> +- "wait": wait for connect (json-bool, optional)
> +- "ipv4": force ipv4-only (json-bool, optional)
> +- "ipv6": force ipv6-only (json-bool, optional)
> +- "telnet": telnet negotiation (json-bool, optional)
> +
> +Example:
> +
> +-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
> +                                              "backend" : "socket",
> +                                              "path"    : "/tmp/foo",
> +                                              "server"  : "on",
> +                                              "wait"    : "off" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
> +        .name       = "chardev_del",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
> +    },
> +
> +
> +SQMP
> +chardev_del
> +-----------
> +
> +Remove a chardev.
> +
> +Arguments:
> +
> +- "id": the chardev's ID, must exist and not be in use (json-string)
> +
> +Example:
> +
> +-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
> +<- { "return": {} }
> +
> +EQMP
>
Gerd Hoffmann - Oct. 12, 2012, 10:23 a.m.
On 10/12/12 11:40, Paolo Bonzini wrote:
> Il 12/10/2012 11:26, Gerd Hoffmann ha scritto:
>> This patch adds chardev_add and chardev_del monitor commands.
>>
>> chardev_del is pretty straight forward, it just takes an id argument and
>> zaps the chardev specified.
>>
>> chardev_add is more tricky as there are tons of arguments for the
>> different backends.  The hmp version limited to the most common use
>> cases, especially when it comes to sockets:  You can only specify port
>> (tcp) or path (unix) and qemu will create a listening socket.  For
>> example this ...
>>
>>    (qemu) chardev_add foo socket 42
>>
>> ... will do the same as ...
>>
>>    -chardev socket,id=foo,port=42,server,nowait
> 
> Why not
> 
> chardev_add socket,id=foo,port=42,server,nowait
> 
> ?

Yea, maybe, but see below.

>> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
>> +                                     'backend' : 'str',
>> +                                     'path'    : 'str',
>> +                                     'name'    : 'str',
>> +                                     'host'    : 'str',
>> +                                     'port'    : 'str',
> 
> You cannot pass NULLs via QMP, so these need to be optional.

Fixed.

> I suggest that you implement the commands in a similar way as netdev_add.

Why?  Isn't the whole point of using josn is that you'll get the stuff
from the josn parser & marshaller in a usable form instead of having it
to feed into yet another parser?  I think the only reason netdev_add
exists in the current form is that it predates qmp.

cheers,
  Gerd
Paolo Bonzini - Oct. 12, 2012, 10:50 a.m.
Il 12/10/2012 12:23, Gerd Hoffmann ha scritto:
>> I suggest that you implement the commands in a similar way as netdev_add.
> 
> Why?  Isn't the whole point of using josn is that you'll get the stuff
> from the josn parser & marshaller in a usable form instead of having it
> to feed into yet another parser?  I think the only reason netdev_add
> exists in the current form is that it predates qmp.

In principle you're right, but I think it's ugly that adding another
chardev argument needs changes in 3 places instead of just one.  (And
I'd like to add another argument soon enough...).

Paolo
Gerd Hoffmann - Oct. 12, 2012, 11:15 a.m.
On 10/12/12 12:50, Paolo Bonzini wrote:
> Il 12/10/2012 12:23, Gerd Hoffmann ha scritto:
>>> I suggest that you implement the commands in a similar way as netdev_add.
>>
>> Why?  Isn't the whole point of using josn is that you'll get the stuff
>> from the josn parser & marshaller in a usable form instead of having it
>> to feed into yet another parser?  I think the only reason netdev_add
>> exists in the current form is that it predates qmp.
> 
> In principle you're right, but I think it's ugly that adding another
> chardev argument needs changes in 3 places instead of just one.

Hmm, I don't have to use the generated marshaller, right?  With direct
access to the QDict I could just transform it into a QemuOpts.  A new
parameter wouldn't need code changes then.  And the code would be
reusable and probably also be simpler.  The qapi schema still needs an
update though.

HMP is more tricky, but I think we should sort QMP first.

cheers,
  Gerd
Paolo Bonzini - Oct. 12, 2012, 11:17 a.m.
Il 12/10/2012 13:15, Gerd Hoffmann ha scritto:
>> > In principle you're right, but I think it's ugly that adding another
>> > chardev argument needs changes in 3 places instead of just one.
> Hmm, I don't have to use the generated marshaller, right?  With direct
> access to the QDict I could just transform it into a QemuOpts.

That's exactly what I was suggesting. :P

> A new parameter wouldn't need code changes then.  And the code would be 
> reusable and probably also be simpler.  The qapi schema still needs
> an update though.

The QAPI schema is only used by the generated marshaller.

> HMP is more tricky, but I think we should sort QMP first.

HMP can just take a string, parse it into QemuOpts, and call a small
wrapper that calls qemu_chr_new_from_opts and returns errors via Error.

Paolo
Gerd Hoffmann - Oct. 12, 2012, 12:37 p.m.
On 10/12/12 13:17, Paolo Bonzini wrote:
> Il 12/10/2012 13:15, Gerd Hoffmann ha scritto:
>>>> In principle you're right, but I think it's ugly that adding another
>>>> chardev argument needs changes in 3 places instead of just one.
>> Hmm, I don't have to use the generated marshaller, right?  With direct
>> access to the QDict I could just transform it into a QemuOpts.
> 
> That's exactly what I was suggesting. :P

Ah, ok.  I actually looked at netdev_add but obviously not close
enougth.  On a quick glance it looked to me like @params is a single
string, not a varargs-style construct.  Guess we are on the same page then.

cheers,
  Gerd

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..e5590f4 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,55 @@  passed since 1970, i.e. unix epoch.
 ETEXI
 
     {
+        .name       = "chardev_add",
+        .args_type  = "id:s,backend:s,arg:s?",
+        .params     = "id backend [ name | port | path ]",
+        .help       = "add chardev",
+        .mhandler.cmd = hmp_chardev_add,
+    },
+
+STEXI
+@item chardev_add id backend [ name | port | path ]
+@findex chardev_add
+
+Create chardev with the specified @var{id} and @var{backend}.  The hmp
+version is limited to a commonly used subset, if you need more control
+use qmp instead.
+
+If @var{backend} is 'spicevmc' @var{arg} is assumed to be the spice
+channel name.
+
+If @var{backend} is 'udp' @var{arg} is assumed to be a port number.
+
+If @var{backend} is 'socket' and @var{arg} starts with a digit the
+argument is assumed to be a port number and a tcp socket is created
+(in server mode).
+
+If @var{backend} is 'socket' and @var{arg} doesn't start with a digit
+@var{arg} is assumed to be a path and a unix socket is created (in
+server mode).
+
+In all other cases @var{arg} is assumed to be a path.
+
+ETEXI
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "id:s",
+        .params     = "id",
+        .help       = "del chardev",
+        .mhandler.cmd = hmp_chardev_del,
+    },
+
+STEXI
+@item chardev_del id
+@findex chardev_del
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 70bdec2..e84d7f8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,42 @@  void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+    const char *backend = qdict_get_str(qdict, "backend");
+    const char *arg = qdict_get_str(qdict, "arg");
+    const char *id = qdict_get_str(qdict, "id");
+    const char *path = NULL, *name = NULL, *port = NULL, *host = NULL;
+    Error *err = NULL;
+
+    if (arg) {
+        if (strcmp(backend, "spicevmc") == 0) {
+            name = arg;
+        } else if (strcmp(backend, "udp") == 0) {
+            port = arg;
+        } else if (strcmp(backend, "socket") == 0 && isdigit(arg[0])) {
+            port = arg;
+        } else {
+            path = arg;
+        }
+    }
+
+    qmp_chardev_add(id, backend,
+                    path, name, host, port,
+                    true,  /* server      */
+                    false, /* wait        */
+                    false, /* ipv4 (only) */
+                    false, /* ipv6 (only) */
+                    false, /* telnet      */
+                    &err);
+    hmp_handle_error(mon, &err);
+}
+
+void hmp_chardev_del(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    qmp_chardev_del(qdict_get_str(qdict, "id"),
+                    &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 71ea384..080afaa 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,7 @@  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(Monitor *mon, const QDict *qdict);
+void hmp_chardev_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..98d92ad 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,49 @@ 
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev_add:
+#
+# Add a chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: the chardev backend: "file", "socket", ...
+# @path: file / device / unix socket path
+# @name: spice channel name
+# @host: host name
+# @port: port number
+# @server: create socket in server mode
+# @wait: wait for connect
+# @ipv4: force ipv4-only
+# @ipv6: force ipv6-only
+# @telnet: telnet negotiation
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev_add', 'data': {'id'      : 'str',
+                                     'backend' : 'str',
+                                     'path'    : 'str',
+                                     'name'    : 'str',
+                                     'host'    : 'str',
+                                     'port'    : 'str',
+                                     'server'  : 'bool',
+                                     'wait'    : 'bool',
+                                     'ipv4'    : 'bool',
+                                     'ipv6'    : 'bool',
+                                     'telnet'  : 'bool' } }
+
+##
+# @chardev_del:
+#
+# 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_del', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index b082bae..2f9d860 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2900,3 +2900,77 @@  CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+void qmp_chardev_add(const char *id, const char *backend,
+                     const char *path, const char *name,
+                     const char *host, const char *port,
+                     bool server, bool wait,
+                     bool ipv4, bool ipv6,
+                     bool telnet, Error **errp)
+{
+    CharDriverState *chr;
+    QemuOpts *opts;
+
+    chr = qemu_chr_find(id);
+    if (NULL != chr) {
+        error_setg(errp, "Chardev id '%s' exists already\n", id);
+        return;
+    }
+
+    opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+    qemu_opt_set(opts, "backend", backend);
+    if (path) {
+        qemu_opt_set(opts, "path", path);
+    }
+    if (name) {
+        qemu_opt_set(opts, "name", name);
+    }
+    if (host) {
+        qemu_opt_set(opts, "host", host);
+    }
+    if (port) {
+        qemu_opt_set(opts, "port", port);
+    }
+    if (server) {
+        qemu_opt_set(opts, "server", "on");
+    }
+    if (!wait) {
+        qemu_opt_set(opts, "wait", "off");
+    }
+    if (ipv4) {
+        qemu_opt_set(opts, "ipv4", "on");
+    }
+    if (ipv6) {
+        qemu_opt_set(opts, "ipv6", "on");
+    }
+    if (telnet) {
+        qemu_opt_set(opts, "telnet", "on");
+    }
+
+    chr = qemu_chr_new_from_opts(opts, NULL);
+    qemu_opts_del(opts);
+
+    if (chr == NULL) {
+        error_setg(errp, "Creating chardev failed\n");
+        return;
+    }
+}
+
+void qmp_chardev_del(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/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..b904df2 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2576,3 +2576,64 @@  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": the chardev backend: "file", "socket", ... (json-string)
+- "path": file / device / unix socket path (json-string, optional)
+- "name": spice channel name (json-string, optional)
+- "host": host name (json-string, optional)
+- "port": port number (json-string, optional)
+- "server": create socket in server mode (json-bool, optional)
+- "wait": wait for connect (json-bool, optional)
+- "ipv4": force ipv4-only (json-bool, optional)
+- "ipv6": force ipv6-only (json-bool, optional)
+- "telnet": telnet negotiation (json-bool, optional)
+
+Example:
+
+-> { "execute": "chardev_add", "arguments": { "id"      : "foo",
+                                              "backend" : "socket",
+                                              "path"    : "/tmp/foo",
+                                              "server"  : "on",
+                                              "wait"    : "off" } }
+<- { "return": {} }
+
+EQMP
+
+    {
+        .name       = "chardev_del",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_chardev_del,
+    },
+
+
+SQMP
+chardev_del
+-----------
+
+Remove a chardev.
+
+Arguments:
+
+- "id": the chardev's ID, must exist and not be in use (json-string)
+
+Example:
+
+-> { "execute": "chardev_del", "arguments": { "id" : "foo" } }
+<- { "return": {} }
+
+EQMP