Use wall for delayed shutdown #501
Conversation
When 1 minute or less remains in delayed shutdown, block shell/exec from the client. Fixes #485
Codecov Report
@@ Coverage Diff @@
## master #501 +/- ##
==========================================
- Coverage 66.48% 66.45% -0.04%
==========================================
Files 146 146
Lines 5646 5676 +30
==========================================
+ Hits 3754 3772 +18
- Misses 1892 1904 +12
Continue to review full report at Codecov.
|
src/daemon/daemon.cpp
Outdated
| { | ||
| return grpc::Status( | ||
| grpc::StatusCode::FAILED_PRECONDITION, | ||
| fmt::format("\"{}\" is in a delayed shutdown. Use 'multipass stop -c {}' to cancel.", name, name), |
There was a problem hiding this comment.
I'd word this more like:
{} is scheduled to shut down in less than a minute,
usemultipass stop --cancel {}to cancel the shutdown.
| mp::DelayedShutdownTimer::DelayedShutdownTimer(VirtualMachine* virtual_machine) : virtual_machine{virtual_machine} | ||
| namespace | ||
| { | ||
| std::string shutdown_message(const std::chrono::minutes& time_left) |
There was a problem hiding this comment.
I wouldn't expect the returned value to include the wall .
There was a problem hiding this comment.
Ok, but I did this to keep from having to add wall to every session.exec() call we make. Would renaming the function make it more clear? Maybe something like shutdown_message_command() or something?
There was a problem hiding this comment.
Yeah, that works too:)
| std::chrono::minutes time_elapsed{1}; | ||
| QObject::connect(&shutdown_timer, &QTimer::timeout, [this, delay, time_elapsed]() mutable { | ||
| time_remaining = delay - time_elapsed; | ||
| ssh_session.exec(shutdown_message(std::chrono::duration_cast<std::chrono::minutes>(time_remaining))); |
There was a problem hiding this comment.
Wonder if we should not spam every minute? Maybe once when first scheduled, then at 5 minutes, only then every minute?
There was a problem hiding this comment.
The shutdown command itself sends a message every minute when the time is set to 15 minutes or less. Any time over that and you don't see a message at all, even when the time remaining dips below 15 minutes (which seems strange).
But we could be more nice on times greater than 5 minutes as not to peeve the user:)
There was a problem hiding this comment.
Yeah we don't have to be that inspired by shutdown ;)
Also, I wonder if we should hint as to what can be done about the scheduled shutdown in the wall messages, too?
There was a problem hiding this comment.
This should be better now.
Be more nice about how often the shutdown message is written in the user's terminal. Change wording of the shutdown messages.
|
Yup, all seems fine. bors r+ |
|
👎 Rejected by code reviews |
|
bors r+ |
501: Use wall for delayed shutdown r=Saviq a=townsend2010 Co-authored-by: Chris Townsend <[email protected]>
Build succeeded |
551: Release 2018.12.1 r=townsend2010 a=Saviq ### Highlights - On Linux, suspending/resuming the instance to/from disk is now supported. (#374) - Better handling of delayed shutdown including posting `wall` messages to logged in users and allowing log ins to the instance unless 1 minute or less remains until shutdown. (#461, #50) - On Linux, all CPU flags should be passed into the running instance on newly created instances. (#516) - Fixed some races around mount handling. (#514, #520) ### Bugs fixed: - make the recover command idempotent (#528) - explicitly stop mounts when deleting an instance to avoid a race (#520) - be smarter about what group owns the multipass socket (#513, #523) - pass through all CPU flags when launching QEMU or libvirt instances (#516) - use `info` log level for metrics issues (#515) - fix potential race when starting a mount (#514) - use `wall` shutdown messages for users logged into VM when delayed shutdown is initiated (#501) - fix crash if exception during daemon start up (#487) - refactor CLI code (#468) - add default uid/gid mapping (#331) - fix file metadata passthrough - display uid/gid maps in info command (#439) - add support for the suspend command (#374) - shell to machine in delayed stop state (#461) - improve uid/gid validation (#479) - avoid leaking the libvirt bridge (#327, #413) - add a restart command (#217) - upgrade 3rd-party versions (#471) Co-authored-by: bors[bot] <bors[bot]@users.noreply.github.com>
No description provided.