ext/audio: set and reload stubdom pulseaudio module #589
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #589 +/- ##
==========================================
- Coverage 68.50% 68.28% -0.23%
==========================================
Files 56 56
Lines 11232 11277 +45
==========================================
+ Hits 7695 7701 +6
- Misses 3537 3576 +39
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Tests failed here. My guess is missing dom0 in the app.domains mock collection (and default_audiovm defaults to dom0). |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.2&build=2024051221-4.2&flavor=pull-requests New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.2&build=2024050210-4.2&flavor=update
Failed tests8 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/98585#dependencies 1 fixed
Unstable tests
|
I fixed it and added a specific test with audiovm not being dom0 in xml. |
qubes/ext/audio.py
Outdated
| subject.log.warning("Cannot change dynamically audiovm: qrexec" | ||
| " is not available (stubdom-qrexec feature)") | ||
| if has_audio_model and has_stubdom_qrexec: | ||
| subject.log.warning(f"setting {newvalue.xid}") |
There was a problem hiding this comment.
Leftover debug message? But if it should stay, make it not explode on None as newvalue, and make more informative (at least mention it's about audiovm).
qubes/ext/audio.py
Outdated
| async def set_stubdom_audiovm_domid(qube, audiovm): | ||
| try: | ||
| await qube.run_service_for_stdio( | ||
| f"qubes.SetAudioVM+{audiovm.xid}", |
templates/libvirt/xen.xml
Outdated
| {% if vm.audiovm is defined %} | ||
| {% set audiovm_xid = vm.audiovm.xid %} | ||
| {% else %} | ||
| {% set audiovm_xid = 0 %} |
There was a problem hiding this comment.
Should this really default to 0 (dom0)? I'd say setting audiovm to None should result in some dummy value here (-1? empty?) which then avoids loading the vchan module (so only the null one is loaded). But setting audiovm to something later should still work.
But that's a minor issue. If you prefer to do that later, it's okay, but open an issue to not forget.
There was a problem hiding this comment.
I'll set it to -1 so I need to adapt the stubdom part to not load module on start.
fepitre
force-pushed
the
devel20240503
branch
2 times, most recently
from
59dd4a8
to
b2e28e0
Compare
fepitre
force-pushed
the
devel20240503
branch
2 times, most recently
from
410ea8e
to
913dcb8
Compare
| @@ -179,13 +184,13 @@ | |||
| {% endif %} | |||
| {% if vm.netvm %} | |||
There was a problem hiding this comment.
This all is added only if netvm is set. You need to add a case for no-netvm too.
|
PipelineRetry |
|
While I like it now, CI doesn't. Pylint complains and some tests need an update... |
|
PipelineRetry |
Fixes QubesOS/qubes-issues#8975