Re: [Qemu-devel] [PATCH 4/4] qemu-char: Remove register_char_driver() machinery

[Markus Armbruster]

Peter Maydell <address@hidden> writes:

> Now that all the char backends have been converted to the QAPI
> framework we can remove the machinery for handling old style
> backends.
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  include/sysemu/char.h |   1 -
>  qemu-char.c           | 126 
> +++++++++++++++-----------------------------------
>  2 files changed, 38 insertions(+), 89 deletions(-)
>
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 98cd4c9..dbfd4e6 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -344,7 +344,6 @@ bool chr_is_ringbuf(const CharDriverState *chr);
>  
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
>  
> -void register_char_driver(const char *name, CharDriverState 
> *(*open)(QemuOpts *));
>  void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
>          void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error 
> **errp));

Recommend to rename register_char_driver_qapi() to register_char_driver().

>  
> diff --git a/qemu-char.c b/qemu-char.c
> index a01ccdc..2ffab1a 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3489,26 +3489,12 @@ static void qemu_chr_parse_udp(QemuOpts *opts, 
> ChardevBackend *backend,
>  
>  typedef struct CharDriver {
>      const char *name;
> -    /* old, pre qapi */
> -    CharDriverState *(*open)(QemuOpts *opts);
> -    /* new, qapi-based */
>      ChardevBackendKind kind;
>      void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
>  } CharDriver;
>  
>  static GSList *backends;
>  
> -void register_char_driver(const char *name, CharDriverState 
> *(*open)(QemuOpts *))
> -{
> -    CharDriver *s;
> -
> -    s = g_malloc0(sizeof(*s));
> -    s->name = g_strdup(name);
> -    s->open = open;
> -
> -    backends = g_slist_append(backends, s);
> -}
> -
>  void register_char_driver_qapi(const char *name, ChardevBackendKind kind,
>          void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp))
>  {

The remaining hunks are much easier to review with diff -w:

  @@ -3530,8 +3516,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
       CharDriver *cd;
       CharDriverState *chr;
       GSList *i;
  +    ChardevReturn *ret = NULL;
  +    ChardevBackend *backend;
  +    const char *id = qemu_opts_id(opts);
  +    char *bid = NULL;

  -    if (qemu_opts_id(opts) == NULL) {
  +    if (id == NULL) {
           error_setg(errp, "chardev: no id specified");
           goto err;
       }
  @@ -3554,12 +3544,7 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
           goto err;
       }

  -    if (!cd->open) {
  -        /* using new, qapi init */
  -        ChardevBackend *backend = g_new0(ChardevBackend, 1);
  -        ChardevReturn *ret = NULL;
  -        const char *id = qemu_opts_id(opts);
  -        char *bid = NULL;
  +    backend = g_new0(ChardevBackend, 1);

       if (qemu_opt_get_bool(opts, "mux", 0)) {
           bid = g_strdup_printf("%s-base", id);
  @@ -3603,41 +3588,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
       qapi_free_ChardevReturn(ret);
       g_free(bid);
       return chr;
  -    }
  -
  -    chr = cd->open(opts);
  -    if (!chr) {
  -        error_setg(errp, "chardev: opening backend \"%s\" failed",
  -                   qemu_opt_get(opts, "backend"));
  -        goto err;
  -    }
  -
  -    if (!chr->filename)
  -        chr->filename = g_strdup(qemu_opt_get(opts, "backend"));
  -    chr->init = init;
  -    /* if we didn't create the chardev via qmp_chardev_add, we
  -     * need to send the OPENED event here
  -     */
  -    if (!chr->explicit_be_open) {
  -        qemu_chr_be_event(chr, CHR_EVENT_OPENED);
  -    }
  -    QTAILQ_INSERT_TAIL(&chardevs, chr, next);
  -
  -    if (qemu_opt_get_bool(opts, "mux", 0)) {
  -        CharDriverState *base = chr;
  -        int len = strlen(qemu_opts_id(opts)) + 6;
  -        base->label = g_malloc(len);
  -        snprintf(base->label, len, "%s-base", qemu_opts_id(opts));
  -        chr = qemu_chr_open_mux(base);
  -        chr->filename = base->filename;
  -        chr->avail_connections = MAX_MUX;
  -        QTAILQ_INSERT_TAIL(&chardevs, chr, next);
  -    } else {
  -        chr->avail_connections = 1;
  -    }
  -    chr->label = g_strdup(qemu_opts_id(opts));
  -    chr->opts = opts;
  -    return chr;

   err:
       qemu_opts_del(opts);

Looks good.