diff mbox

[v3,9/9] chardev: add hotplug support.

Message ID 1350288417-24350-10-git-send-email-kraxel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Oct. 15, 2012, 8:06 a.m. UTC
This patch adds chardev_add and chardev_del monitor commands.

They work simliar 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.

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

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
 hmp.c            |   23 ++++++++++++++++++++
 hmp.h            |    2 +
 qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
 qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
 qemu-char.h      |    1 +
 qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 202 insertions(+), 0 deletions(-)

Comments

Anthony Liguori Oct. 15, 2012, 2:22 p.m. UTC | #1
Gerd Hoffmann <kraxel@redhat.com> writes:

> This patch adds chardev_add and chardev_del monitor commands.
>
> They work simliar 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.
>
> chardev_del just takes an id argument and zaps the chardev specified.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
>  hmp.c            |   23 ++++++++++++++++++++
>  hmp.h            |    2 +
>  qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>  qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
>  qemu-char.h      |    1 +
>  qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 202 insertions(+), 0 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..48504d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1404,6 +1404,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,
> +    },

Should be chardev-add at this point.

> +
> +STEXI
> +@item chardev_add args
> +@findex chardev_add
> +
> +chardev_add accepts the same parameters as the -chardev command line switch.
> +
> +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..96bb900 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,26 @@ 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 *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_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..550e4c7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>  # 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

If we're documenting all of the options, then we probably should specify
them in the schema.  It's a little more cumbersome with QemuOpts, but it
makes the schema more correct wrt the implementation.

Regards,

Anthony Liguori

> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'chardev_add', 'data': {'id'      : 'str',
> +                                     'backend' : 'str',
> +                                     '*props'  : '**' },
> +  'gen': 'no' }
> +
> +##
> +# @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 be4ec61..dae3e3c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
>      return serial_hds[next_serial++];
>  }
>  
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    Error *err = NULL;
> +    QemuOptsList *opts_list;
> +    QemuOpts *opts;
> +
> +    opts_list = qemu_find_opts_err("chardev", &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    qemu_chr_new_from_opts(opts, NULL, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +    return 0;
> +
> +exit_err:
> +    qerror_report_err(err);
> +    error_free(err);
> +    return -1;
> +}
> +
> +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/qemu-char.h b/qemu-char.h
> index 99bc132..407e0fe 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -238,6 +238,7 @@ 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);
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
>  
>  /* 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..ebc1cee 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_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
> -- 
> 1.7.1
Eric Blake Oct. 15, 2012, 6:04 p.m. UTC | #2
On 10/15/2012 02:06 AM, Gerd Hoffmann wrote:
> This patch adds chardev_add and chardev_del monitor commands.
> 
> They work simliar to the netdev_{add,del} commands.  The hmp version of

s/simliar/similar/

> chardev_add accepts like the -chardev command line option does.  The qmp
> version expects the arguments being passed as named parameters.
> 
> chardev_del just takes an id argument and zaps the chardev specified.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'screendump', 'data': {'filename': 'str'} }
> +
> +##
> +# @chardev_add:

chardev-add

> +#
> +# Add a chardev
> +#
> +# @id: the chardev's ID, must be unique
> +# @backend: the chardev backend: "file", "socket", ...

Should this be an enum type, instead of an open-coded string?

> +# @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',
> +                                     '*props'  : '**' },

Having an open-coded list for props feels awkward; it would be nicer to
have the schema completely describe everything, even though that may be
more documentation work.

> +  'gen': 'no' }
> +
> +##
> +# @chardev_del:

chardev-del


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

Given this line...

> +- "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",

...this line is wrong, since "on" is not a json-bool.  It would have to
be "server":true

> +                                              "wait"    : "off" } }

Similar for "wait":false
Lei Li Oct. 16, 2012, 9:13 a.m. UTC | #3
On 10/15/2012 04:06 PM, Gerd Hoffmann wrote:
> This patch adds chardev_add and chardev_del monitor commands.
>
> They work simliar 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.
>
> chardev_del just takes an id argument and zaps the chardev specified.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
>   hmp.c            |   23 ++++++++++++++++++++
>   hmp.h            |    2 +
>   qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>   qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
>   qemu-char.h      |    1 +
>   qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 202 insertions(+), 0 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..48504d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1404,6 +1404,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_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..96bb900 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,26 @@ 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 *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_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..550e4c7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>   # 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',
> +                                     '*props'  : '**' },
> +  'gen': 'no' }
> +
> +##
> +# @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 be4ec61..dae3e3c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
>       return serial_hds[next_serial++];
>   }
>
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    Error *err = NULL;
> +    QemuOptsList *opts_list;
> +    QemuOpts *opts;
> +
> +    opts_list = qemu_find_opts_err("chardev", &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    qemu_chr_new_from_opts(opts, NULL, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +    return 0;
> +
> +exit_err:
> +    qerror_report_err(err);
> +    error_free(err);
> +    return -1;
> +}
> +
> +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/qemu-char.h b/qemu-char.h
> index 99bc132..407e0fe 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -238,6 +238,7 @@ 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);
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
>
>   /* 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..ebc1cee 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_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)
> +

As mentioned in another thread, should it support the "mux" argument
for multiplexing mode?

> +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
Luiz Capitulino Oct. 17, 2012, 1:28 a.m. UTC | #4
On Mon, 15 Oct 2012 10:06:57 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> This patch adds chardev_add and chardev_del monitor commands.
> 
> They work simliar 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.
> 
> chardev_del just takes an id argument and zaps the chardev specified.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hmp-commands.hx  |   32 ++++++++++++++++++++++++++++
>  hmp.c            |   23 ++++++++++++++++++++
>  hmp.h            |    2 +
>  qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>  qemu-char.c      |   44 ++++++++++++++++++++++++++++++++++++++
>  qemu-char.h      |    1 +
>  qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 202 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..48504d1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1404,6 +1404,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_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..96bb900 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,26 @@ 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 *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");

You can monitor_printf() directly if you want.

> +    } else {
> +        qemu_chr_new_from_opts(opts, NULL, &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..550e4c7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2796,3 +2796,42 @@
>  # 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", ...

I agree with Eric's comment that this should be an enum.

> +# @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',
> +                                     '*props'  : '**' },
> +  'gen': 'no' }
> +
> +##
> +# @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 be4ec61..dae3e3c 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
>      return serial_hds[next_serial++];
>  }
>  
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    Error *err = NULL;
> +    QemuOptsList *opts_list;
> +    QemuOpts *opts;
> +
> +    opts_list = qemu_find_opts_err("chardev", &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +
> +    qemu_chr_new_from_opts(opts, NULL, &err);
> +    if (error_is_set(&err)) {
> +        goto exit_err;
> +    }
> +    return 0;
> +
> +exit_err:
> +    qerror_report_err(err);
> +    error_free(err);
> +    return -1;
> +}
> +
> +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/qemu-char.h b/qemu-char.h
> index 99bc132..407e0fe 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -238,6 +238,7 @@ 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);
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
>  
>  /* 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..ebc1cee 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_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
Luiz Capitulino Oct. 17, 2012, 1:49 a.m. UTC | #5
On Mon, 15 Oct 2012 12:04:52 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 10/15/2012 02:06 AM, Gerd Hoffmann wrote:
> > This patch adds chardev_add and chardev_del monitor commands.
> > 
> > They work simliar to the netdev_{add,del} commands.  The hmp version of
> 
> s/simliar/similar/
> 
> > chardev_add accepts like the -chardev command line option does.  The qmp
> > version expects the arguments being passed as named parameters.
> > 
> > chardev_del just takes an id argument and zaps the chardev specified.
> > 
> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> > ---
> > +++ b/qapi-schema.json
> > @@ -2796,3 +2796,42 @@
> >  # Since: 0.14.0
> >  ##
> >  { 'command': 'screendump', 'data': {'filename': 'str'} }
> > +
> > +##
> > +# @chardev_add:
> 
> chardev-add
> 
> > +#
> > +# Add a chardev
> > +#
> > +# @id: the chardev's ID, must be unique
> > +# @backend: the chardev backend: "file", "socket", ...
> 
> Should this be an enum type, instead of an open-coded string?
> 
> > +# @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',
> > +                                     '*props'  : '**' },
> 
> Having an open-coded list for props feels awkward; it would be nicer to
> have the schema completely describe everything, even though that may be
> more documentation work.

I agree it's awkward. Besides, this 'props' thing was supposed to be only used
with old commands that already accept QemuOpts style options. The usage of
'props' forces us having a front-end in old qmp style, which we don't want to
get spread.

Having said that, I'm not sure I can offer a good alternative here. Having
all options in the schema would require us to create the QemuOpts object by
hand vs. automatically building it from the qdict that's passed to old
style qmp commands. It's not only documentation work.

There's the work Laszlo did for net clients too. Where we have a
NetClientOptions type which is a union of all possible net clients
options types. All clients get a NetClientOptions instance. But this would
require massive conversion of chardev backends to use a similar type.

Another idea would be to have a different add command for each backend.
This is more work and more "verbose", but has the good effect of a clean
separation & definition of the set of options used by each backend.

But again, as I'm not sure any of my ideas above are good, I'd be fine
with the chardev_add command from this patch.

> 
> > +  'gen': 'no' }
> > +
> > +##
> > +# @chardev_del:
> 
> chardev-del
> 
> 
> > +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)
> 
> Given this line...
> 
> > +- "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",
> 
> ...this line is wrong, since "on" is not a json-bool.  It would have to
> be "server":true
> 
> > +                                              "wait"    : "off" } }
> 
> Similar for "wait":false
>
Gerd Hoffmann Oct. 17, 2012, 9:27 a.m. UTC | #6
Hi,

>> +- "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)
>> +
> 
> As mentioned in another thread, should it support the "mux" argument
> for multiplexing mode?

I don't think so.  Basically the only use case for the mux chardev is
-nographic with serial line + monitor being multiplexed on stdio.

cheers,
  Gerd
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..48504d1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,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_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..96bb900 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,26 @@  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 *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_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..550e4c7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,42 @@ 
 # 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',
+                                     '*props'  : '**' },
+  'gen': 'no' }
+
+##
+# @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 be4ec61..dae3e3c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2911,3 +2911,47 @@  CharDriverState *qemu_char_get_next_serial(void)
     return serial_hds[next_serial++];
 }
 
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+    Error *err = NULL;
+    QemuOptsList *opts_list;
+    QemuOpts *opts;
+
+    opts_list = qemu_find_opts_err("chardev", &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+
+    opts = qemu_opts_from_qdict(opts_list, qdict, &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+
+    qemu_chr_new_from_opts(opts, NULL, &err);
+    if (error_is_set(&err)) {
+        goto exit_err;
+    }
+    return 0;
+
+exit_err:
+    qerror_report_err(err);
+    error_free(err);
+    return -1;
+}
+
+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/qemu-char.h b/qemu-char.h
index 99bc132..407e0fe 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -238,6 +238,7 @@  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);
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret);
 
 /* 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..ebc1cee 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_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