qui-domains: adapt shutdown dialog to exception type#308
Conversation
|
@marmarek |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #308 +/- ##
=======================================
Coverage 92.99% 92.99%
=======================================
Files 64 64
Lines 13312 13313 +1
=======================================
+ Hits 12379 12380 +1
Misses 933 933 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| self.vm.shutdown(force=True) | ||
| elif action == "retry": | ||
| self.vm.shutdown(force=False) | ||
| else: |
There was a problem hiding this comment.
It would be safer if this would also check for kill explicitly, to avoid killing a qube just because somebody made a typo elsewhere in this file.
Jayant-kernel
force-pushed
the
fix-shutdown-dialog-exception-10650
branch
from
99eaaf1 to
66ea33e
Compare
|
I think that an option to kill the VM should also be available if timeout failed? Especially if timeout failed multiple times, it might suggest something is deeply wrong and killing is the correct behavior. (This sounds very bad out of context :D) |
|
@marmarta |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026040702-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026032404-devel&flavor=update
Failed tests20 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/170766#dependencies 28 fixed
Unstable testsDetailsPerformance TestsPerformance degradation:10 performance degradations
Remaining performance tests:101 tests
|
- Show tailored dialog for QubesVMInUseError, QubesVMShutdownTimeoutError and generic QubesException - Offer Retry + Kill buttons when shutdown times out - Destroy dialog on Cancel to prevent dialog leak - Check response type explicitly before calling kill()
Jayant-kernel
force-pushed
the
fix-shutdown-dialog-exception-10650
branch
from
7fbc56f to
c2110b7
Compare
marmarek
left a comment
marmarek
left a comment
There was a problem hiding this comment.
The changes to exception handling are okay. Enforcing the timeout is broken independently (QubesOS/qubes-issues#10835), but that's for another PR.
|
This PR missed other places where |
|
Also, the buttons of the submenu should be changed to be the same as the dialog. If dialog recommends "Kill" and "Force shutdown", buttons should be adapted to it. Possibly not hiding "Shutdown", as the qube might become responsive again. |
5 participants
The shutdown error dialog always showed "Do you want to force shutdown?" regardless of which exception was raised. A
QubesVMShutdownTimeoutErrorwould offer force shutdown even though retrying normal shutdown is more appropriate, and any other error would not offer kill, which is the correct last-resort action.Adapt the dialog text, title, and action button based on the exception type:
QubesVMInUseError: warn about connected qubes, offer Force shutdown (vm.shutdown(force=True))QubesVMShutdownTimeoutError: note the timeout, offer Retry shutdown (vm.shutdown(force=False))QubesException: show the error, offer Kill (vm.kill())Fixes: QubesOS/qubes-issues#10650