diff mbox series

[PULL,04/14] audio: -audiodev command line option basic implementation

Message ID 20190312071250.12168-5-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series [PULL,01/14] qapi: qapi for audio backends | expand

Commit Message

Gerd Hoffmann March 12, 2019, 7:12 a.m. UTC
From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>

Audio drivers now get an Audiodev * as config paramters, instead of the
global audio_option structs.  There is some code in audio/audio_legacy.c
that converts the old environment variables to audiodev options (this
way backends do not have to worry about legacy options).  It also
contains a replacement of -audio-help, which prints out the equivalent
-audiodev based config of the currently specified environment variables.

Note that backends are not updated and still rely on environment
variables.

Also note that (due to moving try-poll from global to backend specific
option) currently ALSA and OSS will always try poll mode, regardless of
environment variables or -audiodev options.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
Message-id: e99a7cbdac0d13512743880660b2032024703e4c.1552083282.git.DirtY.iCE.hu@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 audio/audio.h          |  18 +-
 audio/audio_int.h      |  20 +-
 audio/audio_template.h |  42 ++-
 audio/alsaaudio.c      |   2 +-
 audio/audio.c          | 630 ++++++++++++++++++-----------------------
 audio/audio_legacy.c   | 293 +++++++++++++++++++
 audio/coreaudio.c      |   2 +-
 audio/dsoundaudio.c    |   2 +-
 audio/noaudio.c        |   2 +-
 audio/ossaudio.c       |   2 +-
 audio/paaudio.c        |   2 +-
 audio/sdlaudio.c       |   2 +-
 audio/spiceaudio.c     |   2 +-
 audio/wavaudio.c       |   2 +-
 vl.c                   |   7 +-
 audio/Makefile.objs    |   2 +-
 16 files changed, 650 insertions(+), 380 deletions(-)
 create mode 100644 audio/audio_legacy.c

Comments

Peter Maydell March 14, 2019, 9:46 a.m. UTC | #1
On Tue, 12 Mar 2019 at 07:13, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
>
> Audio drivers now get an Audiodev * as config paramters, instead of the
> global audio_option structs.  There is some code in audio/audio_legacy.c
> that converts the old environment variables to audiodev options (this
> way backends do not have to worry about legacy options).  It also
> contains a replacement of -audio-help, which prints out the equivalent
> -audiodev based config of the currently specified environment variables.

Hi; Coverity complains (CID 1399706) about this, which isn't
a change in this patch as such, but the code change has
probably caused it to reanalyze:

