Patchwork [1/9] chardev: add support for qapi-based chardev initialization

login
register
mail settings
Submitter Gerd Hoffmann
Date Feb. 25, 2013, 9:03 a.m.
Message ID <1361783023-10082-2-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/222869/
State New
Headers show

Comments

Gerd Hoffmann - Feb. 25, 2013, 9:03 a.m.
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 <kraxel@redhat.com>
---
 qemu-char.c |   30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
Michael Roth - Feb. 26, 2013, 1:45 a.m.
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 <kraxel@redhat.com>
> ---
>  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
> 
>
Gerd Hoffmann - Feb. 26, 2013, 9:35 a.m.
Hi,

> 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.

There is no 1:1 mapping between QemuOpts and ChardevBackend, so I don't
think this is going to fly.

cheers,
  Gerd
Paolo Bonzini - Feb. 26, 2013, 2:03 p.m.
Il 26/02/2013 10:35, Gerd Hoffmann ha scritto:
> 
>> > 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.
> There is no 1:1 mapping between QemuOpts and ChardevBackend, so I don't
> think this is going to fly.

No, it's not.  I looked briefly at it when doing the
socket_{connect,listen} stuff, but the resulting QAPI structs made no sense.

Paolo

Patch

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