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

[Paolo Bonzini]

Il 14/12/2012 10:38, Gerd Hoffmann ha scritto:
> 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.

I'd rather avoid introducing this interface.  Using multiple commands is
different from all previous examples, both HMP and QMP (including recent
ones such as the NBD server).  It is also hard to extend, for example
file descriptor passing is hard to retrofit.

Perhaps you can define a QAPI union and slowly build it up?  Something
that ultimately can become this:

{ 'enum': 'ChardevFileMode', 'data':
  # pty = console under Windows
  # serial = tty under POSIX
  [ 'file', 'pipe', 'parport', 'pty', 'serial' ] }

{ 'enum: 'ChardevFileSource', 'data':
  [ 'path', 'fd' ] }

{ 'type': 'ChardevFile',
  'data': {'source': 'string', 'source-type': 'ChardevFileSource',
           'mode': 'ChardevFileMode'}}

{ 'type': 'ChardevVC',
  'data': {'width': 'int', 'height': 'int', '*characters': 'bool'}}

{ 'type': 'ChardevSocket',
  'data': {'addr': 'SocketAddress', '*server': 'bool',
           '*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} }

# For future extensibility...
{ 'ChardevDummy', 'data': {} }

{ 'union': 'ChardevBackend', 'data': {
  'socket': 'ChardevSocket',
  'udp': 'UDPSocketAddress',
  'file': 'ChardevFile',
  'null': 'ChardevDummy',
  'msmouse': 'ChardevDummy',
  'braille': 'ChardevDummy',
  'stdio': 'ChardevDummy',
  'vc': 'ChardevVC',

  # Solely for HMP usage.
  'legacy': 'str'
}

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

A simple conversion from this to QemuOpts should be easy, later the
backends can move to taking a ChardevBackend *.

Paolo

> chardev_del just takes an id argument and zaps the chardev specified.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  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 010b8c9..82a855a 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1485,6 +1485,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
> address@hidden chardev_add_file id path
> address@hidden 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
> address@hidden chardev_add_tty id path
> address@hidden chardev_add_tty
> +
> +ETEXI
> +
> +    {
> +        .name       = "chardev_remove",
> +        .args_type  = "id:s",
> +        .params     = "id",
> +        .help       = "remove chardev",
> +        .mhandler.cmd = hmp_chardev_remove,
> +    },
> +
> +STEXI
> address@hidden chardev_remove id
> address@hidden chardev_remove
> +
> +Removes the chardev @var{id}.
> +
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index 180ba2b..8780e7c 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1335,3 +1335,32 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict 
> *qdict)
>      qmp_nbd_server_stop(&errp);
>      hmp_handle_error(mon, &errp);
>  }
> +
> +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 0ab03be..8cd50d1 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -80,5 +80,8 @@ 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_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 5dfa052..34c0e58 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3017,3 +3017,46 @@
>  # Since: 1.3.0
>  ##
>  { 'command': 'nbd-server-stop' }
> +
> +##
> +# @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 876714f..169743b 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2922,3 +2922,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 f984071..18b9e99 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -239,6 +239,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 5c692d0..5ff8cd1 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2654,3 +2654,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
>