Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #387 +/- ##
==========================================
+ Coverage 76.16% 76.19% +0.03%
==========================================
Files 53 54 +1
Lines 9245 9273 +28
==========================================
+ Hits 7041 7066 +25
- Misses 2204 2207 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| running_vms = [ | ||
| vm.name | ||
| for vm in args.domains | ||
| if vm.get_power_state() == "Running" |
There was a problem hiding this comment.
Why only Running? What if I want to have a command that ensures that certain qubes are either running or with the latest state:
debian: open the qube and do some changeswork: state: Halted, template: debianpersonal: state: Running, template: debian
Hum, I will need to user work and personal now, let's make sure they are in their latest state:
qvm-shutdown --restart work personal
With the only running check, it would be a bit more longer:
qvm-shutdown --restart personal
qvm-start work
There was a problem hiding this comment.
Honestly, it's rather weird to use qvm-shutdown tool to start a qube in your example...
There was a problem hiding this comment.
Maybe it should be qvm-start --restart instead?
There was a problem hiding this comment.
Why only
Running?
To make qvm-shutdown --all --restart a possibility. To make it possible to restart all currently running qubes.
Maybe it should be
qvm-start --restartinstead?
After some feedback here and on Matrix channel, I believe a new dedicated qvm-restart tool is desirable.
@ArrayBolt3 and some other users had some good explanation on Matrix on why a dedicated tool is a good idea. I will amend this PR and come back.
There was a problem hiding this comment.
To make
qvm-shutdown --all --restarta possibility. To make it possible to restart all currently running qubes.
-Restart all running qubes is --all;
- If
--allis absent, shutdown and start or only start the specified qubes, being shutdown not enforced if the qube is Halted.
About other power states such as paused, I am not sure, possibly it shouldn't mess with those.
There was a problem hiding this comment.
@marmarek @ben-grande I rewrote a new dedicated qvm-restart tool among with manpage and unittests. I appreciate if you could review.
alimirjamali
force-pushed
the
qvm-shutdown-restart
branch
2 times, most recently
from
57d9023 to
1bfeb0c
Compare
|
|
||
| .. option:: --all | ||
|
|
||
| perform the action on all running qubes except dom0 & true DispVMs |
There was a problem hiding this comment.
Is there a better phrase than "true" DispVMs? Maybe "unnamed DispVMs" or "DispVMs with auto_cleanup enabled"?
There was a problem hiding this comment.
Is there a better phrase than "true" DispVMs? Maybe "unnamed DispVMs" or "DispVMs with
auto_cleanupenabled"?
You are right. I agree with "unnamed DispVMs". I will amend the PR accordingly (I should also link the related Github issue).
alimirjamali
force-pushed
the
qvm-shutdown-restart
branch
from
1bfeb0c to
dca546d
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
| @@ -0,0 +1,54 @@ | |||
| .. program:: qvm-restart | |||
There was a problem hiding this comment.
Man page is not installed, you need to add it to the list in doc/conf.py
| ] | ||
|
|
||
| # Forcing shutdown to allow graceful restart of ServiceVMs | ||
| shutdown_cmd = [vm.name for vm in target_domains] + ["--wait", "--force"] |
There was a problem hiding this comment.
Automatic --force is a bad idea. While user might want to use it when restarting sys-firewall for example, the flag will override also other checks - for example qube being used as audiovm or guivm, where restart might not be as seamless as with netvm.
| shutdown_cmd = [vm.name for vm in target_domains] + ["--wait", "--force"] | ||
| shutdown_cmd += ["--timeout", str(args.timeout)] if args.timeout else [] | ||
| shutdown_cmd += ["--quiet"] if args.quiet else [] | ||
| qvm_shutdown.main(shutdown_cmd, app=args.app) |
There was a problem hiding this comment.
I understand the convenience, but error handling here will be bad - if at least one qube fails to shutdown (and something fails when forcefully killing it on shutdown), no qube will be started again. Quite bad if one of restarted qubes was sys-usb, and you have USB keyboard...
Better to handle shutdown here directly, and then start the qubes that were powered off properly, and skip those that failed.
|
|
||
| start_cmd = [vm.name for vm in target_domains] | ||
| start_cmd += ["--quiet"] if args.quiet else [] | ||
| qvm_start.main(start_cmd, app=args.app) |
There was a problem hiding this comment.
Similarly to the above, error handling will be bad when doing it this way. qvm-start will stop at first startup failure. This might be okay when just starting qubes, but not really on restart.
|
ping? |
3 participants
fixes: QubesOS/qubes-issues#4747