>
>      if (!done) {
>          driver = audio_driver_lookup("none");
> -        done = !audio_driver_init(s, driver, false);
> +        done = !audio_driver_init(s, driver, false, dev);

Everywhere else we call audio_driver_lookup() we check
whether the return value is NULL before using it,
but here we don't. I guess this is a false positive
because the "none" driver must always exist ?
If so, I can just silence the warning in the coverity UI.

thanks
-- PMM
Zoltán Kővágó March 15, 2019, 1:13 a.m. UTC | #2
On 2019-03-14 10:46, Peter Maydell wrote:
> On Tue, 12 Mar 2019 at 07:13, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>> From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
>>
>> Audio drivers now get an Audiodev * as config paramters, instead of the
>> global audio_option structs.  There is some code in audio/audio_legacy.c
>> that converts the old environment variables to audiodev options (this
>> way backends do not have to worry about legacy options).  It also
>> contains a replacement of -audio-help, which prints out the equivalent
>> -audiodev based config of the currently specified environment variables.
> 
> Hi; Coverity complains (CID 1399706) about this, which isn't
> a change in this patch as such, but the code change has
> probably caused it to reanalyze:
> 
>>
>>      if (!done) {
>>          driver = audio_driver_lookup("none");
>> -        done = !audio_driver_init(s, driver, false);
>> +        done = !audio_driver_init(s, driver, false, dev);
> 
> Everywhere else we call audio_driver_lookup() we check
> whether the return value is NULL before using it,
> but here we don't. I guess this is a false positive
> because the "none" driver must always exist ?
> If so, I can just silence the warning in the coverity UI.

Yes, "none" (implemented in noaudio.c) is currently unconditionally
compiled in along with "wav".  "none" is used as a fallback when nothing
else works, so I think it's a bug somewhere else if it doesn't exist.

PS. I think I managed to break something, Thunderbird complained that
non-ascii characters in email addresses are not supported.  Somehow
Kővágó@redhat.com ended up on the recipient list.

Regards,
Zoltan
Pavel Hrdina March 15, 2019, 11:35 a.m. UTC | #3
On Tue, Mar 12, 2019 at 08:12:40AM +0100, Gerd Hoffmann wrote:
> From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
> 
> Audio drivers now get an Audiodev * as config paramters, instead of the
> global audio_option structs.  There is some code in audio/audio_legacy.c
> that converts the old environment variables to audiodev options (this
> way backends do not have to worry about legacy options).  It also
> contains a replacement of -audio-help, which prints out the equivalent
> -audiodev based config of the currently specified environment variables.
> 
> Note that backends are not updated and still rely on environment
> variables.
> 
> Also note that (due to moving try-poll from global to backend specific
> option) currently ALSA and OSS will always try poll mode, regardless of
> environment variables or -audiodev options.

Hi,

I'm glad that this is merged now and I wanted to start working on
libvirt patches, but there is one big issue with this command,
it's not introspectable by query-command-line-options.

My guess based on the QEMU code is that it uses the new function that
allows you to use JSON on qemu command line.

Without the introspection libvirt cannot implement the new option in
sensible way (without version check).

Adding Markus to CC so we can figure out how to wire up the
introspection for such command line options.

Pavel
Markus Armbruster March 15, 2019, 4:32 p.m. UTC | #4
Pavel Hrdina <phrdina@redhat.com> writes:

> On Tue, Mar 12, 2019 at 08:12:40AM +0100, Gerd Hoffmann wrote:
>> From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
>> 
>> Audio drivers now get an Audiodev * as config paramters, instead of the
>> global audio_option structs.  There is some code in audio/audio_legacy.c
>> that converts the old environment variables to audiodev options (this
>> way backends do not have to worry about legacy options).  It also
>> contains a replacement of -audio-help, which prints out the equivalent
>> -audiodev based config of the currently specified environment variables.
>> 
>> Note that backends are not updated and still rely on environment
>> variables.
>> 
>> Also note that (due to moving try-poll from global to backend specific
>> option) currently ALSA and OSS will always try poll mode, regardless of
>> environment variables or -audiodev options.
>
> Hi,
>
> I'm glad that this is merged now and I wanted to start working on
> libvirt patches, but there is one big issue with this command,
> it's not introspectable by query-command-line-options.
>
> My guess based on the QEMU code is that it uses the new function that
> allows you to use JSON on qemu command line.
>
> Without the introspection libvirt cannot implement the new option in
> sensible way (without version check).
>
> Adding Markus to CC so we can figure out how to wire up the
> introspection for such command line options.

query-command-line-options has always been woefully incomplete.  Sadly,
my replacement is still not ready.

A reliable "witness" could serve a stop gap.  Unfortunately,
query-qmp-schema doesn't provide one: the series does not change
generated qapi-introspect.c.

Need to think some more.
Markus Armbruster March 28, 2019, 7:32 p.m. UTC | #5
Markus Armbruster <armbru@redhat.com> writes:

> Pavel Hrdina <phrdina@redhat.com> writes:
>
>> On Tue, Mar 12, 2019 at 08:12:40AM +0100, Gerd Hoffmann wrote:
>>> From: Kővágó, Zoltán <dirty.ice.hu@gmail.com>
>>> 
>>> Audio drivers now get an Audiodev * as config paramters, instead of the
>>> global audio_option structs.  There is some code in audio/audio_legacy.c
>>> that converts the old environment variables to audiodev options (this
>>> way backends do not have to worry about legacy options).  It also
>>> contains a replacement of -audio-help, which prints out the equivalent
>>> -audiodev based config of the currently specified environment variables.
>>> 
>>> Note that backends are not updated and still rely on environment
>>> variables.
>>> 
>>> Also note that (due to moving try-poll from global to backend specific
>>> option) currently ALSA and OSS will always try poll mode, regardless of
>>> environment variables or -audiodev options.
>>
>> Hi,
>>
>> I'm glad that this is merged now and I wanted to start working on
>> libvirt patches, but there is one big issue with this command,
>> it's not introspectable by query-command-line-options.
>>
>> My guess based on the QEMU code is that it uses the new function that
>> allows you to use JSON on qemu command line.
>>
>> Without the introspection libvirt cannot implement the new option in
>> sensible way (without version check).
>>
>> Adding Markus to CC so we can figure out how to wire up the
>> introspection for such command line options.
>
> query-command-line-options has always been woefully incomplete.  Sadly,
> my replacement is still not ready.
>
> A reliable "witness" could serve a stop gap.  Unfortunately,
> query-qmp-schema doesn't provide one: the series does not change
> generated qapi-introspect.c.
>
> Need to think some more.

There is no witness in query-qmp-schema.

We don't want to parse output of -help.

query-command-line-options sucks.  To recap, here are its known defects:

0. It doesn't actually return information on command line options, it
returns information on QemuOpts.  All the other defects follow from this
one.

1. It fails to report most command line options.  Example: -audio.

2. It sometimes screws up the option name.  Example: -m is reported as
   "option": "memory".

2. It sometimes reports no parameters when there are in fact parameters.
   Example: -netdev.

3. It sometimes misses parameters.  Example: -drive misses driver-
   specific parameters.

Can we make q-c-l-o suck less?

Command line options are actually defined in qemu-options.hx, which gets
massaged into qemu_options[].  For each option, qemu_options[] gives us
the option name (without leading '-'), and whether the option takes an
argument.

This is less information per option than q-c-l-o provides now.  Can we
add it to its output anyway without confusing existing users?

Trouble is the QAPI schema is rather inflexible.  It returns a array of
objects with two members "option" and "parameters".  Strictly speaking,
the following are all ABI:

    "option" is the name of the option (modulo the defect mentioned
    above).

    The option takes an argument.

    The argument uses QemuOpts syntax.

    At least the parameters in "parameters" are recognized.

Therefore,

    we can't add options that take no argument, and

    we can't add options with different argument syntax

without breaking ABI.

Alternatives:

1. We add only options that take an argument, like this:

        {"option": "NAME", "parameters": []}

   The meaning of "parameters": [] changes.  In addition to "argument
   uses QemuOpts syntax, parameters unknown", it can now also mean
   "argument uses some other syntax".

2. We add all options that way.

   The meaning of "parameters": [] changes more drastically.  In
   addition to "argument uses QemuOpts syntax, parameters unknown", it
   can now also mean "argument uses some other syntax" and "no
   argument".

3. We add options that take an argument like

        {"option": "NAME", "parameters": null}

   and options that don't take one like

        {"option": "NAME"}

   This turns the *semantic* ABI breaks into *syntactic* ones.

4. We make the syntactic ABI breaks opt-in, i.e. make q-c-l-o take a
flag.

5. Screw it, create a new query command to return just the information
   from qemu_options[].

Alternatives 1. to 3. break ABI in different ways.  Finding out which of
the ABI breaks upset libvirt would be easy enough.  But even if one of
them is fine with libvirt, ABI breaks are best avoided.  Risking one in
4.0 is out of the question, we're far too late in the cycle.

Alternative 5. feels simpler than 4., and is similarly ugly.  This might
still be acceptable (barely) for 4.0.

Opinions?
Eric Blake March 28, 2019, 8:06 p.m. UTC | #6
On 3/28/19 2:32 PM, Markus Armbruster wrote:

>>> Adding Markus to CC so we can figure out how to wire up the
>>> introspection for such command line options.
>>
>> query-command-line-options has always been woefully incomplete.  Sadly,
>> my replacement is still not ready.
>>
>> A reliable "witness" could serve a stop gap.  Unfortunately,
>> query-qmp-schema doesn't provide one: the series does not change
>> generated qapi-introspect.c.
>>
>> Need to think some more.
> 
> There is no witness in query-qmp-schema.


> 5. Screw it, create a new query command to return just the information
>    from qemu_options[].
> 
> Alternatives 1. to 3. break ABI in different ways.  Finding out which of
> the ABI breaks upset libvirt would be easy enough.  But even if one of
> them is fine with libvirt, ABI breaks are best avoided.  Risking one in
> 4.0 is out of the question, we're far too late in the cycle.
> 
> Alternative 5. feels simpler than 4., and is similarly ugly.  This might
> still be acceptable (barely) for 4.0.
> 
> Opinions?

Alternative 6:

Don't worry about patching q-c-l-o, but instead patch query-qemu-features:

https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html

Add a new feature: audiodev-command-line

That addition becomes both introspectible (since query-qemu-features
options are introspectible regardless of their runtime state) and
queryable (not that this feature needs runtime queries, but others might).

And, since we're already proposing query-qemu-features for 4.0 for
another reason, making it 2 reasons instead of 1 feels like extra
justification for getting it done in a timely manner.
Eric Blake March 28, 2019, 8:16 p.m. UTC | #7
On 3/28/19 3:06 PM, Eric Blake wrote:
> On 3/28/19 2:32 PM, Markus Armbruster wrote:
> 
>>>> Adding Markus to CC so we can figure out how to wire up the
>>>> introspection for such command line options.
>>>

> 
> Alternative 6:
> 
> Don't worry about patching q-c-l-o, but instead patch query-qemu-features:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html
> 
> Add a new feature: audiodev-command-line
> 
> That addition becomes both introspectible (since query-qemu-features
> options are introspectible regardless of their runtime state) and
> queryable (not that this feature needs runtime queries, but others might).
> 
> And, since we're already proposing query-qemu-features for 4.0 for
> another reason, making it 2 reasons instead of 1 feels like extra
> justification for getting it done in a timely manner.

And answering myself after a bit more thought - the question is not just
about "can we use the command line instead of envvars", but one step
further of "once we are using the command line, what works in this
release as opposed to added in later releases".  So we still want
introspection to land on the full QAPI types for audiodev, even if, for
4.0, we can't actually use QMP to change them. This means we at least
need a QMP command that references the QAPI types (even if the command
is named "x-audiodev-dummy" and always fails), so that the types at
least make it into the introspection output, coupled with the
query-qemu-features bit to state that even when we remove the hack of
the x-audiodev-dummy command later, we can still use audiodev and scrape
enough out of introspection for our needs.
Markus Armbruster March 29, 2019, 7:19 a.m. UTC | #8
Eric Blake <eblake@redhat.com> writes:

> On 3/28/19 3:06 PM, Eric Blake wrote:
>> On 3/28/19 2:32 PM, Markus Armbruster wrote:
>>> Markus Armbruster <armbru@redhat.com> writes:
>>>> Pavel Hrdina <phrdina@redhat.com> writes:
>> 
>>>>> I'm glad that this is merged now and I wanted to start working on
>>>>> libvirt patches, but there is one big issue with this command,
>>>>> it's not introspectable by query-command-line-options.
[...]
>>>>> Adding Markus to CC so we can figure out how to wire up the
>>>>> introspection for such command line options.
[...]
>>> Command line options are actually defined in qemu-options.hx, which gets
>>> massaged into qemu_options[].  For each option, qemu_options[] gives us
>>> the option name (without leading '-'), and whether the option takes an
>>> argument.
>>>
>>> This is less information per option than q-c-l-o provides now.  Can we
>>> add it to its output anyway without confusing existing users?
[...]
>>> Alternatives:
[...]
>>> 5. Screw it, create a new query command to return just the information
>>>    from qemu_options[].
>> 
>> Alternative 6:
>> 
>> Don't worry about patching q-c-l-o, but instead patch query-qemu-features:
>> 
>> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html
>> 
>> Add a new feature: audiodev-command-line
>> 
>> That addition becomes both introspectible (since query-qemu-features
>> options are introspectible regardless of their runtime state) and
>> queryable (not that this feature needs runtime queries, but others might).

What is query-qemu-features about?  Is it about QMP, or is it a grab-bag
of ad hoc flags?  To be discussed further in the thread you quoted.

>> And, since we're already proposing query-qemu-features for 4.0 for
>> another reason, making it 2 reasons instead of 1 feels like extra
>> justification for getting it done in a timely manner.
>
> And answering myself after a bit more thought - the question is not just
> about "can we use the command line instead of envvars", but one step
> further of "once we are using the command line, what works in this
> release as opposed to added in later releases".  So we still want
> introspection to land on the full QAPI types for audiodev, even if, for
> 4.0, we can't actually use QMP to change them. This means we at least
> need a QMP command that references the QAPI types (even if the command
> is named "x-audiodev-dummy" and always fails), so that the types at
> least make it into the introspection output, coupled with the
> query-qemu-features bit to state that even when we remove the hack of
> the x-audiodev-dummy command later, we can still use audiodev and scrape
> enough out of introspection for our needs.

Alternative 7: a hack to make QAPI type Audiodev visible in
query-qmp-schema.

query-qmp-schema shows exactly the types reachable from QMP commands and
events.

You can't look up a type by name there, you have to start at a command
or an event.  We'd have to create a suitable dummy.  Recent precedence:

    commit e1ca8f7e1915496148f6e0ce1f7c2309af013312
    Author: Gerd Hoffmann <kraxel@redhat.com>
    Date:   Thu Nov 22 08:16:13 2018 +0100

        qapi: add query-display-options command

        Add query-display-options command, which allows querying the qemu
        display configuration.  This isn't particularly useful, except it
        exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt
        can discover recently added -display parameter rendernode (commit
        d4dc4ab133b).  Works around lack of sufficiently powerful command line
        introspection.

        Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

As much as I dislike such hacks, they can be the least bad option.

Compare alternative 7. to 5.: 7. exposes complete syntactic information
just for -audiodev, 5. exposes rudimentary syntactic information for all
options.  There's room for both.  Doesn't mean we should do both, of
course.
Pavel Hrdina March 29, 2019, 8:58 a.m. UTC | #9
On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 3/28/19 3:06 PM, Eric Blake wrote:
> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
> >>> Markus Armbruster <armbru@redhat.com> writes:
> >>>> Pavel Hrdina <phrdina@redhat.com> writes:
> >> 
> >>>>> I'm glad that this is merged now and I wanted to start working on
> >>>>> libvirt patches, but there is one big issue with this command,
> >>>>> it's not introspectable by query-command-line-options.
> [...]
> >>>>> Adding Markus to CC so we can figure out how to wire up the
> >>>>> introspection for such command line options.
> [...]
> >>> Command line options are actually defined in qemu-options.hx, which gets
> >>> massaged into qemu_options[].  For each option, qemu_options[] gives us
> >>> the option name (without leading '-'), and whether the option takes an
> >>> argument.
> >>>
> >>> This is less information per option than q-c-l-o provides now.  Can we
> >>> add it to its output anyway without confusing existing users?
> [...]
> >>> Alternatives:
> [...]
> >>> 5. Screw it, create a new query command to return just the information
> >>>    from qemu_options[].

Hi Markus,

Thanks for looking into this issue, it would be perfect to solve it
before QEMU 4.0 is released.

> >> Alternative 6:
> >> 
> >> Don't worry about patching q-c-l-o, but instead patch query-qemu-features:
> >> 
> >> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html
> >> 
> >> Add a new feature: audiodev-command-line
> >> 
> >> That addition becomes both introspectible (since query-qemu-features
> >> options are introspectible regardless of their runtime state) and
> >> queryable (not that this feature needs runtime queries, but others might).
> 
> What is query-qemu-features about?  Is it about QMP, or is it a grab-bag
> of ad hoc flags?  To be discussed further in the thread you quoted.

From what I was reading the query-qemu-features should be used for
features that are not introspectable from qmp-schema or by other means.
Yes, the 'audiodev' is not currently in the schema but in order to have
reasonable introspection we should not report only that the command line
option is present, we should report also all parameters of that option.

> >> And, since we're already proposing query-qemu-features for 4.0 for
> >> another reason, making it 2 reasons instead of 1 feels like extra
> >> justification for getting it done in a timely manner.
> >
> > And answering myself after a bit more thought - the question is not just
> > about "can we use the command line instead of envvars", but one step
> > further of "once we are using the command line, what works in this
> > release as opposed to added in later releases".  So we still want
> > introspection to land on the full QAPI types for audiodev, even if, for
> > 4.0, we can't actually use QMP to change them. This means we at least
> > need a QMP command that references the QAPI types (even if the command
> > is named "x-audiodev-dummy" and always fails), so that the types at
> > least make it into the introspection output, coupled with the
> > query-qemu-features bit to state that even when we remove the hack of
> > the x-audiodev-dummy command later, we can still use audiodev and scrape
> > enough out of introspection for our needs.
> 
> Alternative 7: a hack to make QAPI type Audiodev visible in
> query-qmp-schema.
> 
> query-qmp-schema shows exactly the types reachable from QMP commands and
> events.
> 
> You can't look up a type by name there, you have to start at a command
> or an event.  We'd have to create a suitable dummy.  Recent precedence:
> 
>     commit e1ca8f7e1915496148f6e0ce1f7c2309af013312
>     Author: Gerd Hoffmann <kraxel@redhat.com>
>     Date:   Thu Nov 22 08:16:13 2018 +0100
> 
>         qapi: add query-display-options command
> 
>         Add query-display-options command, which allows querying the qemu
>         display configuration.  This isn't particularly useful, except it
>         exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt
>         can discover recently added -display parameter rendernode (commit
>         d4dc4ab133b).  Works around lack of sufficiently powerful command line
>         introspection.
> 
>         Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> As much as I dislike such hacks, they can be the least bad option.
> 
> Compare alternative 7. to 5.: 7. exposes complete syntactic information
> just for -audiodev, 5. exposes rudimentary syntactic information for all
> options.  There's room for both.  Doesn't mean we should do both, of
> course.

My guess is that QEMU will introduce more command line parameters which
will be able to consume both options and JSON.  Currently we have
'blockdev', 'display' and 'audiodev' where 'blockdev' is present in the
output of 'query-qmp-schema' through the 'blockdev-add' qmp-command and
for 'display' we have dedicated command.

I would see it like alternative 7. can be used now to fix the issue
present in QEMU 4.0 that we cannot introspect 'audiodev' option and
figure out how to implement some generic qmp-command that would report
command line options and all parameters.

Pavel
Markus Armbruster March 29, 2019, 10:12 a.m. UTC | #10
Pavel Hrdina <phrdina@redhat.com> writes:

> On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>> > On 3/28/19 3:06 PM, Eric Blake wrote:
>> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
>> >>> Markus Armbruster <armbru@redhat.com> writes:
>> >>>> Pavel Hrdina <phrdina@redhat.com> writes:
>> >> 
>> >>>>> I'm glad that this is merged now and I wanted to start working on
>> >>>>> libvirt patches, but there is one big issue with this command,
>> >>>>> it's not introspectable by query-command-line-options.
>> [...]
>> >>>>> Adding Markus to CC so we can figure out how to wire up the
>> >>>>> introspection for such command line options.
>> [...]
>> >>> Command line options are actually defined in qemu-options.hx, which gets
>> >>> massaged into qemu_options[].  For each option, qemu_options[] gives us
>> >>> the option name (without leading '-'), and whether the option takes an
>> >>> argument.
>> >>>
>> >>> This is less information per option than q-c-l-o provides now.  Can we
>> >>> add it to its output anyway without confusing existing users?
>> [...]
>> >>> Alternatives:
>> [...]
>> >>> 5. Screw it, create a new query command to return just the information
>> >>>    from qemu_options[].
>
> Hi Markus,
>
> Thanks for looking into this issue, it would be perfect to solve it
> before QEMU 4.0 is released.

To be honest, I wouldn't bet my own money on 4.0 at this point.

I understand why you're eager to switch libvirt to -audiodev, it's such
a massive improvement over the traditional mess.  But is it urgent?
Does it fix bugs?  Does it add features?  Sure it can't wait for 4.1?

>> >> Alternative 6:
>> >> 
>> >> Don't worry about patching q-c-l-o, but instead patch query-qemu-features:
>> >> 
>> >> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07584.html
>> >> 
>> >> Add a new feature: audiodev-command-line
>> >> 
>> >> That addition becomes both introspectible (since query-qemu-features
>> >> options are introspectible regardless of their runtime state) and
>> >> queryable (not that this feature needs runtime queries, but others might).
>> 
>> What is query-qemu-features about?  Is it about QMP, or is it a grab-bag
>> of ad hoc flags?  To be discussed further in the thread you quoted.
>
> From what I was reading the query-qemu-features should be used for
> features that are not introspectable from qmp-schema or by other means.
> Yes, the 'audiodev' is not currently in the schema but in order to have
> reasonable introspection we should not report only that the command line
> option is present, we should report also all parameters of that option.

Yes.  I feel pressing query-qemu-features into service here would border
on abuse.

Of course, abuse can be less bad than doing nothing.  I depends on the
particular case.

>> >> And, since we're already proposing query-qemu-features for 4.0 for
>> >> another reason, making it 2 reasons instead of 1 feels like extra
>> >> justification for getting it done in a timely manner.
>> >
>> > And answering myself after a bit more thought - the question is not just
>> > about "can we use the command line instead of envvars", but one step
>> > further of "once we are using the command line, what works in this
>> > release as opposed to added in later releases".  So we still want
>> > introspection to land on the full QAPI types for audiodev, even if, for
>> > 4.0, we can't actually use QMP to change them. This means we at least
>> > need a QMP command that references the QAPI types (even if the command
>> > is named "x-audiodev-dummy" and always fails), so that the types at
>> > least make it into the introspection output, coupled with the
>> > query-qemu-features bit to state that even when we remove the hack of
>> > the x-audiodev-dummy command later, we can still use audiodev and scrape
>> > enough out of introspection for our needs.
>> 
>> Alternative 7: a hack to make QAPI type Audiodev visible in
>> query-qmp-schema.
>> 
>> query-qmp-schema shows exactly the types reachable from QMP commands and
>> events.
>> 
>> You can't look up a type by name there, you have to start at a command
>> or an event.  We'd have to create a suitable dummy.  Recent precedence:
>> 
>>     commit e1ca8f7e1915496148f6e0ce1f7c2309af013312
>>     Author: Gerd Hoffmann <kraxel@redhat.com>
>>     Date:   Thu Nov 22 08:16:13 2018 +0100
>> 
>>         qapi: add query-display-options command
>> 
>>         Add query-display-options command, which allows querying the qemu
>>         display configuration.  This isn't particularly useful, except it
>>         exposes QAPI type DisplayOptions in query-qmp-schema, so that libvirt
>>         can discover recently added -display parameter rendernode (commit
>>         d4dc4ab133b).  Works around lack of sufficiently powerful command line
>>         introspection.
>> 
>>         Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> 
>> As much as I dislike such hacks, they can be the least bad option.
>> 
>> Compare alternative 7. to 5.: 7. exposes complete syntactic information
>> just for -audiodev, 5. exposes rudimentary syntactic information for all
>> options.  There's room for both.  Doesn't mean we should do both, of
>> course.
>
> My guess is that QEMU will introduce more command line parameters which
> will be able to consume both options and JSON.

I think so, too.

>                                                 Currently we have
> 'blockdev', 'display' and 'audiodev' where 'blockdev' is present in the
> output of 'query-qmp-schema' through the 'blockdev-add' qmp-command and
> for 'display' we have dedicated command.
>
> I would see it like alternative 7. can be used now to fix the issue
> present in QEMU 4.0 that we cannot introspect 'audiodev' option and
> figure out how to implement some generic qmp-command that would report
> command line options and all parameters.

I figure we need to come up with a plan for incremental command line
QAPIfication beyond the ad hoc ways we've used so far, complete with
schema introspection.

This should obsolete any QMP commands that exist only to expose bits of
the command line QAPI schema in the QMP QAPI schema.  I think the
commands should be deprecated then.
Pavel Hrdina March 29, 2019, 11:16 a.m. UTC | #11
On Fri, Mar 29, 2019 at 11:12:55AM +0100, Markus Armbruster wrote:
> Pavel Hrdina <phrdina@redhat.com> writes:
> 
> > On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
> >> Eric Blake <eblake@redhat.com> writes:
> >> 
> >> > On 3/28/19 3:06 PM, Eric Blake wrote:
> >> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
> >> >>> Markus Armbruster <armbru@redhat.com> writes:
> >> >>>> Pavel Hrdina <phrdina@redhat.com> writes:
> >> >> 
> >> >>>>> I'm glad that this is merged now and I wanted to start working on
> >> >>>>> libvirt patches, but there is one big issue with this command,
> >> >>>>> it's not introspectable by query-command-line-options.
> >> [...]
> >> >>>>> Adding Markus to CC so we can figure out how to wire up the
> >> >>>>> introspection for such command line options.
> >> [...]
> >> >>> Command line options are actually defined in qemu-options.hx, which gets
> >> >>> massaged into qemu_options[].  For each option, qemu_options[] gives us
> >> >>> the option name (without leading '-'), and whether the option takes an
> >> >>> argument.
> >> >>>
> >> >>> This is less information per option than q-c-l-o provides now.  Can we
> >> >>> add it to its output anyway without confusing existing users?
> >> [...]
> >> >>> Alternatives:
> >> [...]
> >> >>> 5. Screw it, create a new query command to return just the information
> >> >>>    from qemu_options[].
> >
> > Hi Markus,
> >
> > Thanks for looking into this issue, it would be perfect to solve it
> > before QEMU 4.0 is released.
> 
> To be honest, I wouldn't bet my own money on 4.0 at this point.
> 
> I understand why you're eager to switch libvirt to -audiodev, it's such
> a massive improvement over the traditional mess.  But is it urgent?
> Does it fix bugs?  Does it add features?  Sure it can't wait for 4.1?

It's definitely not urgent, the audio situation is broken the whole
time so we can wait for 4.1.  It will help us to fix some bugs but
nothing critical.

Pavel
Markus Armbruster March 29, 2019, 1:58 p.m. UTC | #12
Pavel Hrdina <phrdina@redhat.com> writes:

> On Fri, Mar 29, 2019 at 11:12:55AM +0100, Markus Armbruster wrote:
>> Pavel Hrdina <phrdina@redhat.com> writes:
>> 
>> > On Fri, Mar 29, 2019 at 08:19:55AM +0100, Markus Armbruster wrote:
>> >> Eric Blake <eblake@redhat.com> writes:
>> >> 
>> >> > On 3/28/19 3:06 PM, Eric Blake wrote:
>> >> >> On 3/28/19 2:32 PM, Markus Armbruster wrote:
>> >> >>> Markus Armbruster <armbru@redhat.com> writes:
>> >> >>>> Pavel Hrdina <phrdina@redhat.com> writes:
>> >> >> 
>> >> >>>>> I'm glad that this is merged now and I wanted to start working on
>> >> >>>>> libvirt patches, but there is one big issue with this command,
>> >> >>>>> it's not introspectable by query-command-line-options.
>> >> [...]
>> >> >>>>> Adding Markus to CC so we can figure out how to wire up the
>> >> >>>>> introspection for such command line options.
>> >> [...]
>> >> >>> Command line options are actually defined in qemu-options.hx, which gets
>> >> >>> massaged into qemu_options[].  For each option, qemu_options[] gives us
>> >> >>> the option name (without leading '-'), and whether the option takes an
>> >> >>> argument.
>> >> >>>
>> >> >>> This is less information per option than q-c-l-o provides now.  Can we
>> >> >>> add it to its output anyway without confusing existing users?
>> >> [...]
>> >> >>> Alternatives:
>> >> [...]
>> >> >>> 5. Screw it, create a new query command to return just the information
>> >> >>>    from qemu_options[].
>> >
>> > Hi Markus,
>> >
>> > Thanks for looking into this issue, it would be perfect to solve it
>> > before QEMU 4.0 is released.
>> 
>> To be honest, I wouldn't bet my own money on 4.0 at this point.
>> 
>> I understand why you're eager to switch libvirt to -audiodev, it's such
>> a massive improvement over the traditional mess.  But is it urgent?
>> Does it fix bugs?  Does it add features?  Sure it can't wait for 4.1?
>
> It's definitely not urgent, the audio situation is broken the whole
> time so we can wait for 4.1.  It will help us to fix some bugs but
> nothing critical.

Let's aim for something decent in 4.1.  Perhaps alternative 5.  Perhaps
even something that can grow into real command line introspection.
Perhaps both.
diff mbox series

Patch

diff --git a/audio/audio.h b/audio/audio.h
index 02f29a3b3ef7..64b0f761bcaa 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -36,12 +36,21 @@  typedef void (*audio_callback_fn) (void *opaque, int avail);
 #define AUDIO_HOST_ENDIANNESS 0
 #endif
 
-struct audsettings {
+typedef struct audsettings {
     int freq;
     int nchannels;
     AudioFormat fmt;
     int endianness;
-};
+} audsettings;
+
+audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo);
+int audioformat_bytes_per_sample(AudioFormat fmt);
+int audio_buffer_frames(AudiodevPerDirectionOptions *pdo,
+                        audsettings *as, int def_usecs);
+int audio_buffer_samples(AudiodevPerDirectionOptions *pdo,
+                         audsettings *as, int def_usecs);
+int audio_buffer_bytes(AudiodevPerDirectionOptions *pdo,
+                       audsettings *as, int def_usecs);
 
 typedef enum {
     AUD_CNOTIFY_ENABLE,
@@ -81,7 +90,6 @@  typedef struct QEMUAudioTimeStamp {
 void AUD_vlog (const char *cap, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
 void AUD_log (const char *cap, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 
-void AUD_help (void);
 void AUD_register_card (const char *name, QEMUSoundCard *card);
 void AUD_remove_card (QEMUSoundCard *card);
 CaptureVoiceOut *AUD_add_capture (
@@ -163,4 +171,8 @@  void audio_sample_to_uint64(void *samples, int pos,
 void audio_sample_from_uint64(void *samples, int pos,
                             uint64_t left, uint64_t right);
 
+void audio_parse_option(const char *opt);
+void audio_init_audiodevs(void);
+void audio_legacy_help(void);
+
 #endif /* QEMU_AUDIO_H */
diff --git a/audio/audio_int.h b/audio/audio_int.h
index 6c451b995ccc..7bf5dfc0b5f6 100644
--- a/audio/audio_int.h
+++ b/audio/audio_int.h
@@ -146,7 +146,7 @@  struct audio_driver {
     const char *name;
     const char *descr;
     struct audio_option *options;
-    void *(*init) (void);
+    void *(*init) (Audiodev *);
     void (*fini) (void *);
     struct audio_pcm_ops *pcm_ops;
     int can_be_default;
@@ -193,6 +193,7 @@  struct SWVoiceCap {
 
 typedef struct AudioState {
     struct audio_driver *drv;
+    Audiodev *dev;
     void *drv_opaque;
 
     QEMUTimer *ts;
@@ -203,10 +204,13 @@  typedef struct AudioState {
     int nb_hw_voices_out;
     int nb_hw_voices_in;
     int vm_running;
+    int64_t period_ticks;
 } AudioState;
 
 extern const struct mixeng_volume nominal_volume;
 
+extern const char *audio_prio_list[];
+
 void audio_driver_register(audio_driver *drv);
 audio_driver *audio_driver_lookup(const char *name);
 
@@ -248,4 +252,18 @@  static inline int audio_ring_dist (int dst, int src, int len)
 #define AUDIO_STRINGIFY_(n) #n
 #define AUDIO_STRINGIFY(n) AUDIO_STRINGIFY_(n)
 
+typedef struct AudiodevListEntry {
+    Audiodev *dev;
+    QSIMPLEQ_ENTRY(AudiodevListEntry) next;
+} AudiodevListEntry;
+
+typedef QSIMPLEQ_HEAD(, AudiodevListEntry) AudiodevListHead;
+AudiodevListHead audio_handle_legacy_opts(void);
+
+void audio_free_audiodev_list(AudiodevListHead *head);
+
+void audio_create_pdos(Audiodev *dev);
+AudiodevPerDirectionOptions *audio_get_pdo_in(Audiodev *dev);
+AudiodevPerDirectionOptions *audio_get_pdo_out(Audiodev *dev);
+
 #endif /* QEMU_AUDIO_INT_H */
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 7de227d2d10c..1232bb54db0e 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -299,11 +299,42 @@  static HW *glue (audio_pcm_hw_add_new_, TYPE) (struct audsettings *as)
     return NULL;
 }
 
+AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev)
+{
+    switch (dev->driver) {
+    case AUDIODEV_DRIVER_NONE:
+        return dev->u.none.TYPE;
+    case AUDIODEV_DRIVER_ALSA:
+        return qapi_AudiodevAlsaPerDirectionOptions_base(dev->u.alsa.TYPE);
+    case AUDIODEV_DRIVER_COREAUDIO:
+        return qapi_AudiodevCoreaudioPerDirectionOptions_base(
+            dev->u.coreaudio.TYPE);
+    case AUDIODEV_DRIVER_DSOUND:
+        return dev->u.dsound.TYPE;
+    case AUDIODEV_DRIVER_OSS:
+        return qapi_AudiodevOssPerDirectionOptions_base(dev->u.oss.TYPE);
+    case AUDIODEV_DRIVER_PA:
+        return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
+    case AUDIODEV_DRIVER_SDL:
+        return dev->u.sdl.TYPE;
+    case AUDIODEV_DRIVER_SPICE:
+        return dev->u.spice.TYPE;
+    case AUDIODEV_DRIVER_WAV:
+        return dev->u.wav.TYPE;
+
+    case AUDIODEV_DRIVER__MAX:
+        break;
+    }
+    abort();
+}
+
 static HW *glue (audio_pcm_hw_add_, TYPE) (struct audsettings *as)
 {
     HW *hw;
+    AudioState *s = &glob_audio_state;
+    AudiodevPerDirectionOptions *pdo = glue(audio_get_pdo_, TYPE)(s->dev);
 
-    if (glue (conf.fixed_, TYPE).enabled && glue (conf.fixed_, TYPE).greedy) {
+    if (pdo->fixed_settings) {
         hw = glue (audio_pcm_hw_add_new_, TYPE) (as);
         if (hw) {
             return hw;
@@ -331,9 +362,11 @@  static SW *glue (audio_pcm_create_voice_pair_, TYPE) (
     SW *sw;
     HW *hw;
     struct audsettings hw_as;
+    AudioState *s = &glob_audio_state;
+    AudiodevPerDirectionOptions *pdo = glue(audio_get_pdo_, TYPE)(s->dev);
 
-    if (glue (conf.fixed_, TYPE).enabled) {
-        hw_as = glue (conf.fixed_, TYPE).settings;
+    if (pdo->fixed_settings) {
+        hw_as = audiodev_to_audsettings(pdo);
     }
     else {
         hw_as = *as;
@@ -398,6 +431,7 @@  SW *glue (AUD_open_, TYPE) (
     )
 {
     AudioState *s = &glob_audio_state;
+    AudiodevPerDirectionOptions *pdo = glue(audio_get_pdo_, TYPE)(s->dev);
 
     if (audio_bug(__func__, !card || !name || !callback_fn || !as)) {
         dolog ("card=%p name=%p callback_fn=%p as=%p\n",
@@ -422,7 +456,7 @@  SW *glue (AUD_open_, TYPE) (
         return sw;
     }
 
-    if (!glue (conf.fixed_, TYPE).enabled && sw) {
+    if (!pdo->fixed_settings && sw) {
         glue (AUD_close_, TYPE) (card, sw);
         sw = NULL;
     }
diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 5bd034267fce..8302f3e882d7 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -1125,7 +1125,7 @@  static ALSAConf glob_conf = {
     .pcm_name_in = "default",
 };
 
-static void *alsa_audio_init (void)
+static void *alsa_audio_init(Audiodev *dev)
 {
     ALSAConf *conf = g_malloc(sizeof(ALSAConf));
     *conf = glob_conf;
diff --git a/audio/audio.c b/audio/audio.c
index 77216e50102e..79ed36066604 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -26,6 +26,9 @@ 
 #include "audio.h"
 #include "monitor/monitor.h"
 #include "qemu/timer.h"
+#include "qapi/error.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qapi-visit-audio.h"
 #include "sysemu/sysemu.h"
 #include "qemu/cutils.h"
 #include "sysemu/replay.h"
@@ -46,14 +49,16 @@ 
    The 1st one is the one used by default, that is the reason
     that we generate the list.
 */
-static const char *audio_prio_list[] = {
+const char *audio_prio_list[] = {
     "spice",
     CONFIG_AUDIO_DRIVERS
     "none",
     "wav",
+    NULL
 };
 
 static QLIST_HEAD(, audio_driver) audio_drivers;
+static AudiodevListHead audiodevs = QSIMPLEQ_HEAD_INITIALIZER(audiodevs);
 
 void audio_driver_register(audio_driver *drv)
 {
@@ -80,61 +85,6 @@  audio_driver *audio_driver_lookup(const char *name)
     return NULL;
 }
 
-static void audio_module_load_all(void)
-{
-    int i;
-
-    for (i = 0; i < ARRAY_SIZE(audio_prio_list); i++) {
-        audio_driver_lookup(audio_prio_list[i]);
-    }
-}
-
-struct fixed_settings {
-    int enabled;
-    int nb_voices;
-    int greedy;
-    struct audsettings settings;
-};
-
-static struct {
-    struct fixed_settings fixed_out;
-    struct fixed_settings fixed_in;
-    union {
-        int hertz;
-        int64_t ticks;
-    } period;
-    int try_poll_in;
-    int try_poll_out;
-} conf = {
-    .fixed_out = { /* DAC fixed settings */
-        .enabled = 1,
-        .nb_voices = 1,
-        .greedy = 1,
-        .settings = {
-            .freq = 44100,
-            .nchannels = 2,
-            .fmt = AUDIO_FORMAT_S16,
-            .endianness =  AUDIO_HOST_ENDIANNESS,
-        }
-    },
-
-    .fixed_in = { /* ADC fixed settings */
-        .enabled = 1,
-        .nb_voices = 1,
-        .greedy = 1,
-        .settings = {
-            .freq = 44100,
-            .nchannels = 2,
-            .fmt = AUDIO_FORMAT_S16,
-            .endianness = AUDIO_HOST_ENDIANNESS,
-        }
-    },
-
-    .period = { .hertz = 100 },
-    .try_poll_in = 1,
-    .try_poll_out = 1,
-};
-
 static AudioState glob_audio_state;
 
 const struct mixeng_volume nominal_volume = {
@@ -151,9 +101,6 @@  const struct mixeng_volume nominal_volume = {
 #ifdef AUDIO_IS_FLAWLESS_AND_NO_CHECKS_ARE_REQURIED
 #error No its not
 #else
-static void audio_print_options (const char *prefix,
-                                 struct audio_option *opt);
-
 int audio_bug (const char *funcname, int cond)
 {
     if (cond) {
@@ -161,16 +108,9 @@  int audio_bug (const char *funcname, int cond)
 
         AUD_log (NULL, "A bug was just triggered in %s\n", funcname);
         if (!shown) {
-            struct audio_driver *d;
-
             shown = 1;
             AUD_log (NULL, "Save all your work and restart without audio\n");
-            AUD_log (NULL, "Please send bug report to av1474@comtv.ru\n");
             AUD_log (NULL, "I am sorry\n");
-            d = glob_audio_state.drv;
-            if (d) {
-                audio_print_options (d->name, d->options);
-            }
         }
         AUD_log (NULL, "Context:\n");
 
@@ -232,31 +172,6 @@  void *audio_calloc (const char *funcname, int nmemb, size_t size)
     return g_malloc0 (len);
 }
 
-static char *audio_alloc_prefix (const char *s)
-{
-    const char qemu_prefix[] = "QEMU_";
-    size_t len, i;
-    char *r, *u;
-
-    if (!s) {
-        return NULL;
-    }
-
-    len = strlen (s);
-    r = g_malloc (len + sizeof (qemu_prefix));
-
-    u = r + sizeof (qemu_prefix) - 1;
-
-    pstrcpy (r, len + sizeof (qemu_prefix), qemu_prefix);
-    pstrcat (r, len + sizeof (qemu_prefix), s);
-
-    for (i = 0; i < len; ++i) {
-        u[i] = qemu_toupper(u[i]);
-    }
-
-    return r;
-}
-
 static const char *audio_audfmt_to_string (AudioFormat fmt)
 {
     switch (fmt) {
@@ -382,78 +297,6 @@  void AUD_log (const char *cap, const char *fmt, ...)
     va_end (ap);
 }
 
-static void audio_print_options (const char *prefix,
-                                 struct audio_option *opt)
-{
-    char *uprefix;
-
-    if (!prefix) {
-        dolog ("No prefix specified\n");
-        return;
-    }
-
-    if (!opt) {
-        dolog ("No options\n");
-        return;
-    }
-
-    uprefix = audio_alloc_prefix (prefix);
-
-    for (; opt->name; opt++) {
-        const char *state = "default";
-        printf ("  %s_%s: ", uprefix, opt->name);
-
-        if (opt->overriddenp && *opt->overriddenp) {
-            state = "current";
-        }
-
-        switch (opt->tag) {
-        case AUD_OPT_BOOL:
-            {
-                int *intp = opt->valp;
-                printf ("boolean, %s = %d\n", state, *intp ? 1 : 0);
-            }
-            break;
-
-        case AUD_OPT_INT:
-            {
-                int *intp = opt->valp;
-                printf ("integer, %s = %d\n", state, *intp);
-            }
-            break;
-
-        case AUD_OPT_FMT:
-            {
-                AudioFormat *fmtp = opt->valp;
-                printf (
-                    "format, %s = %s, (one of: U8 S8 U16 S16 U32 S32)\n",
-                    state,
-                    audio_audfmt_to_string (*fmtp)
-                    );
-            }
-            break;
-
-        case AUD_OPT_STR:
-            {
-                const char **strp = opt->valp;
-                printf ("string, %s = %s\n",
-                        state,
-                        *strp ? *strp : "(not set)");
-            }
-            break;
-
-        default:
-            printf ("???\n");
-            dolog ("Bad value tag for option %s_%s %d\n",
-                   uprefix, opt->name, opt->tag);
-            break;
-        }
-        printf ("    %s\n", opt->descr);
-    }
-
-    g_free (uprefix);
-}
-
 static void audio_process_options (const char *prefix,
                                    struct audio_option *opt)
 {
@@ -1141,11 +984,11 @@  static void audio_reset_timer (AudioState *s)
 {
     if (audio_is_timer_needed ()) {
         timer_mod_anticipate_ns(s->ts,
-            qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
+            qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->period_ticks);
         if (!audio_timer_running) {
             audio_timer_running = true;
             audio_timer_last = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            trace_audio_timer_start(conf.period.ticks / SCALE_MS);
+            trace_audio_timer_start(s->period_ticks / SCALE_MS);
         }
     } else {
         timer_del(s->ts);
@@ -1159,16 +1002,17 @@  static void audio_reset_timer (AudioState *s)
 static void audio_timer (void *opaque)
 {
     int64_t now, diff;
+    AudioState *s = opaque;
 
     now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     diff = now - audio_timer_last;
-    if (diff > conf.period.ticks * 3 / 2) {
+    if (diff > s->period_ticks * 3 / 2) {
         trace_audio_timer_delayed(diff / SCALE_MS);
     }
     audio_timer_last = now;
 
-    audio_run ("timer");
-    audio_reset_timer (opaque);
+    audio_run("timer");
+    audio_reset_timer(s);
 }
 
 /*
@@ -1228,7 +1072,7 @@  void AUD_set_active_out (SWVoiceOut *sw, int on)
             if (!hw->enabled) {
                 hw->enabled = 1;
                 if (s->vm_running) {
-                    hw->pcm_ops->ctl_out (hw, VOICE_ENABLE, conf.try_poll_out);
+                    hw->pcm_ops->ctl_out(hw, VOICE_ENABLE, true);
                     audio_reset_timer (s);
                 }
             }
@@ -1273,7 +1117,7 @@  void AUD_set_active_in (SWVoiceIn *sw, int on)
             if (!hw->enabled) {
                 hw->enabled = 1;
                 if (s->vm_running) {
-                    hw->pcm_ops->ctl_in (hw, VOICE_ENABLE, conf.try_poll_in);
+                    hw->pcm_ops->ctl_in(hw, VOICE_ENABLE, true);
                     audio_reset_timer (s);
                 }
             }
@@ -1594,169 +1438,13 @@  void audio_run (const char *msg)
 #endif
 }
 
-static struct audio_option audio_options[] = {
-    /* DAC */
-    {
-        .name  = "DAC_FIXED_SETTINGS",
-        .tag   = AUD_OPT_BOOL,
-        .valp  = &conf.fixed_out.enabled,
-        .descr = "Use fixed settings for host DAC"
-    },
-    {
-        .name  = "DAC_FIXED_FREQ",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.fixed_out.settings.freq,
-        .descr = "Frequency for fixed host DAC"
-    },
-    {
-        .name  = "DAC_FIXED_FMT",
-        .tag   = AUD_OPT_FMT,
-        .valp  = &conf.fixed_out.settings.fmt,
-        .descr = "Format for fixed host DAC"
-    },
-    {
-        .name  = "DAC_FIXED_CHANNELS",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.fixed_out.settings.nchannels,
-        .descr = "Number of channels for fixed DAC (1 - mono, 2 - stereo)"
-    },
-    {
-        .name  = "DAC_VOICES",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.fixed_out.nb_voices,
-        .descr = "Number of voices for DAC"
-    },
-    {
-        .name  = "DAC_TRY_POLL",
-        .tag   = AUD_OPT_BOOL,
-        .valp  = &conf.try_poll_out,
-        .descr = "Attempt using poll mode for DAC"
-    },
-    /* ADC */
-    {
-        .name  = "ADC_FIXED_SETTINGS",
-        .tag   = AUD_OPT_BOOL,
-        .valp  = &conf.fixed_in.enabled,
-        .descr = "Use fixed settings for host ADC"
-    },
-    {
-        .name  = "ADC_FIXED_FREQ",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.fixed_in.settings.freq,
-        .descr = "Frequency for fixed host ADC"
-    },
-    {
-        .name  = "ADC_FIXED_FMT",
-        .tag   = AUD_OPT_FMT,
-        .valp  = &conf.fixed_in.settings.fmt,
-        .descr = "Format for fixed host ADC"
-    },
-    {
-        .name  = "ADC_FIXED_CHANNELS",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.fixed_in.settings.nchannels,
-        .descr = "Number of channels for fixed ADC (1 - mono, 2 - stereo)"
-    },
-    {
-        .name  = "ADC_VOICES",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.fixed_in.nb_voices,
-        .descr = "Number of voices for ADC"
-    },
-    {
-        .name  = "ADC_TRY_POLL",
-        .tag   = AUD_OPT_BOOL,
-        .valp  = &conf.try_poll_in,
-        .descr = "Attempt using poll mode for ADC"
-    },
-    /* Misc */
-    {
-        .name  = "TIMER_PERIOD",
-        .tag   = AUD_OPT_INT,
-        .valp  = &conf.period.hertz,
-        .descr = "Timer period in HZ (0 - use lowest possible)"
-    },
-    { /* End of list */ }
-};
-
-static void audio_pp_nb_voices (const char *typ, int nb)
-{
-    switch (nb) {
-    case 0:
-        printf ("Does not support %s\n", typ);
-        break;
-    case 1:
-        printf ("One %s voice\n", typ);
-        break;
-    case INT_MAX:
-        printf ("Theoretically supports many %s voices\n", typ);
-        break;
-    default:
-        printf ("Theoretically supports up to %d %s voices\n", nb, typ);
-        break;
-    }
-
-}
-
-void AUD_help (void)
-{
-    struct audio_driver *d;
-
-    /* make sure we print the help text for modular drivers too */
-    audio_module_load_all();
-
-    audio_process_options ("AUDIO", audio_options);
-    QLIST_FOREACH(d, &audio_drivers, next) {
-        if (d->options) {
-            audio_process_options (d->name, d->options);
-        }
-    }
-
-    printf ("Audio options:\n");
-    audio_print_options ("AUDIO", audio_options);
-    printf ("\n");
-
-    printf ("Available drivers:\n");
-
-    QLIST_FOREACH(d, &audio_drivers, next) {
-
-        printf ("Name: %s\n", d->name);
-        printf ("Description: %s\n", d->descr);
-
-        audio_pp_nb_voices ("playback", d->max_voices_out);
-        audio_pp_nb_voices ("capture", d->max_voices_in);
-
-        if (d->options) {
-            printf ("Options:\n");
-            audio_print_options (d->name, d->options);
-        }
-        else {
-            printf ("No options\n");
-        }
-        printf ("\n");
-    }
-
-    printf (
-        "Options are settable through environment variables.\n"
-        "Example:\n"
-#ifdef _WIN32
-        "  set QEMU_AUDIO_DRV=wav\n"
-        "  set QEMU_WAV_PATH=c:\\tune.wav\n"
-#else
-        "  export QEMU_AUDIO_DRV=wav\n"
-        "  export QEMU_WAV_PATH=$HOME/tune.wav\n"
-        "(for csh replace export with setenv in the above)\n"
-#endif
-        "  qemu ...\n\n"
-        );
-}
-
-static int audio_driver_init(AudioState *s, struct audio_driver *drv, bool msg)
+static int audio_driver_init(AudioState *s, struct audio_driver *drv,
+                             bool msg, Audiodev *dev)
 {
     if (drv->options) {
         audio_process_options (drv->name, drv->options);
     }
-    s->drv_opaque = drv->init ();
+    s->drv_opaque = drv->init(dev);
 
     if (s->drv_opaque) {
         audio_init_nb_voices_out (drv);
@@ -1782,11 +1470,11 @@  static void audio_vm_change_state_handler (void *opaque, int running,
 
     s->vm_running = running;
     while ((hwo = audio_pcm_hw_find_any_enabled_out (hwo))) {
-        hwo->pcm_ops->ctl_out (hwo, op, conf.try_poll_out);
+        hwo->pcm_ops->ctl_out(hwo, op, true);
     }
 
     while ((hwi = audio_pcm_hw_find_any_enabled_in (hwi))) {
-        hwi->pcm_ops->ctl_in (hwi, op, conf.try_poll_in);
+        hwi->pcm_ops->ctl_in(hwi, op, true);
     }
     audio_reset_timer (s);
 }
@@ -1836,6 +1524,11 @@  void audio_cleanup(void)
         s->drv->fini (s->drv_opaque);
         s->drv = NULL;
     }
+
+    if (s->dev) {
+        qapi_free_Audiodev(s->dev);
+        s->dev = NULL;
+    }
 }
 
 static const VMStateDescription vmstate_audio = {
@@ -1847,19 +1540,58 @@  static const VMStateDescription vmstate_audio = {
     }
 };
 
-static void audio_init (void)
+static void audio_validate_opts(Audiodev *dev, Error **errp);
+
+static AudiodevListEntry *audiodev_find(
+    AudiodevListHead *head, const char *drvname)
+{
+    AudiodevListEntry *e;
+    QSIMPLEQ_FOREACH(e, head, next) {
+        if (strcmp(AudiodevDriver_str(e->dev->driver), drvname) == 0) {
+            return e;
+        }
+    }
+
+    return NULL;
+}
+
+static int audio_init(Audiodev *dev)
 {
     size_t i;
     int done = 0;
-    const char *drvname;
+    const char *drvname = NULL;
     VMChangeStateEntry *e;
     AudioState *s = &glob_audio_state;
     struct audio_driver *driver;
+    /* silence gcc warning about uninitialized variable */
+    AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
 
     if (s->drv) {
-        return;
+        if (dev) {
+            dolog("Cannot create more than one audio backend, sorry\n");
+            qapi_free_Audiodev(dev);
+        }
+        return -1;
     }
 
+    if (dev) {
+        /* -audiodev option */
+        drvname = AudiodevDriver_str(dev->driver);
+    } else {
+        /* legacy implicit initialization */
+        head = audio_handle_legacy_opts();
+        /*
+         * In case of legacy initialization, all Audiodevs in the list will have
+         * the same configuration (except the driver), so it does't matter which
+         * one we chose.  We need an Audiodev to set up AudioState before we can
+         * init a driver.  Also note that dev at this point is still in the
+         * list.
+         */
+        dev = QSIMPLEQ_FIRST(&head)->dev;
+        audio_validate_opts(dev, &error_abort);
+    }
+    s->dev = dev;
+
     QLIST_INIT (&s->hw_head_out);
     QLIST_INIT (&s->hw_head_in);
     QLIST_INIT (&s->cap_head);
@@ -1867,10 +1599,8 @@  static void audio_init (void)
 
     s->ts = timer_new_ns(QEMU_CLOCK_VIRTUAL, audio_timer, s);
 
-    audio_process_options ("AUDIO", audio_options);
-
-    s->nb_hw_voices_out = conf.fixed_out.nb_voices;
-    s->nb_hw_voices_in = conf.fixed_in.nb_voices;
+    s->nb_hw_voices_out = audio_get_pdo_out(dev)->voices;
+    s->nb_hw_voices_in = audio_get_pdo_in(dev)->voices;
 
     if (s->nb_hw_voices_out <= 0) {
         dolog ("Bogus number of playback voices %d, setting to 1\n",
@@ -1884,46 +1614,42 @@  static void audio_init (void)
         s->nb_hw_voices_in = 0;
     }
 
-    {
-        int def;
-        drvname = audio_get_conf_str ("QEMU_AUDIO_DRV", NULL, &def);
-    }
-
     if (drvname) {
         driver = audio_driver_lookup(drvname);
         if (driver) {
-            done = !audio_driver_init(s, driver, true);
+            done = !audio_driver_init(s, driver, true, dev);
         } else {
             dolog ("Unknown audio driver `%s'\n", drvname);
-            dolog ("Run with -audio-help to list available drivers\n");
         }
-    }
-
-    if (!done) {
-        for (i = 0; !done && i < ARRAY_SIZE(audio_prio_list); i++) {
+    } else {
+        for (i = 0; audio_prio_list[i]; i++) {
+            AudiodevListEntry *e = audiodev_find(&head, audio_prio_list[i]);
             driver = audio_driver_lookup(audio_prio_list[i]);
-            if (driver && driver->can_be_default) {
-                done = !audio_driver_init(s, driver, false);
+
+            if (e && driver) {
+                s->dev = dev = e->dev;
+                audio_validate_opts(dev, &error_abort);
+                done = !audio_driver_init(s, driver, false, dev);
+                if (done) {
+                    e->dev = NULL;
+                    break;
+                }
             }
         }
     }
+    audio_free_audiodev_list(&head);
 
     if (!done) {
         driver = audio_driver_lookup("none");
-        done = !audio_driver_init(s, driver, false);
+        done = !audio_driver_init(s, driver, false, dev);
         assert(done);
         dolog("warning: Using timer based audio emulation\n");
     }
 
-    if (conf.period.hertz <= 0) {
-        if (conf.period.hertz < 0) {
-            dolog ("warning: Timer period is negative - %d "
-                   "treating as zero\n",
-                   conf.period.hertz);
-        }
-        conf.period.ticks = 1;
+    if (dev->timer_period <= 0) {
+        s->period_ticks = 1;
     } else {
-        conf.period.ticks = NANOSECONDS_PER_SECOND / conf.period.hertz;
+        s->period_ticks = NANOSECONDS_PER_SECOND / dev->timer_period;
     }
 
     e = qemu_add_vm_change_state_handler (audio_vm_change_state_handler, s);
@@ -1934,11 +1660,22 @@  static void audio_init (void)
 
     QLIST_INIT (&s->card_head);
     vmstate_register (NULL, 0, &vmstate_audio, s);
+    return 0;
+}
+
+void audio_free_audiodev_list(AudiodevListHead *head)
+{
+    AudiodevListEntry *e;
+    while ((e = QSIMPLEQ_FIRST(head))) {
+        QSIMPLEQ_REMOVE_HEAD(head, next);
+        qapi_free_Audiodev(e->dev);
+        g_free(e);
+    }
 }
 
 void AUD_register_card (const char *name, QEMUSoundCard *card)
 {
-    audio_init ();
+    audio_init(NULL);
     card->name = g_strdup (name);
     memset (&card->entries, 0, sizeof (card->entries));
     QLIST_INSERT_HEAD (&glob_audio_state.card_head, card, entries);
@@ -2078,3 +1815,174 @@  void AUD_set_volume_in (SWVoiceIn *sw, int mute, uint8_t lvol, uint8_t rvol)
         }
     }
 }
+
+void audio_create_pdos(Audiodev *dev)
+{
+    switch (dev->driver) {
+#define CASE(DRIVER, driver, pdo_name)                              \
+    case AUDIODEV_DRIVER_##DRIVER:                                  \
+        if (!dev->u.driver.has_in) {                                \
+            dev->u.driver.in = g_malloc0(                           \
+                sizeof(Audiodev##pdo_name##PerDirectionOptions));   \
+            dev->u.driver.has_in = true;                            \
+        }                                                           \
+        if (!dev->u.driver.has_out) {                               \
+            dev->u.driver.out = g_malloc0(                          \
+                sizeof(AudiodevAlsaPerDirectionOptions));           \
+            dev->u.driver.has_out = true;                           \
+        }                                                           \
+        break
+
+        CASE(NONE, none, );
+        CASE(ALSA, alsa, Alsa);
+        CASE(COREAUDIO, coreaudio, Coreaudio);
+        CASE(DSOUND, dsound, );
+        CASE(OSS, oss, Oss);
+        CASE(PA, pa, Pa);
+        CASE(SDL, sdl, );
+        CASE(SPICE, spice, );
+        CASE(WAV, wav, );
+
+    case AUDIODEV_DRIVER__MAX:
+        abort();
+    };
+}
+
+static void audio_validate_per_direction_opts(
+    AudiodevPerDirectionOptions *pdo, Error **errp)
+{
+    if (!pdo->has_fixed_settings) {
+        pdo->has_fixed_settings = true;
+        pdo->fixed_settings = true;
+    }
+    if (!pdo->fixed_settings &&
+        (pdo->has_frequency || pdo->has_channels || pdo->has_format)) {
+        error_setg(errp,
+                   "You can't use frequency, channels or format with fixed-settings=off");
+        return;
+    }
+
+    if (!pdo->has_frequency) {
+        pdo->has_frequency = true;
+        pdo->frequency = 44100;
+    }
+    if (!pdo->has_channels) {
+        pdo->has_channels = true;
+        pdo->channels = 2;
+    }
+    if (!pdo->has_voices) {
+        pdo->has_voices = true;
+        pdo->voices = 1;
+    }
+    if (!pdo->has_format) {
+        pdo->has_format = true;
+        pdo->format = AUDIO_FORMAT_S16;
+    }
+}
+
+static void audio_validate_opts(Audiodev *dev, Error **errp)
+{
+    Error *err = NULL;
+
+    audio_create_pdos(dev);
+
+    audio_validate_per_direction_opts(audio_get_pdo_in(dev), &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    audio_validate_per_direction_opts(audio_get_pdo_out(dev), &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    if (!dev->has_timer_period) {
+        dev->has_timer_period = true;
+        dev->timer_period = 10000; /* 100Hz -> 10ms */
+    }
+}
+
+void audio_parse_option(const char *opt)
+{
+    AudiodevListEntry *e;
+    Audiodev *dev = NULL;
+
+    Visitor *v = qobject_input_visitor_new_str(opt, "driver", &error_fatal);
+    visit_type_Audiodev(v, NULL, &dev, &error_fatal);
+    visit_free(v);
+
+    audio_validate_opts(dev, &error_fatal);
+
+    e = g_malloc0(sizeof(AudiodevListEntry));
+    e->dev = dev;
+    QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
+}
+
+void audio_init_audiodevs(void)
+{
+    AudiodevListEntry *e;
+
+    QSIMPLEQ_FOREACH(e, &audiodevs, next) {
+        audio_init(e->dev);
+    }
+}
+
+audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
+{
+    return (audsettings) {
+        .freq = pdo->frequency,
+        .nchannels = pdo->channels,
+        .fmt = pdo->format,
+        .endianness = AUDIO_HOST_ENDIANNESS,
+    };
+}
+
+int audioformat_bytes_per_sample(AudioFormat fmt)
+{
+    switch (fmt) {
+    case AUDIO_FORMAT_U8:
+    case AUDIO_FORMAT_S8:
+        return 1;
+
+    case AUDIO_FORMAT_U16:
+    case AUDIO_FORMAT_S16:
+        return 2;
+
+    case AUDIO_FORMAT_U32:
+    case AUDIO_FORMAT_S32:
+        return 4;
+
+    case AUDIO_FORMAT__MAX:
+        ;
+    }
+    abort();
+}
+
+
+/* frames = freq * usec / 1e6 */
+int audio_buffer_frames(AudiodevPerDirectionOptions *pdo,
+                        audsettings *as, int def_usecs)
+{
+    uint64_t usecs = pdo->has_buffer_length ? pdo->buffer_length : def_usecs;
+    return (as->freq * usecs + 500000) / 1000000;
+}
+
+/* samples = channels * frames = channels * freq * usec / 1e6 */
+int audio_buffer_samples(AudiodevPerDirectionOptions *pdo,
+                         audsettings *as, int def_usecs)
+{
+    return as->nchannels * audio_buffer_frames(pdo, as, def_usecs);
+}
+
+/*
+ * bytes = bytes_per_sample * samples =
+ *     bytes_per_sample * channels * freq * usec / 1e6
+ */
+int audio_buffer_bytes(AudiodevPerDirectionOptions *pdo,
+                       audsettings *as, int def_usecs)
+{
+    return audio_buffer_samples(pdo, as, def_usecs) *
+        audioformat_bytes_per_sample(as->fmt);
+}
diff --git a/audio/audio_legacy.c b/audio/audio_legacy.c
new file mode 100644
index 000000000000..ad3b781addf8
--- /dev/null
+++ b/audio/audio_legacy.c
@@ -0,0 +1,293 @@ 
+/*
+ * QEMU Audio subsystem: legacy configuration handling
+ *
+ * Copyright (c) 2015-2019 Zoltán Kővágó <DirtY.iCE.hu@gmail.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "audio.h"
+#include "audio_int.h"
+#include "qemu-common.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "qapi/qapi-visit-audio.h"
+#include "qapi/visitor-impl.h"
+
+#define AUDIO_CAP "audio-legacy"
+#include "audio_int.h"
+
+static uint32_t toui32(const char *str)
+{
+    unsigned long long ret;
+    if (parse_uint_full(str, &ret, 10) || ret > UINT32_MAX) {
+        dolog("Invalid integer value `%s'\n", str);
+        exit(1);
+    }
+    return ret;
+}
+
+/* helper functions to convert env variables */
+static void get_bool(const char *env, bool *dst, bool *has_dst)
+{
+    const char *val = getenv(env);
+    if (val) {
+        *dst = toui32(val) != 0;
+        *has_dst = true;
+    }
+}
+
+static void get_int(const char *env, uint32_t *dst, bool *has_dst)
+{
+    const char *val = getenv(env);
+    if (val) {
+        *dst = toui32(val);
+        *has_dst = true;
+    }
+}
+
+static void get_fmt(const char *env, AudioFormat *dst, bool *has_dst)
+{
+    const char *val = getenv(env);
+    if (val) {
+        size_t i;
+        for (i = 0; AudioFormat_lookup.size; ++i) {
+            if (strcasecmp(val, AudioFormat_lookup.array[i]) == 0) {
+                *dst = i;
+                *has_dst = true;
+                return;
+            }
+        }
+
+        dolog("Invalid audio format `%s'\n", val);
+        exit(1);
+    }
+}
+
+/* backend specific functions */
+/* todo */
+
+/* general */
+static void handle_per_direction(
+    AudiodevPerDirectionOptions *pdo, const char *prefix)
+{
+    char buf[64];
+    size_t len = strlen(prefix);
+
+    memcpy(buf, prefix, len);
+    strcpy(buf + len, "FIXED_SETTINGS");
+    get_bool(buf, &pdo->fixed_settings, &pdo->has_fixed_settings);
+
+    strcpy(buf + len, "FIXED_FREQ");
+    get_int(buf, &pdo->frequency, &pdo->has_frequency);
+
+    strcpy(buf + len, "FIXED_FMT");
+    get_fmt(buf, &pdo->format, &pdo->has_format);
+
+    strcpy(buf + len, "FIXED_CHANNELS");
+    get_int(buf, &pdo->channels, &pdo->has_channels);
+
+    strcpy(buf + len, "VOICES");
+    get_int(buf, &pdo->voices, &pdo->has_voices);
+}
+
+static AudiodevListEntry *legacy_opt(const char *drvname)
+{
+    AudiodevListEntry *e = g_malloc0(sizeof(AudiodevListEntry));
+    e->dev = g_malloc0(sizeof(Audiodev));
+    e->dev->id = g_strdup(drvname);
+    e->dev->driver = qapi_enum_parse(
+        &AudiodevDriver_lookup, drvname, -1, &error_abort);
+
+    audio_create_pdos(e->dev);
+
+    handle_per_direction(audio_get_pdo_in(e->dev), "QEMU_AUDIO_ADC_");
+    handle_per_direction(audio_get_pdo_out(e->dev), "QEMU_AUDIO_DAC_");
+
+    get_int("QEMU_AUDIO_TIMER_PERIOD",
+            &e->dev->timer_period, &e->dev->has_timer_period);
+
+    return e;
+}
+
+AudiodevListHead audio_handle_legacy_opts(void)
+{
+    const char *drvname = getenv("QEMU_AUDIO_DRV");
+    AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
+
+    if (drvname) {
+        AudiodevListEntry *e;
+        audio_driver *driver = audio_driver_lookup(drvname);
+        if (!driver) {
+            dolog("Unknown audio driver `%s'\n", drvname);
+            exit(1);
+        }
+        e = legacy_opt(drvname);
+        QSIMPLEQ_INSERT_TAIL(&head, e, next);
+    } else {
+        for (int i = 0; audio_prio_list[i]; i++) {
+            audio_driver *driver = audio_driver_lookup(audio_prio_list[i]);
+            if (driver && driver->can_be_default) {
+                AudiodevListEntry *e = legacy_opt(driver->name);
+                QSIMPLEQ_INSERT_TAIL(&head, e, next);
+            }
+        }
+        if (QSIMPLEQ_EMPTY(&head)) {
+            dolog("Internal error: no default audio driver available\n");
+            exit(1);
+        }
+    }
+
+    return head;
+}
+
+/* visitor to print -audiodev option */
+typedef struct {
+    Visitor visitor;
+
+    bool comma;
+    GList *path;
+} LegacyPrintVisitor;
+
+static void lv_start_struct(Visitor *v, const char *name, void **obj,
+                            size_t size, Error **errp)
+{
+    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
+    lv->path = g_list_append(lv->path, g_strdup(name));
+}
+
+static void lv_end_struct(Visitor *v, void **obj)
+{
+    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
+    lv->path = g_list_delete_link(lv->path, g_list_last(lv->path));
+}
+
+static void lv_print_key(Visitor *v, const char *name)
+{
+    GList *e;
+    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
+    if (lv->comma) {
+        putchar(',');
+    } else {
+        lv->comma = true;
+    }
+
+    for (e = lv->path; e; e = e->next) {
+        if (e->data) {
+            printf("%s.", (const char *) e->data);
+        }
+    }
+
+    printf("%s=", name);
+}
+
+static void lv_type_int64(Visitor *v, const char *name, int64_t *obj,
+                          Error **errp)
+{
+    lv_print_key(v, name);
+    printf("%" PRIi64, *obj);
+}
+
+static void lv_type_uint64(Visitor *v, const char *name, uint64_t *obj,
+                           Error **errp)
+{
+    lv_print_key(v, name);
+    printf("%" PRIu64, *obj);
+}
+
+static void lv_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
+{
+    lv_print_key(v, name);
+    printf("%s", *obj ? "on" : "off");
+}
+
+static void lv_type_str(Visitor *v, const char *name, char **obj, Error **errp)
+{
+    const char *str = *obj;
+    lv_print_key(v, name);
+
+    while (*str) {
+        if (*str == ',') {
+            putchar(',');
+        }
+        putchar(*str++);
+    }
+}
+
+static void lv_complete(Visitor *v, void *opaque)
+{
+    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
+    assert(lv->path == NULL);
+}
+
+static void lv_free(Visitor *v)
+{
+    LegacyPrintVisitor *lv = (LegacyPrintVisitor *) v;
+
+    g_list_free_full(lv->path, g_free);
+    g_free(lv);
+}
+
+static Visitor *legacy_visitor_new(void)
+{
+    LegacyPrintVisitor *lv = g_malloc0(sizeof(LegacyPrintVisitor));
+
+    lv->visitor.start_struct = lv_start_struct;
+    lv->visitor.end_struct = lv_end_struct;
+    /* lists not supported */
+    lv->visitor.type_int64 = lv_type_int64;
+    lv->visitor.type_uint64 = lv_type_uint64;
+    lv->visitor.type_bool = lv_type_bool;
+    lv->visitor.type_str = lv_type_str;
+
+    lv->visitor.type = VISITOR_OUTPUT;
+    lv->visitor.complete = lv_complete;
+    lv->visitor.free = lv_free;
+
+    return &lv->visitor;
+}
+
+void audio_legacy_help(void)
+{
+    AudiodevListHead head;
+    AudiodevListEntry *e;
+
+    printf("Environment variable based configuration deprecated.\n");
+    printf("Please use the new -audiodev option.\n");
+
+    head = audio_handle_legacy_opts();
+    printf("\nEquivalent -audiodev to your current environment variables:\n");
+    if (!getenv("QEMU_AUDIO_DRV")) {
+        printf("(Since you didn't specify QEMU_AUDIO_DRV, I'll list all "
+               "possibilities)\n");
+    }
+
+    QSIMPLEQ_FOREACH(e, &head, next) {
+        Visitor *v;
+        Audiodev *dev = e->dev;
+        printf("-audiodev ");
+
+        v = legacy_visitor_new();
+        visit_type_Audiodev(v, NULL, &dev, &error_abort);
+        visit_free(v);
+
+        printf("\n");
+    }
+    audio_free_audiodev_list(&head);
+}
diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 638c60b300b2..7d4225dbeeff 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -685,7 +685,7 @@  static CoreaudioConf glob_conf = {
     .nbuffers = 4,
 };
 
-static void *coreaudio_audio_init (void)
+static void *coreaudio_audio_init(Audiodev *dev)
 {
     CoreaudioConf *conf = g_malloc(sizeof(CoreaudioConf));
     *conf = glob_conf;
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index 3ed73a30d1e3..02fe777cba75 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -783,7 +783,7 @@  static void dsound_audio_fini (void *opaque)
     g_free(s);
 }
 
-static void *dsound_audio_init (void)
+static void *dsound_audio_init(Audiodev *dev)
 {
     int err;
     HRESULT hr;
diff --git a/audio/noaudio.c b/audio/noaudio.c
index 1bfebeca7da9..79690af1eabd 100644
--- a/audio/noaudio.c
+++ b/audio/noaudio.c
@@ -136,7 +136,7 @@  static int no_ctl_in (HWVoiceIn *hw, int cmd, ...)
     return 0;
 }
 
-static void *no_audio_init (void)
+static void *no_audio_init(Audiodev *dev)
 {
     return &no_audio_init;
 }
diff --git a/audio/ossaudio.c b/audio/ossaudio.c
index 355e8fbda519..e0cadbef29df 100644
--- a/audio/ossaudio.c
+++ b/audio/ossaudio.c
@@ -842,7 +842,7 @@  static OSSConf glob_conf = {
     .policy = 5
 };
 
-static void *oss_audio_init (void)
+static void *oss_audio_init(Audiodev *dev)
 {
     OSSConf *conf = g_malloc(sizeof(OSSConf));
     *conf = glob_conf;
diff --git a/audio/paaudio.c b/audio/paaudio.c
index 8246f260a8ac..d649c58e3d4c 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -812,7 +812,7 @@  static PAConf glob_conf = {
     .samples = 4096,
 };
 
-static void *qpa_audio_init (void)
+static void *qpa_audio_init(Audiodev *dev)
 {
     if (glob_conf.server == NULL) {
         char pidfile[64];
diff --git a/audio/sdlaudio.c b/audio/sdlaudio.c
index 4cd4cbaf0043..b9ae506a5651 100644
--- a/audio/sdlaudio.c
+++ b/audio/sdlaudio.c
@@ -315,7 +315,7 @@  static int sdl_ctl_out (HWVoiceOut *hw, int cmd, ...)
     return 0;
 }
 
-static void *sdl_audio_init (void)
+static void *sdl_audio_init(Audiodev *dev)
 {
     SDLAudioState *s = &glob_sdl;
     if (s->driver_created) {
diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index 3aeb0cb35722..affc3df17f90 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -77,7 +77,7 @@  static const SpiceRecordInterface record_sif = {
     .base.minor_version = SPICE_INTERFACE_RECORD_MINOR,
 };
 
-static void *spice_audio_init (void)
+static void *spice_audio_init(Audiodev *dev)
 {
     if (!using_spice) {
         return NULL;
diff --git a/audio/wavaudio.c b/audio/wavaudio.c
index 35a614785ece..9eff3555b35f 100644
--- a/audio/wavaudio.c
+++ b/audio/wavaudio.c
@@ -232,7 +232,7 @@  static WAVConf glob_conf = {
     .wav_path           = "qemu.wav"
 };
 
-static void *wav_audio_init (void)
+static void *wav_audio_init(Audiodev *dev)
 {
     WAVConf *conf = g_malloc(sizeof(WAVConf));
     *conf = glob_conf;
diff --git a/vl.c b/vl.c
index 4a350de5cd19..db04aea7f8c9 100644
--- a/vl.c
+++ b/vl.c
@@ -3253,9 +3253,12 @@  int main(int argc, char **argv, char **envp)
                 add_device_config(DEV_BT, optarg);
                 break;
             case QEMU_OPTION_audio_help:
-                AUD_help ();
+                audio_legacy_help();
                 exit (0);
                 break;
+            case QEMU_OPTION_audiodev:
+                audio_parse_option(optarg);
+                break;
             case QEMU_OPTION_soundhw:
                 select_soundhw (optarg);
                 break;
@@ -4447,6 +4450,8 @@  int main(int argc, char **argv, char **envp)
     /* do monitor/qmp handling at preconfig state if requested */
     main_loop();
 
+    audio_init_audiodevs();
+
     /* from here on runstate is RUN_STATE_PRELAUNCH */
     machine_run_board_init(current_machine);
 
diff --git a/audio/Makefile.objs b/audio/Makefile.objs
index db4fa7f18f25..dca87f63470d 100644
--- a/audio/Makefile.objs
+++ b/audio/Makefile.objs
@@ -1,4 +1,4 @@ 
-common-obj-y = audio.o noaudio.o wavaudio.o mixeng.o
+common-obj-y = audio.o audio_legacy.o noaudio.o wavaudio.o mixeng.o
 common-obj-$(CONFIG_SPICE) += spiceaudio.o
 common-obj-$(CONFIG_AUDIO_COREAUDIO) += coreaudio.o
 common-obj-$(CONFIG_AUDIO_DSOUND) += dsoundaudio.o