Re: [Qemu-devel] [PATCH 1/9] chardev: add support for qapi-based chardev initialization

[mdroth]

On Mon, Feb 25, 2013 at 10:03:33AM +0100, Gerd Hoffmann wrote:
> This patch add support for a new way to initialize chardev devices.
> Instead of calling a initialization function with a QemuOpts we will
> now create a (qapi) ChardevBackend, optionally call a function to
> fill ChardevBackend from QemuOpts, then go create the chardev using
> the new qapi code path which is also used by chardev-add.
> 
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---
>  qemu-char.c |   30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 160decc..cf6b98b 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2974,7 +2974,11 @@ static CharDriverState *qemu_chr_open_pp(QemuOpts 
> *opts)
> 
>  static const struct {
>      const char *name;
> +    /* old, pre qapi */
>      CharDriverState *(*open)(QemuOpts *opts);
> +    /* new, qapi-based */
> +    const int kind;
> +    void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
>  } backend_table[] = {
>      { .name = "null",      .open = qemu_chr_open_null },
>      { .name = "socket",    .open = qemu_chr_open_socket },
> @@ -3040,6 +3044,32 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>          goto err;
>      }
> 
> +    if (!backend_table[i].open) {
> +        /* using new, qapi init */
> +        ChardevBackend *backend = g_new0(ChardevBackend, 1);
> +        ChardevReturn *ret = NULL;
> +        const char *id = qemu_opts_id(opts);
> +
> +        chr = NULL;
> +        backend->kind = backend_table[i].kind;
> +        if (backend_table[i].parse) {
> +            backend_table[i].parse(opts, backend, errp);
> +            if (error_is_set(errp)) {
> +                goto qapi_out;
> +            }
> +        }

Have you tried using visit_type_ChardevBackend() with an OptsVisitor to
handle the option parsing? It's how -netdev options are parsed now, so
it should "just work" in theory.

Might be worth looking at now as opposed to a follow-up since it'll
avoid the need to introduce .parse functions to the backend table as you
go.

> +        ret = qmp_chardev_add(qemu_opts_id(opts), backend, errp);
> +        if (error_is_set(errp)) {
> +            goto qapi_out;
> +        }
> +        chr = qemu_chr_find(id);
> +
> +    qapi_out:
> +        qapi_free_ChardevBackend(backend);
> +        qapi_free_ChardevReturn(ret);
> +        return chr;
> +    }
> +
>      chr = backend_table[i].open(opts);
>      if (!chr) {
>          error_setg(errp, "chardev: opening backend \"%s\" failed",
> -- 
> 1.7.9.7
> 
>