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

[Lei Li <lilei@linux.vnet.ibm.com>]

On 10/12/2012 08:39 PM, Gerd Hoffmann wrote:
> 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  |   32 ++++++++++++++++++++++++++++
>   hmp.c            |   31 +++++++++++++++++++++++++++
>   hmp.h            |    2 +
>   qapi-schema.json |   39 ++++++++++++++++++++++++++++++++++
>   qemu-char.c      |   50 +++++++++++++++++++++++++++++++++++++++++++-
>   qemu-char.h      |    2 +
>   qmp-commands.hx  |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   7 files changed, 216 insertions(+), 1 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..b494d05 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1209,3 +1209,34 @@ 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");
> +    CharDriverState *chr;
> +    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");
> +        goto out;
> +    }
> +
> +    chr = qemu_chr_new_from_opts(opts, NULL);
> +    if (chr == NULL) {
> +        qemu_opts_del(opts);
> +        error_setg(&err, "Creating chardev failed\n");
> +    }
> +
> +out:
> +    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 b082bae..7bbc490 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2805,6 +2805,7 @@ 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;
>   }
>
> @@ -2826,7 +2827,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;
>   }
>
> @@ -2856,6 +2856,7 @@ void qemu_chr_delete(CharDriverState *chr)
>       QTAILQ_REMOVE(&chardevs, chr, next);
>       if (chr->chr_close)
>           chr->chr_close(chr);
> +    qemu_opts_del(chr->opts);
>       g_free(chr->filename);
>       g_free(chr->label);
>       g_free(chr);
> @@ -2900,3 +2901,50 @@ CharDriverState *qemu_char_get_next_serial(void)
>       return serial_hds[next_serial++];
>   }
>
> +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
> +{
> +    CharDriverState *chr;
> +    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;
> +    }
> +
> +    chr = qemu_chr_new_from_opts(opts, NULL);
> +    if (chr == NULL) {
> +        qemu_opts_del(opts);
> +        error_setg(&err, "Creating chardev failed\n");
> +        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);

Maybe this should be replaced by QERR_ macros to keep
compatibility since this one is listed in ErrorClass
in the schema, like:

error_set(errp, QERR_DEVICE_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\n", id);
> +        return;
> +    }
> +    qemu_chr_delete(chr);
> +}
> diff --git a/qemu-char.h b/qemu-char.h
> index 486644b..a910a60 100644
> --- a/qemu-char.h
> +++ b/qemu-char.h
> @@ -74,6 +74,7 @@ struct CharDriverState {
>       char *filename;
>       int opened;
>       int avail_connections;
> +    QemuOpts *opts;
>       QTAILQ_ENTRY(CharDriverState) next;
>   };
>
> @@ -236,6 +237,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)
> +

Should it support the option "mux" to enable 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


-- 
Lei