Reject wrong default_dispvm and management_dispvm#809
Merged
Conversation
ben-grande
force-pushed
the
validate-disp-tpl
branch
from
afbb24f to
bb2ea80
Compare
32 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #809 +/- ##
==========================================
+ Coverage 70.24% 70.43% +0.19%
==========================================
Files 61 61
Lines 14073 14139 +66
==========================================
+ Hits 9885 9959 +74
+ Misses 4188 4180 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced May 21, 2026
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026052509-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 tests23 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/176874#dependencies 26 fixed
Unstable testsDetails
Performance TestsPerformance degradation:16 performance degradations
Remaining performance tests:56 tests
|
This was referenced May 21, 2026
Add disk2 status and update volume retrieval method
QubesOS/qubes-desktop-linux-i3-settings-qubes#23
Open
marmarek
requested changes
May 22, 2026
| ) | ||
|
|
||
| mock_events.reset_mock() | ||
| self.app.default_dispvm = False |
Member
There was a problem hiding this comment.
Suggested change
| self.app.default_dispvm = False | |
| self.app.default_dispvm = None |
Comment on lines
+379
to
+395
| used_by_system = [] | ||
| if self == getattr(self.app, "default_dispvm", None): | ||
| used_by_system.append("default_dispvm") | ||
| if self == getattr(self.app, "management_dispvm", None): | ||
| used_by_system.append("management_dispvm") | ||
| if used_by_system: | ||
| msg = ( | ||
| "Cannot change template_for_dispvms to False while it is the " | ||
| "system's %s" % (", ".join(used_by_system)) | ||
| ) | ||
| self.log.error("%s", msg) | ||
| raise qubes.exc.QubesVMInUseError(self, msg) | ||
| used_by_qube: dict[str, list] = {} | ||
| for qube in self.app.domains: | ||
| for prop in ["default_dispvm", "management_dispvm"]: | ||
| if self == getattr(qube, prop, None): | ||
| used_by_qube.setdefault(str(qube), []).append(prop) |
Member
There was a problem hiding this comment.
This looks quite similar to qubes.on_domain_pre_deleted() handler. Maybe better extract that into a helper function and use in both places? That function would need to have a parameter for properties list, because here you want just template, management_dispvm and default_dispvm, but in remove case you want all (including netvm, guivm for example).
ben-grande
force-pushed
the
validate-disp-tpl
branch
3 times, most recently
from
010bec3 to
80cd0d2
Compare
marmarek
requested changes
May 25, 2026
Validation on pre-events is not called when using "app.add_new_vm()", a setter is necessary for those cases. Setter plus pre-set is unnecessary when validating only newvalue. Also prohibit removing "template_for_dispvms" when disposable template is referenced as a property by the system or any other qube, including itself. Fixes: QubesOS/qubes-issues#10881
ben-grande
force-pushed
the
validate-disp-tpl
branch
from
80cd0d2 to
95895a4
Compare
This was referenced May 25, 2026
Contributor
Author
|
PipelineRetry |
marmarek
approved these changes
May 26, 2026
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.
Validation on pre-events is not called when using "app.add_new_vm()", a
setter is necessary for those cases. Setter plus pre-set is unnecessary
when validating only newvalue.
Also prohibit removing template_for_dispvms when disposable template is referenced as a property by the system or any other qube, including itself.
Fixes: QubesOS/qubes-issues#10881