Introduce a Stopping state

include/multipass/virtual_machine.h

@@ -55,15 +55,16 @@ class VirtualMachine : private DisabledCopyMove
         delayed_shutdown,
         suspending,
         suspended,
-        unknown
+        unknown,
+        stopping // weird order preserves backward compatibility with persisted states (this was added later)
     };
 
     using UPtr = std::unique_ptr<VirtualMachine>;
     using ShPtr = std::shared_ptr<VirtualMachine>;
 
     virtual ~VirtualMachine() = default;
     virtual void start() = 0;
-    virtual void shutdown() = 0;
+    virtual void shutdown(const bool force = false) = 0;
     virtual void suspend() = 0;
     virtual State current_state() = 0;
     virtual int ssh_port() = 0;

src/client/cli/cmd/stop.cpp

@@ -84,7 +84,8 @@ mp::ParseCode cmd::Stop::parse_args(mp::ArgParser* parser)
     QCommandLineOption time_option({"t", "time"}, "Time from now, in minutes, to delay shutdown of the instance",
                                    "time", "0");
     QCommandLineOption cancel_option({"c", "cancel"}, "Cancel a pending delayed shutdown");
-    parser->addOptions({all_option, time_option, cancel_option});
+    QCommandLineOption force_option("force", "Force switching the instance off");
+    parser->addOptions({all_option, time_option, cancel_option, force_option});
 
     auto status = parser->commandParse(this);
     if (status != ParseCode::Ok)
@@ -105,6 +106,14 @@ mp::ParseCode cmd::Stop::parse_args(mp::ArgParser* parser)
         return ParseCode::CommandLineError;
     }
 
+    if (parser->isSet(force_option) && (parser->isSet(time_option) || parser->isSet(cancel_option)))
+    {
+        cerr << "Cannot set \'force\' along with \'time\' or \'cancel\' options at the same time\n";
+        return ParseCode::CommandLineError;
+    }
+
+    request.set_force_stop(parser->isSet(force_option));
+
     auto time = parser->value(time_option);
 
     if (time.startsWith("+"))

src/client/cli/formatter/format_utils.cpp

@@ -71,6 +71,9 @@ std::string mp::format::status_string_for(const mp::InstanceStatus& status)
     case mp::InstanceStatus::RESTARTING:
         status_val = "Restarting";
         break;
+    case mp::InstanceStatus::STOPPING:
+        status_val = "Stopping";
+        break;
     case mp::InstanceStatus::DELAYED_SHUTDOWN:
         status_val = "Delayed Shutdown";
         break;

src/daemon/daemon.cpp

