Re: [RFC PATCH 6/6] vl: introduce -audio as a replacement for -soundhw

[Martin Kletzander]

On Wed, Apr 27, 2022 at 01:32:25PM +0200, Paolo Bonzini wrote:
> -audio is used like "-audio pa,model=sb16".  It is almost as simple as
> -soundhw, but it reuses the -audiodev parsing machinery and attaches an
> audiodev to the newly-created device.  The main 'feature' is that
> it knows about adding the codec device for model=intel-hda, and adding
> the audiodev to the codec device.
>
> In the future, it could be extended to support default models or
> builtin devices, just like -nic, or even a default backend.  For now,
> keep it simple.
>
> JSON parsing is not supported for -audio.  This is okay because the
> option is targeted at end users, not programs.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> audio/audio.c                   |  8 +++++-
> audio/audio.h                   |  1 +
> docs/about/deprecated.rst       |  9 ------
> docs/about/removed-features.rst |  7 +++++
> hw/audio/intel-hda.c            |  5 ++--
> hw/audio/soundhw.c              | 12 +++++---
> include/hw/audio/soundhw.h      |  4 +--
> qemu-options.hx                 | 51 ++++++++++++++++-----------------
> softmmu/vl.c                    | 28 ++++++++++++++++--
> 9 files changed, 76 insertions(+), 49 deletions(-)

[...]

> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5bea0eb3eb..979bbda5aa 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -3018,13 +3020,33 @@ void qemu_init(int argc, char **argv, char **envp)
>             case QEMU_OPTION_audiodev:
>                 audio_parse_option(optarg);
>                 break;
> -            case QEMU_OPTION_soundhw:
> -                if (is_help_option(optarg)) {
> +            case QEMU_OPTION_audio: {
> +                QDict *dict = keyval_parse(optarg, "driver", NULL, 
> &error_fatal);
> +                char *model;
> +                Audiodev *dev = NULL;
> +                Visitor *v;
> +
> +                if (!qdict_haskey(dict, "id")) {
> +                    qdict_put_str(dict, "id", "audiodev0");
> +                }
> +                if (!qdict_haskey(dict, "model")) {
> +                    error_setg(&error_fatal, "Parameter 'model' is missing");
> +                }
> +                model = g_strdup(qdict_get_str(dict, "model"));
> +                qdict_del(dict, "model");
> +                if (is_help_option(model)) {
>                     show_valid_soundhw();
>                     exit(0);
>                 }
> -                select_soundhw (optarg);
> +                v = qobject_input_visitor_new_keyval(QOBJECT(dict));
> +                qobject_unref(dict);
> +                visit_type_Audiodev(v, NULL, &dev, &error_fatal);
> +                visit_free(v);
> +                audio_define(dev);

I was looking at this and thought that you might be creating multiple
audiodevs if there are multiple -audio options.  And I found out that
even with current master it is possible to create multiple audiodevs not
only with the same driver, but even with the same ID (and different
drivers).

I guess that is not preferable, but I can't say for sure how this is
supposed to be handled.

Other than that it looks fine to me.

signature.asc
Description: PGP signature