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

[Paolo Bonzini]

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
>