Re: [PATCH v3 07/11] ui/dbus: run without Audio interface when audio is disabled

[Markus Armbruster <armbru@redhat.com>]

"Sergei Heifetz" <heifetz@yandex-team.com> writes:

> On Thu Mar 19, 2026 at 12:58 PM +05, Markus Armbruster wrote:
>> Sergei Heifetz <heifetz@yandex-team.com> writes:
>>
>>> D-Bus display can be used even when QEMU is configured with
>>> `--disable-audio`. In that case, audio interface will not be available
>>> on `/org/qemu/Display1/Audio`.
>>>
>>> (The current handling of the situation when audio is enabled but
>>> no D-Bus-compatible audio backend is available is different and
>>> hasn’t been changed.)
>>>
>>> Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
>>> ---
>>>  qapi/ui.json    | 2 +-
>>>  qemu-options.hx | 4 ++++
>>>  ui/dbus.c       | 8 ++++++++
>>>  3 files changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/ui.json b/qapi/ui.json
>>> index e3da77632a8..62eb05c6091 100644
>>> --- a/qapi/ui.json
>>> +++ b/qapi/ui.json
>>> @@ -1385,7 +1385,7 @@
>>>    'data'    : { '*rendernode' : 'str',
>>>                  '*addr': 'str',
>>>                  '*p2p': 'bool',
>>> -                '*audiodev': 'str' } }
>>> +                '*audiodev': { 'type': 'str', 'if': 'CONFIG_AUDIO' } } }
>>
>> Is this a compatibility break?
>>
>> DisplayDBus is only used in DisplayOptions, which is only used as return
>> type of query-display-options in the QAPI schema.  However, vl.c also
>> uses it to parse -display.
>>
>> Removing parameter @audiodev from -display is technically a
>> compatibility break.  However, we're removing an optional paramater that
>> would always be rejected when present: it has no valid value, because
>> there are no audiodevs.
>>
>> Removal from query-display-options is similar: it could not be present.
>>
>> Removal affects introspection, which could conceivably confuse client.
>> I don't expect such trouble here, because @audiodev always existed, i.e.
>> there's no real reason to look for it in introspection.
>>
>> I think we're okay.  I'm cc'ing the libvirt list just in case.
>
> These are questions I would like to know the answers to as well. From my
> perspective, disabling compilation of the audio subsystem is worth it,
> but you should expect to potentially tweak a few audio-related things to
> make it work. I agree that technically makes it a compatibility break.
> But preserving meaningless parameters (like audiodev in the total
> absence of audio) certainly makes less sense to me. 
>
>> Do we need a release note?
>>
>> Do we need an entry in docs/about/removed-features.rst?  Feels a bit
>> odd; we're not removing a feature, we're adding one, namely "can compile
>> out audio".
>>
>
> Maybe we should add something like, "If you compile out audio, be
> careful with these things: [list of changes]". 
>
> But honestly, I don't know. I'd be happy to add whatever is needed.

I guess a brief release note should suffice.  As long as the affected
interfaces are obviously audio-related, enumerating them seems
unnecessary.

>>> +        (If audio is disabled at compile time via the ``--disable-audio`` option,
>>> +        no audio backend is used, and none of the audio-related D-Bus interfaces
>>> +        are exported.)
>>> [...]
>>
>> Well, audio is either compiled in or it isn't.
>>
>> If it's compiled in, then why tell the user how things behave when it's
>> not?
>>
>> If it's not compiled in, then why document stuff that doesn't work?  How
>> much of -audiodev still works then?  -audiodev none?  Anything else?
>
> I agree it's weird, but I don't see a clear way around it. Simple
> ifdef'ing isn't possible in a reStructuredText section (at least I
> couldn't get it to work).

Context: this is in an SRST...ERST section of qemu-options.hx.

As far as I can tell, we simply ignore the possibility of the thing
being compiled out when we document a thing there.

Drop the patch hunk?

>> [...]