@@ -970,6 +970,8 @@ mp::InstanceStatus::Status grpc_instance_status_for(const mp::VirtualMachine::St
     case mp::VirtualMachine::State::off:
     case mp::VirtualMachine::State::stopped:
         return mp::InstanceStatus::STOPPED;
+    case mp::VirtualMachine::State::stopping:
+        return mp::InstanceStatus::STOPPING;
     case mp::VirtualMachine::State::starting:
         return mp::InstanceStatus::STARTING;
     case mp::VirtualMachine::State::restarting:
@@ -2109,7 +2111,8 @@ try // clang-format on
         std::lock_guard lock{start_mutex};
         const auto& name = vm_it->first;
         auto& vm = *vm_it->second;
-        switch (vm.current_state())
+        auto state = vm.current_state();
+        switch (state)
         {
         case VirtualMachine::State::unknown:
         {
@@ -2118,9 +2121,16 @@ try // clang-format on
             fmt::format_to(std::back_inserter(start_errors), error_string);
             continue;
         }
+        case VirtualMachine::State::stopping:
         case VirtualMachine::State::suspending:
-            fmt::format_to(std::back_inserter(start_errors), "Cannot start the instance \'{}\' while suspending", name);
+        {
+            auto state_str = state == VirtualMachine::State::stopping ? "stopping" : "suspending";
+            fmt::format_to(std::back_inserter(start_errors),
+                           "Cannot start the instance \'{}\' while {}",
+                           name,
+                           state_str);
             continue;
+        }
         case VirtualMachine::State::delayed_shutdown:
             delayed_shutdown_instances.erase(name);
             continue;
@@ -2176,6 +2186,8 @@ try // clang-format on
         std::function<grpc::Status(VirtualMachine&)> operation;
         if (request->cancel_shutdown())
             operation = std::bind(&Daemon::cancel_vm_shutdown, this, std::placeholders::_1);
+        else if (request->force_stop())
+            operation = std::bind(&Daemon::switch_off_vm, this, std::placeholders::_1);
         else
             operation = std::bind(&Daemon::shutdown_vm, this, std::placeholders::_1,
                                   std::chrono::minutes(request->time_minutes()));
@@ -3114,7 +3126,7 @@ bool mp::Daemon::delete_vm(InstanceTable::iterator vm_it, bool purge, DeleteRepl
             delayed_shutdown_instances.erase(name);
 
         mounts[name].clear();
-        instance->shutdown();
+        instance->shutdown(purge);
 
         if (!purge)
         {
@@ -3148,7 +3160,14 @@ grpc::Status mp::Daemon::reboot_vm(VirtualMachine& vm)
     if (vm.state == VirtualMachine::State::delayed_shutdown)
         delayed_shutdown_instances.erase(vm.vm_name);
 
-    if (!MP_UTILS.is_running(vm.current_state()))
+    // TODO@no-merge streamline this stuff
+    auto state = vm.current_state();
+    if (state == VirtualMachine::State::stopping)
+        return grpc::Status{grpc::StatusCode::INVALID_ARGUMENT,
+                            fmt::format("instance \"{}\" is currently stopping", vm.vm_name),
+                            ""};
+
+    if (!MP_UTILS.is_running(state))
         return grpc::Status{grpc::StatusCode::INVALID_ARGUMENT,
                             fmt::format("instance \"{}\" is not running", vm.vm_name), ""};
 
@@ -3162,7 +3181,7 @@ grpc::Status mp::Daemon::shutdown_vm(VirtualMachine& vm, const std::chrono::mill
     const auto& state = vm.current_state();
 
     using St = VirtualMachine::State;
-    const auto skip_states = {St::off, St::stopped, St::suspended};
+    const auto skip_states = {St::off, St::stopped, St::suspended, St::stopping};
 
     if (std::none_of(cbegin(skip_states), cend(skip_states), [&state](const auto& st) { return state == st; }))
     {
@@ -3183,6 +3202,26 @@ grpc::Status mp::Daemon::shutdown_vm(VirtualMachine& vm, const std::chrono::mill
     return grpc::Status::OK;
 }
 
+grpc::Status mp::Daemon::switch_off_vm(VirtualMachine& vm)
+{
+    const auto& name = vm.vm_name;
+    const auto& state = vm.current_state();
+
+    using St = VirtualMachine::State;
+    const auto skip_states = {St::off, St::stopped};
+
+    if (std::none_of(cbegin(skip_states), cend(skip_states), [&state](const auto& st) { return state == st; }))
+    {
+        delayed_shutdown_instances.erase(name);
+
+        vm.shutdown(true);
+    }
+    else
+        mpl::log(mpl::Level::debug, category, fmt::format("instance \"{}\" does not need stopping", name));
+
+    return grpc::Status::OK;
+}
+
 grpc::Status mp::Daemon::cancel_vm_shutdown(const VirtualMachine& vm)
 {
     auto it = delayed_shutdown_instances.find(vm.vm_name);
@@ -3198,10 +3237,16 @@ grpc::Status mp::Daemon::cancel_vm_shutdown(const VirtualMachine& vm)
 grpc::Status mp::Daemon::get_ssh_info_for_vm(VirtualMachine& vm, SSHInfoReply& response)
 {
     const auto& name = vm.vm_name;
-    if (vm.current_state() == VirtualMachine::State::unknown)
+    auto state = vm.current_state();
+
+    // TODO@no-merge streamline this stuff
+    if (state == VirtualMachine::State::unknown)
         throw std::runtime_error("Cannot retrieve credentials in unknown state");
 
-    if (!MP_UTILS.is_running(vm.current_state()))
+    if (state == VirtualMachine::State::stopping)
+        return grpc::Status{grpc::StatusCode::ABORTED, fmt::format("instance \"{}\" is stopping", name)};
+
+    if (!MP_UTILS.is_running(state))
         return grpc::Status{grpc::StatusCode::ABORTED, fmt::format("instance \"{}\" is not running", name)};
 
     if (vm.state == VirtualMachine::State::delayed_shutdown &&

src/daemon/daemon.h

@@ -156,6 +156,7 @@ public slots:
     bool delete_vm(InstanceTable::iterator vm_it, bool purge, DeleteReply& response);
     grpc::Status reboot_vm(VirtualMachine& vm);
     grpc::Status shutdown_vm(VirtualMachine& vm, const std::chrono::milliseconds delay);
+    grpc::Status switch_off_vm(VirtualMachine& vm);
     grpc::Status cancel_vm_shutdown(const VirtualMachine& vm);
     grpc::Status get_ssh_info_for_vm(VirtualMachine& vm, SSHInfoReply& response);
 

src/daemon/delayed_shutdown_timer.cpp

@@ -76,7 +76,13 @@ mp::DelayedShutdownTimer::~DelayedShutdownTimer()
         {
             shutdown_timer.stop();
             mpl::log(mpl::Level::info, virtual_machine->vm_name, "Cancelling delayed shutdown");
-            virtual_machine->state = VirtualMachine::State::running;
+
+            {
+                std::lock_guard lock{virtual_machine->state_mutex};
+                if (virtual_machine->state == VirtualMachine::State::delayed_shutdown)
+                    virtual_machine->state = VirtualMachine::State::running;
+            }
+
             attempt_ssh_exec(*virtual_machine, "wall The system shutdown has been cancelled");
         }
     });

src/platform/backends/libvirt/libvirt_virtual_machine.cpp

@@ -209,19 +209,26 @@ auto refresh_instance_state_for_domain(virDomainPtr domain, const mp::VirtualMac
         domain_state == VIR_DOMAIN_NOSTATE)
         return mp::VirtualMachine::State::unknown;
 
+    if (domain_state == VIR_DOMAIN_SHUTDOWN)
+        return mp::VirtualMachine::State::stopping;
+
     if (libvirt_wrapper->virDomainHasManagedSaveImage(domain, 0) == 1)
         return mp::VirtualMachine::State::suspended;
 
     // Most of these libvirt domain states don't have a Multipass instance state
     // analogue, so we'll treat them as "off".
-    const auto domain_off_states = {VIR_DOMAIN_BLOCKED, VIR_DOMAIN_PAUSED,  VIR_DOMAIN_SHUTDOWN,
-                                    VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_CRASHED, VIR_DOMAIN_PMSUSPENDED};
+    const auto domain_off_states = {
+        VIR_DOMAIN_BLOCKED,
+        VIR_DOMAIN_PAUSED,
+        VIR_DOMAIN_SHUTOFF,
+        VIR_DOMAIN_CRASHED,
+        VIR_DOMAIN_PMSUSPENDED}; // TODO shouldn't we treat PMSUSPENDED as suspended? and maybe PAUSED as well?
 
     if (std::find(domain_off_states.begin(), domain_off_states.end(), domain_state) != domain_off_states.end())
         return mp::VirtualMachine::State::off;
 
-    if (domain_state == VIR_DOMAIN_RUNNING && current_instance_state == mp::VirtualMachine::State::off)
-        return mp::VirtualMachine::State::running;
+    if (domain_state == VIR_DOMAIN_RUNNING && current_instance_state == mp::VirtualMachine::State::stopping)
+        return mp::VirtualMachine::State::stopping;
 
     return current_instance_state;
 }
@@ -299,7 +306,7 @@ mp::LibVirtVirtualMachine::~LibVirtVirtualMachine()
     update_suspend_status = false;
 
     if (state == State::running)
-        suspend();
+        suspend(); // TODO this can throw!
 }
 
 void mp::LibVirtVirtualMachine::start()
@@ -313,9 +320,10 @@ void mp::LibVirtVirtualMachine::start()
         domain = domain_by_name_for(vm_name, connection.get(), libvirt_wrapper);
 
     state = refresh_instance_state_for_domain(domain.get(), state, libvirt_wrapper);
-
     if (state == State::suspended)
         mpl::log(mpl::Level::info, vm_name, fmt::format("Resuming from a suspended state"));
+    else if (state == State::stopping)
+        throw std::runtime_error{fmt::format("Cannot start {} while it is stopping", vm_name)};
 
     state = State::starting;
     update_state();
@@ -345,38 +353,51 @@ void mp::LibVirtVirtualMachine::start()
     monitor->on_resume();
 }
 
-void mp::LibVirtVirtualMachine::shutdown()
+void mp::LibVirtVirtualMachine::shutdown(const bool force)
 {
     std::unique_lock<decltype(state_mutex)> lock{state_mutex};
     auto domain = domain_by_name_for(vm_name, open_libvirt_connection(libvirt_wrapper).get(), libvirt_wrapper);
-    state = refresh_instance_state_for_domain(domain.get(), state, libvirt_wrapper);
-    if (state == State::suspended)
+
+    if (force)
     {
-        mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended"));
+        mpl::log(mpl::Level::info, vm_name, "Forced shutdown");
+
+        auto domain = domain_by_name_for(vm_name, open_libvirt_connection(libvirt_wrapper).get(), libvirt_wrapper);
+
+        libvirt_wrapper->virDomainDestroy(domain.get());
+
+        state = State::off;
+        update_state();
     }
     else
     {
-        drop_ssh_session();
-
+        state = refresh_instance_state_for_domain(domain.get(), state, libvirt_wrapper);
         if (state == State::running || state == State::delayed_shutdown || state == State::unknown)
         {
+            state = State::stopping;
+            update_state();
+
+            drop_ssh_session();
+
             if (!domain || libvirt_wrapper->virDomainShutdown(domain.get()) == -1)
             {
                 auto warning_string{
                     fmt::format("Cannot shutdown '{}': {}", vm_name, libvirt_wrapper->virGetLastErrorMessage())};
                 mpl::log(mpl::Level::warning, vm_name, warning_string);
                 throw std::runtime_error(warning_string);
             }
-
-            state = State::off;
-            update_state();
         }
         else if (state == State::starting)
         {
             libvirt_wrapper->virDomainDestroy(domain.get());
             state_wait.wait(lock, [this] { return shutdown_while_starting; });
             update_state();
         }
+        else if (state == State::suspended || state == State::stopping)
+        {
+            auto state_str = state == State::suspended ? "suspended" : "stopping";
+            mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while {}", state_str));
+        }
     }
 
     lock.unlock();
@@ -404,9 +425,10 @@ void mp::LibVirtVirtualMachine::suspend()
             update_state();
         }
     }
-    else if (state == State::off)
+    else if (state == State::off || state == State::stopped || state == State::stopping)
     {
-        mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring suspend issued while stopped"));
+        auto state_str = state == State::stopping ? "stopping" : "stopped";
+        mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring suspend issued while {}", state_str));
     }
 
     monitor->on_suspend();

src/platform/backends/libvirt/libvirt_virtual_machine.h

@@ -44,7 +44,7 @@ class LibVirtVirtualMachine final : public BaseVirtualMachine
     ~LibVirtVirtualMachine();
 
     void start() override;
-    void shutdown() override;
+    void shutdown(const bool force = false) override;
     void suspend() override;
     State current_state() override;
     int ssh_port() override;