Skip cleaning named disp - #867
Draft
ben-grande wants to merge 12 commits into
Draft
ben-grande wants to merge 12 commits into
ben-grande wants to merge 12 commits into
Conversation
Contributor
Author
|
openQArun TEST=system_tests_dispvm |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #867 +/- ##
==========================================
- Coverage 70.72% 70.62% -0.11%
==========================================
Files 61 61
Lines 14315 14418 +103
==========================================
+ Hits 10124 10182 +58
- Misses 4191 4236 +45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=202609100837-devel&flavor=pull-requests Test run included the following: Upload failures
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026050504-devel&flavor=update
Failed tests6 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/176874#dependencies 38 fixed
Unstable testsDetails
Performance TestsPerformance degradation:No issues Remaining performance tests:79 tests
|
Contributor
Author
|
I have two ideas for fixing the libvirt issue:
|
Contributor
Author
This is a timeout: # test_016_preload_race_less
# failure:
# timestamp 2026-08-10T17:04:21.886123
Traceback (most recent call last):
File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
return await fut
^^^^^^^^^
asyncio.exceptions.CancelledError
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/usr/lib/python3.13/site-packages/qubes/tests/__init__.py", line 592, in cleanup_loop
self.loop.run_until_complete(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
asyncio.wait_for(libvirt_event_impl.drain(), timeout=30)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
)
^
TimeoutError
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/usr/lib/python3.13/site-packages/qubes/tests/__init__.py", line 596, in cleanup_loop
raise AssertionError("libvirt event impl drain timeout")
AssertionError: libvirt event impl drain timeout
# system-out:
# failure:
# timestamp 1970-01-01T00:00:00
Traceback (most recent call last):
File "/usr/lib/python3.13/site-packages/qubes/tests/__init__.py", line 573, in cleanup_gc
assert not leaked
^^^^^^^^^^
AssertionError
And all the following ones have # test_017_preload_autostart
# failure:
# timestamp 2026-08-10T17:06:47.005305
Traceback (most recent call last):
File "/usr/lib/python3.13/site-packages/qubes/tests/__init__.py", line 592, in cleanup_loop
self.loop.run_until_complete(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
asyncio.wait_for(libvirt_event_impl.drain(), timeout=30)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
)
^
File "/usr/lib64/python3.13/asyncio/base_events.py", line 725, in run_until_complete
return future.result()
~~~~~~~~~~~~~^^
File "/usr/lib64/python3.13/asyncio/tasks.py", line 507, in wait_for
return await fut
^^^^^^^^^
File "/usr/lib64/python3.13/site-packages/libvirtaio.py", line 345, in drain
assert self._finished is None
^^^^^^^^^^^^^^^^^^^^^^
AssertionError
# system-out:
# failure:
# timestamp 1970-01-01T00:00:00
Traceback (most recent call last):
File "/usr/lib/python3.13/site-packages/qubes/tests/__init__.py", line 573, in cleanup_gc
assert not leaked
^^^^^^^^^^
AssertionError |
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
from
August 11, 2026 07:56
ba8185d to
7d886ad
Compare
Contributor
Author
|
openQArun TEST=system_tests_dispvm TEST_TEMPLATES=fedora-44-xfce |
Contributor
Author
|
And just letting the reviewers know, this error only happens on OpenQA, not on my testbench, so testing is slow... |
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
from
August 11, 2026 08:19
7d886ad to
702d4c2
Compare
Contributor
Author
|
openQArun TEST=system_tests_dispvm TEST_TEMPLATES=fedora-44-xfce UPDATE_TEMPLATES=fedora-44-xfce DEFAULT_TEMPLATE=fedora-44-xfce Edit: unfortunately. especifying just a single template didn't reduce the templates that were being upgraded. |
Contributor
Author
Logs failed to upload, but you can see them in the video in minute 1:52: https://openqa.qubes-os.org/tests/192081/video?filename=video.webm&t=111.92,111.96 This one I can reproduce locally: logs
|
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
from
August 11, 2026 15:12
3f42ddd to
68af9fc
Compare
Contributor
Author
|
Without the line to remove preload excess, just below the |
Contributor
Author
|
openQArun TEST=system_tests_dispvm TEST_TEMPLATES=fedora-44-xfce UPDATE_TEMPLATES=fedora-44-xfce |
Contributor
Author
Ok, so removing |
Contributor
Author
|
I think the error might be on the storage side, that has issues when cleanup and startup (even for different qubes) happens too close to each other. |
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
3 times, most recently
from
August 12, 2026 13:28
fc6c1d6 to
fdbb441
Compare
Contributor
Author
|
openQArun TEST=system_tests_dispvm TEST_TEMPLATES=fedora-44-xfce |
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
3 times, most recently
from
August 12, 2026 14:37
8f737d3 to
e6fe724
Compare
Contributor
Author
|
openQArun TEST=system_tests_dispvm TEST_TEMPLATES=fedora-44-xfce |
Contributor
Author
|
PipelineRetryFailed |
Contributor
Author
|
https://openqa.qubes-os.org/tests/195058/file/system_tests-tests-qubes.tests.integ.dispvm.log Details
https://openqa.qubes-os.org/tests/195057/file/system_tests-tests-qubes.tests.integ.basic.log Details
|
Contributor
Author
Couldn't reproduce this issue locally, but it seems that
This exception seems to be on purpose and happens on main, so not related to this PR. |
Contributor
Author
Still trying to reproduce reliably. I got it to fail two or three times in thirty runs... Using both HALs available to me and another test machine. |
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
from
September 8, 2026 14:34
ec7442d to
c46e5f1
Compare
Contributor
Author
|
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
2 times, most recently
from
September 8, 2026 17:29
8232f5a to
f81e2f5
Compare
Contributor
Author
|
openQArun TEST=system_tests_dispvm,system_tests_basic_vm_qrexec_gui |
Contributor
Author
Contributor
Author
|
Review? Especially the experimental commits. Pending openqa tests don't seem relevant, only the completed ones. |
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
from
September 14, 2026 09:14
36faf42 to
23245e6
Compare
This error was uncaught because it was introduced at different commits and different merge requests, and each one was fixing an issue important to them, but the last one enabled "force", which should not clean up the named disposable if it was not for the first commit. - f1cbf21 - a82f4f2 The "force" parameter is not a good name for what it does. Remove it. And is not needed if the methods and events are ordered correctly and shared state runs within a lock. Introduce tests for cases where cleanup is called implicitly or explicitly: - Implicit: - Failed startup - Shutdown/Kill - Explicit: - Cleanup when not running - Cleanup when running Fixes: QubesOS/qubes-issues#11042 Fixes: QubesOS/qubes-issues#10928
When "DispVM.cleanup()" runs, it attempts to "QubesVM.kill()" the domain, but in the case the domain has started "QubesVM.start()", but the power state is still not running because it hasn't reached that stage, "QubesVM.kill" is skipped, the domain is deleted from the store, but when attempting to "QubesVM.remove_from_disk()", it fails, because at the same time this asynchronous task is running, the "QubesVM.start()" continues starting up the domain. In order to avoid this racy condition, always cancel the startup when stopping the domain is requested, even if not attempting to "libvirt_domain.destroy()", as the domain might indeed not be running yet, as the purpose of "kill()" is to not have a running domain, even on early start. When cancelling "start()", do not cancel exception handling, it might even be already at that stage, such as a "kill()" called from "start()". To avoid this issue, "asyncio.shield()" is used to prevent cancellation of select tasks.
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
2 times, most recently
from
September 14, 2026 09:33
c6934da to
1fc2c6d
Compare
Makes no sense to complain to clients that domain isn't running when kill is requested and the startup cancellation was done.
Instead of having several places to edit the logging format, use a single source of truth to define the format. This comes with the removal of logger from disposables integration tests, that were never needed at all, and they were duplicating/propagating the messages twice. Ideally, I'd just use the debug format, as that would help when collection logs from OpenQA or users that have not changed the log level.
With this patch, it avoids leaving domain cleanup for the test instance, avoiding certain events from being called twice.
When an unclean shutdown of a preloaded disposable happened after the qube was created on disk but before the startup completed, the qube was not saved to the store, so the script "cleanup-dispvms" can't find the disposable to delete on the next restart of "qubes-core.service". Save the domain to the store prior to the long startup so it can be deleted on the next reboot, in case there is an unclean shutdown. Although "create_on_disk()" takes some time, there doesn't seem to be a need to "app.save()" before it, as creating the domain object doesn't leave remnants on the system, until the qube is created on disk. Fixes: QubesOS/qubes-issues#11086
ben-grande
force-pushed
the
skip-cleaning-named-disp
branch
from
September 14, 2026 09:53
1fc2c6d to
7b29d64
Compare
Contributor
Author
|
PipelineRetryFailed |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See commit messages.
Fixes: QubesOS/qubes-issues#11042
Fixes: QubesOS/qubes-issues#10928
Tests locally with:
% cat ~/run-tests% ~/run-tests -o /dev/stdout -L INFO qubes.tests.integ.dispvm/TC_10_DispVM_Misc/test_