From 4e8fd7dace64dd1f35c5edd30330c844966d7d69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 29 Jan 2021 12:10:13 -0300 Subject: [PATCH 01/49] [rpc] Add option to force stopping an instance. --- src/rpc/multipass.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index 8718194c55..e1eaa4b964 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -395,6 +395,7 @@ message StopRequest { int32 time_minutes = 2; bool cancel_shutdown = 3; int32 verbosity_level = 4; + bool force_stop = 5; } message StopReply { From 862802bca44e560cead8e0e87d3cc16050d8685a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 29 Jan 2021 12:10:51 -0300 Subject: [PATCH 02/49] [vm] Add bool to force shutdown. --- include/multipass/virtual_machine.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index 365e26be77..537fc4b27c 100644 --- a/include/multipass/virtual_machine.h +++ b/include/multipass/virtual_machine.h @@ -63,7 +63,7 @@ class VirtualMachine : private DisabledCopyMove virtual ~VirtualMachine() = default; virtual void start() = 0; - virtual void shutdown() = 0; + virtual void shutdown(bool force = false) = 0; virtual void suspend() = 0; virtual State current_state() = 0; virtual int ssh_port() = 0; From 992cfc86c1ea89d8c7f5680a3254fb6cd4002094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 29 Jan 2021 12:11:39 -0300 Subject: [PATCH 03/49] [cli] Handle command. --- src/client/cli/cmd/stop.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/client/cli/cmd/stop.cpp b/src/client/cli/cmd/stop.cpp index 534ffc65ce..db3947a7fd 100644 --- a/src/client/cli/cmd/stop.cpp +++ b/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({"f", "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,12 @@ 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; + } + auto time = parser->value(time_option); if (time.startsWith("+")) From f79ed519c98863db4a7a9cc8ef7decefcbb0024e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 29 Jan 2021 12:12:16 -0300 Subject: [PATCH 04/49] [tests] Adapt stub and mock to new vm interface. --- tests/mock_virtual_machine.h | 2 +- tests/stub_virtual_machine.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mock_virtual_machine.h b/tests/mock_virtual_machine.h index f9607b87fe..5fde7a99b4 100644 --- a/tests/mock_virtual_machine.h +++ b/tests/mock_virtual_machine.h @@ -56,7 +56,7 @@ struct MockVirtualMachineT : public T } MOCK_METHOD(void, start, (), (override)); - MOCK_METHOD(void, shutdown, (), (override)); + MOCK_METHOD(void, shutdown, (bool), (override)); MOCK_METHOD(void, suspend, (), (override)); MOCK_METHOD(multipass::VirtualMachine::State, current_state, (), (override)); MOCK_METHOD(int, ssh_port, (), (override)); diff --git a/tests/stub_virtual_machine.h b/tests/stub_virtual_machine.h index d9f388a245..0bd5c8f760 100644 --- a/tests/stub_virtual_machine.h +++ b/tests/stub_virtual_machine.h @@ -47,7 +47,7 @@ struct StubVirtualMachine final : public multipass::VirtualMachine { } - void shutdown() override + void shutdown(bool force = false) override { } From 2d698540db92368066a2c6aa2e349051ff50b19a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 28 May 2021 15:53:22 -0300 Subject: [PATCH 05/49] [daemon] Handle forced stop command --- src/daemon/daemon.cpp | 24 +++++++++++++++++++++++- src/daemon/daemon.h | 1 + 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index aa0d99fd93..11ef416d74 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -2176,6 +2176,8 @@ try // clang-format on std::function 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 +3116,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) { @@ -3183,6 +3185,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); diff --git a/src/daemon/daemon.h b/src/daemon/daemon.h index 9106e9be19..346a056b5d 100644 --- a/src/daemon/daemon.h +++ b/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); From ea86feaf46d978ba823ef02e6a9caebdbf95dc8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 28 May 2021 15:55:07 -0300 Subject: [PATCH 06/49] [tests] Use the new virtual_machine.h interface. --- tests/test_base_virtual_machine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index ccec073219..cde5e89bf3 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -131,7 +131,7 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine state = St::running; } - void shutdown() override + void shutdown(bool force = false) override { state = St::off; } From 04edcc35ae38e8426d5c9dc550854a728b5e1c82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 28 May 2021 15:56:13 -0300 Subject: [PATCH 07/49] [qemu] Implement forced stop. --- .../backends/qemu/qemu_virtual_machine.cpp | 24 ++++++++++++++++++- .../backends/qemu/qemu_virtual_machine.h | 2 +- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 5fbdd65723..2abd2d70a8 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -343,8 +343,30 @@ void mp::QemuVirtualMachine::start() vm_process->write(qmp_execute_json("qmp_capabilities")); } -void mp::QemuVirtualMachine::shutdown() +void mp::QemuVirtualMachine::shutdown(bool force) { + if (force) + { + state = State::off; + mpl::log(mpl::Level::info, vm_name, "Forced shutdown"); + + if (vm_process) + { + mpl::log(mpl::Level::info, vm_name, "Killing process"); + try + { + vm_process->kill(); + } + catch (std::exception&) + { + } + } + else + mpl::log(mpl::Level::info, vm_name, "No process to kill"); + + return; + } + if (state == State::suspended) { mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h index f0f409ac68..3a7ae72d39 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.h +++ b/src/platform/backends/qemu/qemu_virtual_machine.h @@ -50,7 +50,7 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine ~QemuVirtualMachine(); void start() override; - void shutdown() override; + void shutdown(bool force = false) override; void suspend() override; State current_state() override; int ssh_port() override; From c9579e7965eecdc1aae4965fd41475d73dd0db79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 28 May 2021 15:57:37 -0300 Subject: [PATCH 08/49] [lxd] Implement forced stop. --- .../backends/lxd/lxd_virtual_machine.cpp | 29 ++++++++++++++++++- .../backends/lxd/lxd_virtual_machine.h | 3 +- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index af9a4ff649..c3d96b54d4 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -261,8 +261,35 @@ void mp::LXDVirtualMachine::start() update_state(); } -void mp::LXDVirtualMachine::shutdown() +void mp::LXDVirtualMachine::shutdown(bool force) { + if (force) + { + auto present_state = current_state(); + + if (present_state != State::stopped && present_state != State::off) + { + const QJsonObject state_json{{"action", "stop"}, {"force", true}}; + + auto state_task = lxd_request(manager, "PUT", state_url(), state_json, 5000); + + try + { + lxd_wait(manager, base_url, state_task, 60000); + } + catch (const LXDNotFoundException&) + { + } + + state = State::stopped; + } + + if (update_shutdown_status) + update_state(); + + return; + } + std::unique_lock lock{state_mutex}; auto present_state = current_state(); diff --git a/src/platform/backends/lxd/lxd_virtual_machine.h b/src/platform/backends/lxd/lxd_virtual_machine.h index cc93fbd07f..104495f584 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.h +++ b/src/platform/backends/lxd/lxd_virtual_machine.h @@ -41,8 +41,9 @@ class LXDVirtualMachine : public BaseVirtualMachine const SSHKeyProvider& key_provider, const Path& instance_dir); ~LXDVirtualMachine() override; + void start() override; - void shutdown() override; + void shutdown(bool force = false) override; void suspend() override; State current_state() override; int ssh_port() override; From 48846e6701147074717525e92f3ed6daef8f374b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pe=C3=B1aranda?= Date: Fri, 28 May 2021 15:57:51 -0300 Subject: [PATCH 09/49] [libvirt] Implement forced stop. --- .../libvirt/libvirt_virtual_machine.cpp | 24 ++++++++++++++----- .../libvirt/libvirt_virtual_machine.h | 2 +- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index 53a1d8e390..76faef0263 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -345,21 +345,29 @@ void mp::LibVirtVirtualMachine::start() monitor->on_resume(); } -void mp::LibVirtVirtualMachine::shutdown() +void mp::LibVirtVirtualMachine::shutdown(bool force) { std::unique_lock 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) { + drop_ssh_session(); + if (!domain || libvirt_wrapper->virDomainShutdown(domain.get()) == -1) { auto warning_string{ @@ -377,6 +385,10 @@ void mp::LibVirtVirtualMachine::shutdown() state_wait.wait(lock, [this] { return shutdown_while_starting; }); update_state(); } + else if (state == State::suspended) + { + mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); + } } lock.unlock(); diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.h b/src/platform/backends/libvirt/libvirt_virtual_machine.h index 8b40de2664..f31949f537 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.h +++ b/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(bool force = false) override; void suspend() override; State current_state() override; int ssh_port() override; From dce9c9f91c8bfa6b6483e9f4ee6bbcf5f3e36896 Mon Sep 17 00:00:00 2001 From: sharder996 Date: Fri, 19 Aug 2022 10:51:31 -0500 Subject: [PATCH 10/49] refactor with main --- src/client/cli/cmd/stop.cpp | 2 + .../backends/lxd/lxd_virtual_machine.cpp | 40 ++++--------------- .../backends/lxd/lxd_virtual_machine.h | 2 +- .../backends/qemu/qemu_virtual_machine.cpp | 15 ++----- 4 files changed, 15 insertions(+), 44 deletions(-) diff --git a/src/client/cli/cmd/stop.cpp b/src/client/cli/cmd/stop.cpp index db3947a7fd..e0740da17d 100644 --- a/src/client/cli/cmd/stop.cpp +++ b/src/client/cli/cmd/stop.cpp @@ -112,6 +112,8 @@ mp::ParseCode cmd::Stop::parse_args(mp::ArgParser* parser) return ParseCode::CommandLineError; } + request.set_force_stop(parser->isSet(force_option)); + auto time = parser->value(time_option); if (time.startsWith("+")) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index c3d96b54d4..56d17bb7da 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -263,33 +263,6 @@ void mp::LXDVirtualMachine::start() void mp::LXDVirtualMachine::shutdown(bool force) { - if (force) - { - auto present_state = current_state(); - - if (present_state != State::stopped && present_state != State::off) - { - const QJsonObject state_json{{"action", "stop"}, {"force", true}}; - - auto state_task = lxd_request(manager, "PUT", state_url(), state_json, 5000); - - try - { - lxd_wait(manager, base_url, state_task, 60000); - } - catch (const LXDNotFoundException&) - { - } - - state = State::stopped; - } - - if (update_shutdown_status) - update_state(); - - return; - } - std::unique_lock lock{state_mutex}; auto present_state = current_state(); @@ -298,14 +271,13 @@ void mp::LXDVirtualMachine::shutdown(bool force) mpl::log(mpl::Level::debug, vm_name, "Ignoring stop request since instance is already stopped"); return; } - - if (present_state == State::suspended) + else if (present_state == State::suspended && !force) { mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); return; } - request_state("stop"); + request_state("stop", {{"force", force}}); state = State::stopped; @@ -433,9 +405,13 @@ const QUrl mp::LXDVirtualMachine::network_leases_url() return base_url.toString() + "/networks/" + bridge_name + "/leases"; } -void mp::LXDVirtualMachine::request_state(const QString& new_state) +void mp::LXDVirtualMachine::request_state(const QString& new_state, const QJsonObject args) { - const QJsonObject state_json{{"action", new_state}}; + QJsonObject state_json{{"action", new_state}}; + for (auto it = args.constBegin(); it != args.constEnd(); it++) + { + state_json.insert(it.key(), it.value()); + } auto state_task = lxd_request(manager, "PUT", state_url(), state_json, 5000); diff --git a/src/platform/backends/lxd/lxd_virtual_machine.h b/src/platform/backends/lxd/lxd_virtual_machine.h index 104495f584..312f17b018 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.h +++ b/src/platform/backends/lxd/lxd_virtual_machine.h @@ -77,7 +77,7 @@ class LXDVirtualMachine : public BaseVirtualMachine const QUrl url(); const QUrl state_url(); const QUrl network_leases_url(); - void request_state(const QString& new_state); + void request_state(const QString& new_state, const QJsonObject args = {}); }; } // namespace multipass #endif // MULTIPASS_LXD_VIRTUAL_MACHINE_H diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 2abd2d70a8..87035d5743 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -353,21 +353,14 @@ void mp::QemuVirtualMachine::shutdown(bool force) if (vm_process) { mpl::log(mpl::Level::info, vm_name, "Killing process"); - try - { - vm_process->kill(); - } - catch (std::exception&) - { - } + vm_process->kill(); } else + { mpl::log(mpl::Level::info, vm_name, "No process to kill"); - - return; + } } - - if (state == State::suspended) + else if (state == State::suspended) { mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); } From 99f93681f6a60f81bc4ee4ffc1a691ad63c5fc0c Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Thu, 21 Mar 2024 16:15:39 -0400 Subject: [PATCH 11/49] [cli/stop] Make force a long option only This will make it explicit that the user wants to force stop an instance. --- src/client/cli/cmd/stop.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/cli/cmd/stop.cpp b/src/client/cli/cmd/stop.cpp index e0740da17d..984201d2d4 100644 --- a/src/client/cli/cmd/stop.cpp +++ b/src/client/cli/cmd/stop.cpp @@ -84,7 +84,7 @@ 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"); - QCommandLineOption force_option({"f", "force"}, "Force switching the instance off"); + QCommandLineOption force_option("force", "Force switching the instance off"); parser->addOptions({all_option, time_option, cancel_option, force_option}); auto status = parser->commandParse(this); From 3cca6dbcb0a3fa3b21b14a9d79fcc34f89d5a56d Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Tue, 26 Mar 2024 15:13:53 -0400 Subject: [PATCH 12/49] [virtual_machine] Make force shutdown bool const --- include/multipass/virtual_machine.h | 2 +- src/platform/backends/libvirt/libvirt_virtual_machine.cpp | 2 +- src/platform/backends/libvirt/libvirt_virtual_machine.h | 2 +- src/platform/backends/lxd/lxd_virtual_machine.cpp | 2 +- src/platform/backends/lxd/lxd_virtual_machine.h | 2 +- src/platform/backends/qemu/qemu_virtual_machine.cpp | 2 +- src/platform/backends/qemu/qemu_virtual_machine.h | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index 537fc4b27c..3279b7d700 100644 --- a/include/multipass/virtual_machine.h +++ b/include/multipass/virtual_machine.h @@ -63,7 +63,7 @@ class VirtualMachine : private DisabledCopyMove virtual ~VirtualMachine() = default; virtual void start() = 0; - virtual void shutdown(bool force = false) = 0; + virtual void shutdown(const bool force = false) = 0; virtual void suspend() = 0; virtual State current_state() = 0; virtual int ssh_port() = 0; diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index 76faef0263..6972d3422b 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -345,7 +345,7 @@ void mp::LibVirtVirtualMachine::start() monitor->on_resume(); } -void mp::LibVirtVirtualMachine::shutdown(bool force) +void mp::LibVirtVirtualMachine::shutdown(const bool force) { std::unique_lock lock{state_mutex}; auto domain = domain_by_name_for(vm_name, open_libvirt_connection(libvirt_wrapper).get(), libvirt_wrapper); diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.h b/src/platform/backends/libvirt/libvirt_virtual_machine.h index f31949f537..f3cdb7de8d 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.h +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.h @@ -44,7 +44,7 @@ class LibVirtVirtualMachine final : public BaseVirtualMachine ~LibVirtVirtualMachine(); void start() override; - void shutdown(bool force = false) override; + void shutdown(const bool force = false) override; void suspend() override; State current_state() override; int ssh_port() override; diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index 56d17bb7da..1d733f582e 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -261,7 +261,7 @@ void mp::LXDVirtualMachine::start() update_state(); } -void mp::LXDVirtualMachine::shutdown(bool force) +void mp::LXDVirtualMachine::shutdown(const bool force) { std::unique_lock lock{state_mutex}; auto present_state = current_state(); diff --git a/src/platform/backends/lxd/lxd_virtual_machine.h b/src/platform/backends/lxd/lxd_virtual_machine.h index 312f17b018..b4f358c2da 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.h +++ b/src/platform/backends/lxd/lxd_virtual_machine.h @@ -43,7 +43,7 @@ class LXDVirtualMachine : public BaseVirtualMachine ~LXDVirtualMachine() override; void start() override; - void shutdown(bool force = false) override; + void shutdown(const bool force = false) override; void suspend() override; State current_state() override; int ssh_port() override; diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 87035d5743..8f9c9b0e60 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -343,7 +343,7 @@ void mp::QemuVirtualMachine::start() vm_process->write(qmp_execute_json("qmp_capabilities")); } -void mp::QemuVirtualMachine::shutdown(bool force) +void mp::QemuVirtualMachine::shutdown(const bool force) { if (force) { diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h index 3a7ae72d39..f182407b46 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.h +++ b/src/platform/backends/qemu/qemu_virtual_machine.h @@ -50,7 +50,7 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine ~QemuVirtualMachine(); void start() override; - void shutdown(bool force = false) override; + void shutdown(const bool force = false) override; void suspend() override; State current_state() override; int ssh_port() override; From 4450fecbd479f66f0eaf69097ba32a80ccd1ecb5 Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Thu, 4 Apr 2024 09:38:21 -0400 Subject: [PATCH 13/49] [tests] Fix QEMU test for setting bool for shutdown() --- tests/qemu/test_qemu_backend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 759febfb55..654e59d35f 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -742,7 +742,7 @@ TEST_F(QemuBackend, dropsSSHSessionWhenStopping) EXPECT_CALL(machine, drop_ssh_session()); MP_DELEGATE_MOCK_CALLS_ON_BASE(machine, shutdown, mp::QemuVirtualMachine); - machine.shutdown(); + machine.shutdown(false); } TEST_F(QemuBackend, supportsSnapshots) From b73e5a06daf79913a8c168e56f1a80b96393c74a Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Fri, 5 Apr 2024 12:28:53 -0400 Subject: [PATCH 14/49] [tests/cli] Test the --force option for stop command --- tests/test_cli_client.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 36a70dd7ee..162cd265a7 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -2518,6 +2518,25 @@ TEST_F(Client, stop_cmd_disabled_petenv_all) EXPECT_THAT(send_command({"stop", "--all"}), Eq(mp::ReturnCode::Ok)); } +TEST_F(Client, stop_cmd_force_sends_proper_request) +{ + const auto force_set_matcher = Property(&mp::StopRequest::force_stop, IsTrue()); + EXPECT_CALL(mock_daemon, stop) + .WillOnce(WithArg<1>(check_request_and_return(force_set_matcher, ok))); + + EXPECT_THAT(send_command({"stop", "foo", "--force"}), Eq(mp::ReturnCode::Ok)); +} + +TEST_F(Client, stop_cmd_force_conflicts_with_time_option) +{ + EXPECT_THAT(send_command({"stop", "foo", "--force", "--time", "10"}), Eq(mp::ReturnCode::CommandLineError)); +} + +TEST_F(Client, stop_cmd_force_conflicts_with_cancel_option) +{ + EXPECT_THAT(send_command({"stop", "foo", "--force", "--cancel"}), Eq(mp::ReturnCode::CommandLineError)); +} + // suspend cli tests TEST_F(Client, suspend_cmd_ok_with_one_arg) { From 9e6737b20e1c078b0f025589067a663b52c99bf2 Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Fri, 5 Apr 2024 16:14:31 -0400 Subject: [PATCH 15/49] [tests/qemu] Add tests for the force shutdown handling --- tests/qemu/test_qemu_backend.cpp | 94 ++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 654e59d35f..c6d040312a 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -68,6 +68,7 @@ struct QemuBackend : public mpt::TestWithMockedBinPath { EXPECT_CALL(*mock_qemu_platform, remove_resources_for(_)).WillRepeatedly(Return()); EXPECT_CALL(*mock_qemu_platform, vm_platform_args(_)).WillRepeatedly(Return(QStringList())); + EXPECT_CALL(*mock_qemu_platform, get_directory_name()).WillRepeatedly(Return(QString())); }; mpt::TempFile dummy_image; @@ -367,6 +368,99 @@ TEST_F(QemuBackend, machine_unknown_state_properly_shuts_down) EXPECT_THAT(machine->current_state(), Eq(mp::VirtualMachine::State::off)); } +TEST_F(QemuBackend, suspended_state_ignores_shutdown) +{ + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + auto logger_scope = mpt::MockLogger::inject(); + logger_scope.mock_logger->screen_logs(mpl::Level::info); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Ignoring shutdown issued while suspended"); + + mpt::StubVMStatusMonitor stub_monitor; + mp::QemuVirtualMachineFactory backend{data_dir.path()}; + + auto machine = backend.create_virtual_machine(default_description, key_provider, stub_monitor); + + machine->state = mp::VirtualMachine::State::suspended; + + machine->shutdown(); + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::suspended); +} + +TEST_F(QemuBackend, force_shutdown_kills_process_and_logs) +{ + mpt::MockProcess* vmproc = nullptr; + process_factory->register_callback([&vmproc](mpt::MockProcess* process) { + if (process->program().startsWith("qemu-system-") && + !process->arguments().contains("-dump-vmstate")) // we only care about the actual vm process + { + vmproc = process; // save this to control later + EXPECT_CALL(*process, kill()).WillOnce([process] { + mp::ProcessState exit_state{ + std::nullopt, + mp::ProcessState::Error{QProcess::Crashed, QStringLiteral("Force stopped")}}; + emit process->error_occurred(QProcess::Crashed, "Killed"); + emit process->finished(exit_state); + }); + } + }); + + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + auto logger_scope = mpt::MockLogger::inject(); + logger_scope.mock_logger->screen_logs(mpl::Level::info); + logger_scope.mock_logger->expect_log(mpl::Level::info, "process program"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "process arguments"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "process started"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Forced shutdown"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Killing process"); + logger_scope.mock_logger->expect_log(mpl::Level::error, "Killed"); + logger_scope.mock_logger->expect_log(mpl::Level::error, "Force stopped"); + + mpt::StubVMStatusMonitor stub_monitor; + mp::QemuVirtualMachineFactory backend{data_dir.path()}; + + auto machine = backend.create_virtual_machine(default_description, key_provider, stub_monitor); + + machine->start(); // we need this so that Process signals get connected to their handlers + + ASSERT_TRUE(vmproc); + + machine->state = mp::VirtualMachine::State::running; + + machine->shutdown(true); // force shutdown + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); +} + +TEST_F(QemuBackend, force_shutdown_no_process_logs) +{ + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + auto logger_scope = mpt::MockLogger::inject(); + logger_scope.mock_logger->screen_logs(mpl::Level::info); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Forced shutdown"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "No process to kill"); + + mpt::StubVMStatusMonitor stub_monitor; + mp::QemuVirtualMachineFactory backend{data_dir.path()}; + + auto machine = backend.create_virtual_machine(default_description, key_provider, stub_monitor); + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); + + machine->shutdown(true); // force shutdown + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); +} + TEST_F(QemuBackend, verify_dnsmasq_qemuimg_and_qemu_processes_created) { EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { From c39895e9cd1344f823e57bcc2b9752422f4a2766 Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Thu, 11 Apr 2024 14:38:26 -0400 Subject: [PATCH 16/49] Update src/platform/backends/qemu/qemu_virtual_machine.cpp Change "Forced" to "Forcing" Co-authored-by: Ricardo Abreu <6698114+ricab@users.noreply.github.com> Signed-off-by: Chris Townsend --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 8f9c9b0e60..d9a271fbb5 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -348,7 +348,7 @@ void mp::QemuVirtualMachine::shutdown(const bool force) if (force) { state = State::off; - mpl::log(mpl::Level::info, vm_name, "Forced shutdown"); + mpl::log(mpl::Level::info, vm_name, "Forcing shutdown"); if (vm_process) { From 8ac8d93a6f130efc056d047ddfa7a2060f20b6fd Mon Sep 17 00:00:00 2001 From: Chris Townsend Date: Fri, 12 Apr 2024 14:01:16 -0400 Subject: [PATCH 17/49] [tests] Fix tests due to change in log string --- tests/qemu/test_qemu_backend.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index c6d040312a..b8a1a8df32 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -417,7 +417,7 @@ TEST_F(QemuBackend, force_shutdown_kills_process_and_logs) logger_scope.mock_logger->expect_log(mpl::Level::info, "process program"); logger_scope.mock_logger->expect_log(mpl::Level::info, "process arguments"); logger_scope.mock_logger->expect_log(mpl::Level::info, "process started"); - logger_scope.mock_logger->expect_log(mpl::Level::info, "Forced shutdown"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Forcing shutdown"); logger_scope.mock_logger->expect_log(mpl::Level::info, "Killing process"); logger_scope.mock_logger->expect_log(mpl::Level::error, "Killed"); logger_scope.mock_logger->expect_log(mpl::Level::error, "Force stopped"); @@ -446,7 +446,7 @@ TEST_F(QemuBackend, force_shutdown_no_process_logs) auto logger_scope = mpt::MockLogger::inject(); logger_scope.mock_logger->screen_logs(mpl::Level::info); - logger_scope.mock_logger->expect_log(mpl::Level::info, "Forced shutdown"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Forcing shutdown"); logger_scope.mock_logger->expect_log(mpl::Level::info, "No process to kill"); mpt::StubVMStatusMonitor stub_monitor; From 96894a84526f4269efa061ff02e8c482ad7263b2 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:23:08 +0100 Subject: [PATCH 18/49] [vm] Add a stopping state --- include/multipass/virtual_machine.h | 3 ++- src/daemon/daemon.cpp | 2 ++ src/rpc/multipass.proto | 1 + 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index 3279b7d700..80801cc373 100644 --- a/include/multipass/virtual_machine.h +++ b/include/multipass/virtual_machine.h @@ -55,7 +55,8 @@ 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; diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 11ef416d74..035675f935 100644 --- a/src/daemon/daemon.cpp +++ b/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: diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index e1eaa4b964..2034617995 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -203,6 +203,7 @@ message InstanceStatus { DELAYED_SHUTDOWN = 6; SUSPENDING = 7; SUSPENDED = 8; + STOPPING = 9; // weird order preserves backward compatibility } Status status = 1; } From a18958888be214976874f7632de52eae13f4eee9 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:26:37 +0100 Subject: [PATCH 19/49] [daemon] Handle start while stopping --- src/daemon/daemon.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 035675f935..4b0a201e3f 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -2111,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: { @@ -2120,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; From 86490ed99f4bc703843700d41834924924d28486 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 19:33:02 +0100 Subject: [PATCH 20/49] [daemon] Handle reboot while stopping --- src/daemon/daemon.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 4b0a201e3f..225786bfda 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -3160,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), ""}; From 54615539dc460092e6dfa420d4a5d2e60fb0e0e1 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 19:33:44 +0100 Subject: [PATCH 21/49] [daemon] Refuse SSH info while stopping --- src/daemon/daemon.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 225786bfda..06652d7186 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -3237,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 && From 291592d61c091a6f2874a0060cf3ec4903cac62f Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:32:52 +0100 Subject: [PATCH 22/49] [libvirt] Recognize stopping VMs --- .../backends/libvirt/libvirt_virtual_machine.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index 6972d3422b..ae8ccf494b 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -209,13 +209,20 @@ 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; @@ -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() From a8225daff010cdae20ef1903fbb0cb11f9554f2b Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:34:50 +0100 Subject: [PATCH 23/49] [libvirt] Prevent start while stopping --- src/platform/backends/libvirt/libvirt_virtual_machine.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index ae8ccf494b..110c718c7d 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -320,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(); From d52683b2476130de3f8a91d0228fd782213c321d Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:35:15 +0100 Subject: [PATCH 24/49] [libvirt] Ignore shutdown while stopping --- src/platform/backends/libvirt/libvirt_virtual_machine.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index 110c718c7d..e14e7d9f71 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -393,9 +393,10 @@ void mp::LibVirtVirtualMachine::shutdown(const bool force) state_wait.wait(lock, [this] { return shutdown_while_starting; }); update_state(); } - else if (state == State::suspended) + else if (state == State::suspended || state == State::stopping) { - mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); + auto state_str = state == State::suspended ? "suspended" : "stopping"; + mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while {}", state_str)); } } From fbda51f9a3630bbc6b3b3d337fae022415f83a35 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:35:51 +0100 Subject: [PATCH 25/49] [libvirt] Ignore suspend while stopping --- src/platform/backends/libvirt/libvirt_virtual_machine.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index e14e7d9f71..6a0905afd6 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -425,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(); From 352cc5830f3bf9d38ce3b00927f0eca109f9abdd Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:36:24 +0100 Subject: [PATCH 26/49] [lxd] Recognize stopping VMs --- src/platform/backends/lxd/lxd_virtual_machine.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index 1d733f582e..cbaef0c46e 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -61,9 +61,10 @@ auto instance_state_for(const QString& name, mp::NetworkAccessManager* manager, { case 101: // Started case 103: // Running - case 107: // Stopping case 111: // Thawed return mp::VirtualMachine::State::running; + case 107: // Stopping + return mp::VirtualMachine::State::stopping; case 102: // Stopped return mp::VirtualMachine::State::stopped; case 106: // Starting From 88d259f791102c731a44f5b8f7e27ba3852f194b Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:36:56 +0100 Subject: [PATCH 27/49] [lxd] Refuse to start a stopping VM --- src/platform/backends/lxd/lxd_virtual_machine.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index cbaef0c46e..c2db9933e5 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -253,6 +253,10 @@ void mp::LXDVirtualMachine::start() mpl::log(mpl::Level::info, vm_name, fmt::format("Resuming from a suspended state")); request_state("unfreeze"); } + else if (state == State::stopping) + { + throw std::runtime_error(fmt::format("Cannot start {} while it is stopping", vm_name)); + } else { request_state("start"); From 696ab5c981a1155ee0c829eaed39dfe561cd83f2 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:38:08 +0100 Subject: [PATCH 28/49] [lxd] Ignore stop if off --- src/platform/backends/lxd/lxd_virtual_machine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index c2db9933e5..797273bada 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -271,7 +271,7 @@ void mp::LXDVirtualMachine::shutdown(const bool force) std::unique_lock lock{state_mutex}; auto present_state = current_state(); - if (present_state == State::stopped) + if (present_state == State::stopped || present_state == State::off) { mpl::log(mpl::Level::debug, vm_name, "Ignoring stop request since instance is already stopped"); return; From fb363265319e6834fcad2a8a3194da2380f2f4d1 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:38:37 +0100 Subject: [PATCH 29/49] [lxd] Ignore shutdown while stopping --- src/platform/backends/lxd/lxd_virtual_machine.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index 797273bada..c35298ad83 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -276,9 +276,12 @@ void mp::LXDVirtualMachine::shutdown(const bool force) mpl::log(mpl::Level::debug, vm_name, "Ignoring stop request since instance is already stopped"); return; } - else if (present_state == State::suspended && !force) + else if ((present_state == State::suspended || present_state == State::stopping) && !force) { - mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); + mpl::log(mpl::Level::info, + vm_name, + fmt::format("Ignoring shutdown issued while {}", + present_state == State::suspended ? "suspended" : "stopping")); return; } From 6fa15bcc2227e9e89582af861f19f9623c5ee10a Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:39:16 +0100 Subject: [PATCH 30/49] [lxd] Drop SSH session when stopping Drop the SSH session from LXD VMs when the stopping state is detected. --- src/platform/backends/lxd/lxd_virtual_machine.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index c35298ad83..69ed2c934d 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -316,7 +316,8 @@ mp::VirtualMachine::State mp::LXDVirtualMachine::current_state() return state; state = present_state; - if (state == State::suspended || state == State::suspending || state == State::restarting) + if (state == State::suspended || state == State::suspending || state == State::restarting || + state == State::stopping) drop_ssh_session(); } catch (const LocalSocketConnectionException& e) From c6d8d5231dd3b9ce7d3229df1eb7ccc711972fde Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 16:42:57 +0100 Subject: [PATCH 31/49] [lxd] Handle further states when ensuring running Handle the new stopping state, along with other states, when ensuring that a LXD VM is running. --- src/platform/backends/lxd/lxd_virtual_machine.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index 69ed2c934d..cecc328770 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -341,8 +341,17 @@ void mp::LXDVirtualMachine::ensure_vm_is_running() void mp::LXDVirtualMachine::ensure_vm_is_running(const std::chrono::milliseconds& timeout) { + static constexpr auto accepted_state = [](auto st) { + static constexpr auto skip_states = {State::off, + State::stopped, + State::stopping, + State::suspended, + State::suspending}; + return std::none_of(skip_states.begin(), skip_states.end(), [st](auto s) { return st == s; }); + }; + auto is_vm_running = [this, timeout] { - if (current_state() != State::stopped) + if (accepted_state(current_state())) { return true; } @@ -350,7 +359,7 @@ void mp::LXDVirtualMachine::ensure_vm_is_running(const std::chrono::milliseconds // Sleep to see if LXD is just rebooting the instance std::this_thread::sleep_for(timeout); - if (current_state() != State::stopped) + if (accepted_state(current_state())) { state = State::starting; return true; From f8b19553ec8810f90937caaf96c31259b7493b48 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 17:02:50 +0100 Subject: [PATCH 32/49] [qemu] Recognize stopping VMs --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 7 +++++++ src/platform/backends/qemu/qemu_virtual_machine.h | 1 + 2 files changed, 8 insertions(+) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index d9a271fbb5..30d7552a17 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -438,6 +438,12 @@ void mp::QemuVirtualMachine::on_error() update_state(); } +void mp::QemuVirtualMachine::on_stopping() +{ + state = State::stopping; + update_state(); +} + void mp::QemuVirtualMachine::on_shutdown() { { @@ -571,6 +577,7 @@ void mp::QemuVirtualMachine::initialize_vm_process() else if (event.toString() == "POWERDOWN") { mpl::log(mpl::Level::info, vm_name, "VM powering down"); + on_stopping(); } else if (event.toString() == "SHUTDOWN") { diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h index f182407b46..6a9d163fc7 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.h +++ b/src/platform/backends/qemu/qemu_virtual_machine.h @@ -106,6 +106,7 @@ class QemuVirtualMachine : public QObject, public BaseVirtualMachine bool update_shutdown_status{true}; bool is_starting_from_suspend{false}; std::chrono::steady_clock::time_point network_deadline; + void on_stopping(); }; } // namespace multipass From 046ac69ea0b63d096bbeac65f0493b36a612ee67 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 18:56:40 +0100 Subject: [PATCH 33/49] [qemu] Handle multiple QMP entries in single read Deal with cases where we read multiple QMP entries in a single read of QEMU's output. This is motivated mainly by the POWERDOWN event coming together with a "return" entry (right after it) when stopping a VM. --- .../backends/qemu/qemu_virtual_machine.cpp | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 30d7552a17..d3a8866c57 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -564,36 +564,40 @@ void mp::QemuVirtualMachine::initialize_vm_process() QObject::connect(vm_process.get(), &Process::ready_read_standard_output, [this]() { auto qmp_output = vm_process->read_all_standard_output(); mpl::log(mpl::Level::debug, vm_name, fmt::format("QMP: {}", qmp_output)); - auto qmp_object = QJsonDocument::fromJson(qmp_output.split('\n').first()).object(); - auto event = qmp_object["event"]; - if (!event.isNull()) + for (const auto& line : qmp_output.split('\n')) { - if (event.toString() == "RESET" && state != State::restarting) - { - mpl::log(mpl::Level::info, vm_name, "VM restarting"); - on_restart(); - } - else if (event.toString() == "POWERDOWN") - { - mpl::log(mpl::Level::info, vm_name, "VM powering down"); - on_stopping(); - } - else if (event.toString() == "SHUTDOWN") - { - mpl::log(mpl::Level::info, vm_name, "VM shut down"); - } - else if (event.toString() == "STOP") - { - mpl::log(mpl::Level::info, vm_name, "VM suspending"); - } - else if (event.toString() == "RESUME") + auto qmp_object = QJsonDocument::fromJson(line).object(); + auto event = qmp_object["event"]; + + if (!event.isNull()) { - mpl::log(mpl::Level::info, vm_name, "VM suspended"); - if (state == State::suspending || state == State::running) + if (event.toString() == "RESET" && state != State::restarting) + { + mpl::log(mpl::Level::info, vm_name, "VM restarting"); + on_restart(); + } + else if (event.toString() == "POWERDOWN") + { + mpl::log(mpl::Level::info, vm_name, "VM powering down"); + on_stopping(); + } + else if (event.toString() == "SHUTDOWN") + { + mpl::log(mpl::Level::info, vm_name, "VM shut down"); + } + else if (event.toString() == "STOP") + { + mpl::log(mpl::Level::info, vm_name, "VM suspending"); + } + else if (event.toString() == "RESUME") { - vm_process->kill(); - on_suspend(); + mpl::log(mpl::Level::info, vm_name, "VM suspended"); + if (state == State::suspending || state == State::running) + { + vm_process->kill(); + on_suspend(); + } } } } From 10ab157f00a4270c3e916f3b1eee3166ea2ddc6e Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 19:01:48 +0100 Subject: [PATCH 34/49] [qemu] Warn if we read outside newline boundaries Issue a warning when reading QMP output outside of newline boundaries, which the code is otherwise unable to handle, as it would most likely result in bad JSON on the corresponding and following reads. --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index d3a8866c57..1d482476b5 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -563,7 +563,11 @@ void mp::QemuVirtualMachine::initialize_vm_process() QObject::connect(vm_process.get(), &Process::ready_read_standard_output, [this]() { auto qmp_output = vm_process->read_all_standard_output(); - mpl::log(mpl::Level::debug, vm_name, fmt::format("QMP: {}", qmp_output)); + + if (!qmp_output.endsWith('\n')) // TODO actually deal with this - it will probably have bad json + mpl::log(logging::Level::warning, vm_name, "partial QMP output"); + else + mpl::log(mpl::Level::debug, vm_name, fmt::format("QMP: {}", qmp_output)); for (const auto& line : qmp_output.split('\n')) { From c58570c6bc540fc303898e63b3badf3bc677e82a Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Apr 2024 12:44:31 +0100 Subject: [PATCH 35/49] [qemu] Drop SSH session when stop is detected --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 1d482476b5..61ee1f644b 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -440,6 +440,7 @@ void mp::QemuVirtualMachine::on_error() void mp::QemuVirtualMachine::on_stopping() { + drop_ssh_session(); state = State::stopping; update_state(); } From 77f9b9fe9803e3338c569a898353df497db11c81 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 19:30:28 +0100 Subject: [PATCH 36/49] [qemu] Refuse to start while stopping --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 61ee1f644b..efa32200f1 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -307,6 +307,10 @@ void mp::QemuVirtualMachine::start() is_starting_from_suspend = true; network_deadline = std::chrono::steady_clock::now() + 5s; } + else if (state == State::stopping) + { + throw std::runtime_error(fmt::format("Cannot start {} while it is stopping", vm_name)); + } else { // remove the mount arguments from the rest of the arguments, as they are stored separately for easier retrieval From ff76b49c1a412a67d284f06251bf1c414a734013 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 12 Apr 2024 19:31:02 +0100 Subject: [PATCH 37/49] [qemu] Ignore suspend while stopping --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index efa32200f1..a457065f17 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -407,9 +407,10 @@ void mp::QemuVirtualMachine::suspend() vm_process->wait_for_finished(timeout); vm_process.reset(nullptr); } - else if (state == State::off || state == State::suspended) + else if (state == State::off || state == State::stopped || state == State::suspended || state == State::stopping) { - mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring suspend issued while stopped/suspended")); + // TODO@no-merge use an util to get string from state enum (reuse where needed, e.g. CLI format utils) + mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring suspend issued while stopped/stopping/suspended")); monitor->on_suspend(); } } From 769c4ab9c780f7c84651501dd9b496d784125c2a Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Apr 2024 18:55:23 +0100 Subject: [PATCH 38/49] [daemon] Skip (ACPI) shutdown on stopping VMs --- src/daemon/daemon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 06652d7186..50fa545aed 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -3181,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; })) { From 69f7862b1565bd95d86b87bdfe3c2e80c9c8e504 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Apr 2024 18:56:27 +0100 Subject: [PATCH 39/49] [tests] Cover Stopping state in existing test --- tests/lxd/mock_lxd_server_responses.h | 26 ++++++++++++++++++++++++ tests/lxd/test_lxd_backend.cpp | 1 + tests/test_base_snapshot.cpp | 1 + tests/test_instance_settings_handler.cpp | 13 +++++++++--- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/tests/lxd/mock_lxd_server_responses.h b/tests/lxd/mock_lxd_server_responses.h index 632ca565ac..11df6a092e 100644 --- a/tests/lxd/mock_lxd_server_responses.h +++ b/tests/lxd/mock_lxd_server_responses.h @@ -513,6 +513,32 @@ const QByteArray vm_state_starting_data{"{" "\"type\": \"sync\"" "}\n"}; +const QByteArray vm_state_stopping_data{"{" + "\"error\": \"\"," + "\"error_code\": 0," + "\"metadata\": {" + " \"cpu\": {" + " \"usage\": 0" + " }," + " \"disk\": null," + " \"memory\": {" + " \"swap_usage\": 0," + " \"swap_usage_peak\": 0," + " \"usage\": 0," + " \"usage_peak\": 0" + " }," + " \"network\": null," + " \"pid\": 0," + " \"processes\": 0," + " \"status\": \"Stopping\"," + " \"status_code\": 107" + " }," + "\"operation\": \"\"," + "\"status\": \"Success\"," + "\"status_code\": 200," + "\"type\": \"sync\"" + "}\n"}; + const QByteArray vm_state_freezing_data{"{" "\"error\": \"\"," "\"error_code\": 0," diff --git a/tests/lxd/test_lxd_backend.cpp b/tests/lxd/test_lxd_backend.cpp index 2ad5ceda87..a092556ba4 100644 --- a/tests/lxd/test_lxd_backend.cpp +++ b/tests/lxd/test_lxd_backend.cpp @@ -95,6 +95,7 @@ struct LXDInstanceStatusTestSuite : LXDBackend, WithParamInterface lxd_instance_status_suite_inputs{ {mpt::vm_state_stopped_data, mp::VirtualMachine::State::stopped}, {mpt::vm_state_starting_data, mp::VirtualMachine::State::starting}, + {mpt::vm_state_stopping_data, mp::VirtualMachine::State::stopping}, {mpt::vm_state_freezing_data, mp::VirtualMachine::State::suspending}, {mpt::vm_state_frozen_data, mp::VirtualMachine::State::suspended}, {mpt::vm_state_cancelling_data, mp::VirtualMachine::State::unknown}, diff --git a/tests/test_base_snapshot.cpp b/tests/test_base_snapshot.cpp index f22ffb166b..09fb705fcc 100644 --- a/tests/test_base_snapshot.cpp +++ b/tests/test_base_snapshot.cpp @@ -261,6 +261,7 @@ INSTANTIATE_TEST_SUITE_P(TestBaseSnapshot, mp::VirtualMachine::State::restarting, mp::VirtualMachine::State::running, mp::VirtualMachine::State::delayed_shutdown, + mp::VirtualMachine::State::stopping, mp::VirtualMachine::State::suspending, mp::VirtualMachine::State::suspended, mp::VirtualMachine::State::unknown)); diff --git a/tests/test_instance_settings_handler.cpp b/tests/test_instance_settings_handler.cpp index 63576ad85b..e112bfd300 100644 --- a/tests/test_instance_settings_handler.cpp +++ b/tests/test_instance_settings_handler.cpp @@ -561,10 +561,17 @@ TEST_P(TestInstanceModOnNonStoppedInstance, setRefusesToModifyNonStoppedInstance EXPECT_EQ(original_specs, specs[target_instance_name]); } -INSTANTIATE_TEST_SUITE_P(TestInstanceSettingsHandler, TestInstanceModOnNonStoppedInstance, +INSTANTIATE_TEST_SUITE_P(TestInstanceSettingsHandler, + TestInstanceModOnNonStoppedInstance, Combine(ValuesIn(TestInstanceSettingsHandler::properties), - Values(VMSt::running, VMSt::restarting, VMSt::starting, VMSt::delayed_shutdown, - VMSt::suspended, VMSt::suspending, VMSt::unknown))); + Values(VMSt::running, + VMSt::restarting, + VMSt::starting, + VMSt::stopping, + VMSt::delayed_shutdown, + VMSt::suspended, + VMSt::suspending, + VMSt::unknown))); struct TestInstanceModOnStoppedInstance : public TestInstanceSettingsHandler, public WithParamInterface From 9c5d246c17974f815d6870f47a7c9e0379d274d5 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Mon, 15 Apr 2024 18:57:45 +0100 Subject: [PATCH 40/49] [test] Verify is_running on stopping state --- tests/test_utils.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp index 1357bfac96..98d66ffb2d 100644 --- a/tests/test_utils.cpp +++ b/tests/test_utils.cpp @@ -656,6 +656,13 @@ TEST(Utils, vm_stopped_returns_false) EXPECT_FALSE(MP_UTILS.is_running(state)); } +TEST(Utils, vm_stopping_returns_false) +{ + mp::VirtualMachine::State state = mp::VirtualMachine::State::stopping; + + EXPECT_FALSE(MP_UTILS.is_running(state)); +} + TEST(Utils, absent_config_file_and_dir_are_created) { mpt::TempDir temp_dir; From 212be771f91ecc76b0b17e5f30ff9eec17343079 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 23 Apr 2024 11:24:59 +0100 Subject: [PATCH 41/49] [cli] Convert stopping state to string --- src/client/cli/formatter/format_utils.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/client/cli/formatter/format_utils.cpp b/src/client/cli/formatter/format_utils.cpp index 170a30cc1d..2d3f16aca2 100644 --- a/src/client/cli/formatter/format_utils.cpp +++ b/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; From edc1195baeca1d339500e86ec804b349e861cc68 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 23 Apr 2024 11:32:39 +0100 Subject: [PATCH 42/49] [tests] Check str representation of STOPPING state --- tests/test_format_utils.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_format_utils.cpp b/tests/test_format_utils.cpp index 80252d380a..cc345a9285 100644 --- a/tests/test_format_utils.cpp +++ b/tests/test_format_utils.cpp @@ -77,6 +77,15 @@ TEST(InstanceStatusString, RESTARTING_status_returns_Restarting) EXPECT_THAT(status_string, Eq("Restarting")); } +TEST(InstanceStatusString, StoppingStatusConvertsToString) +{ + mp::InstanceStatus status; + status.set_status(mp::InstanceStatus::STOPPING); + auto status_string = mp::format::status_string_for(status); + + EXPECT_THAT(status_string, Eq("Stopping")); +} + TEST(InstanceStatusString, bogus_status_returns_Unknown) { mp::InstanceStatus status; From fcd1fb5af051edbd2a68accf328eb6015591f97f Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 23 Apr 2024 12:08:49 +0100 Subject: [PATCH 43/49] [tests] Add stopping instance to formatter tests --- tests/test_output_formatter.cpp | 193 +++++++++++++++++++++++++++++++- 1 file changed, 189 insertions(+), 4 deletions(-) diff --git a/tests/test_output_formatter.cpp b/tests/test_output_formatter.cpp index e66382ae5e..791434bbbe 100644 --- a/tests/test_output_formatter.cpp +++ b/tests/test_output_formatter.cpp @@ -86,6 +86,11 @@ auto construct_multiple_instances_list_reply() list_entry->mutable_instance_status()->set_status(mp::InstanceStatus::STOPPED); list_entry->set_current_release("18.04 LTS"); + list_entry = list_reply.mutable_instance_list()->add_instances(); + list_entry->set_name("fantastic"); + list_entry->mutable_instance_status()->set_status(mp::InstanceStatus::STOPPING); + list_entry->set_current_release("24.04 LTS"); + return list_reply; } @@ -345,6 +350,13 @@ auto construct_multiple_instances_info_reply() info_entry->mutable_instance_info()->set_id("ab5191cc172564e7cc0eafd397312a32598823e645279c820f0935393aead509"); info_entry->mutable_instance_info()->set_num_snapshots(3); + info_entry = info_reply.add_details(); + info_entry->set_name("fantastic"); + info_entry->mutable_instance_status()->set_status(mp::InstanceStatus::STOPPING); + info_entry->mutable_instance_info()->set_image_release("24.04 LTS"); + info_entry->mutable_instance_info()->set_id("abcabcabc"); + info_entry->mutable_instance_info()->set_num_snapshots(0); + return info_reply; } @@ -460,6 +472,13 @@ auto construct_mixed_instance_and_snapshot_info_reply() info_entry->mutable_instance_info()->set_id("ab5191cc172564e7cc0eafd397312a32598823e645279c820f0935393aead509"); info_entry->mutable_instance_info()->set_num_snapshots(3); + info_entry = info_reply.add_details(); + info_entry->set_name("fantastic"); + info_entry->mutable_instance_status()->set_status(mp::InstanceStatus::STOPPING); + info_entry->mutable_instance_info()->set_image_release("24.04 LTS"); + info_entry->mutable_instance_info()->set_id("abcabcabc"); + info_entry->mutable_instance_info()->set_num_snapshots(0); + return info_reply; } @@ -542,6 +561,13 @@ auto construct_multiple_mixed_instances_and_snapshots_info_reply() info_entry->mutable_instance_info()->set_id("ab5191cc172564e7cc0eafd397312a32598823e645279c820f0935393aead509"); info_entry->mutable_instance_info()->set_num_snapshots(3); + info_entry = info_reply.add_details(); + info_entry->set_name("fantastic"); + info_entry->mutable_instance_status()->set_status(mp::InstanceStatus::STOPPING); + info_entry->mutable_instance_info()->set_image_release("24.04 LTS"); + info_entry->mutable_instance_info()->set_id("abcabcabc"); + info_entry->mutable_instance_info()->set_num_snapshots(0); + info_entry = info_reply.add_details(); fundamentals = info_entry->mutable_snapshot_info()->mutable_fundamentals(); @@ -841,7 +867,8 @@ const std::vector orderable_list_info_formatter_outputs{ &multiple_instances_list_reply, "Name State IPv4 Image\n" "bogus-instance Running 10.21.124.56 Ubuntu 16.04 LTS\n" - "bombastic Stopped -- Ubuntu 18.04 LTS\n", + "bombastic Stopped -- Ubuntu 18.04 LTS\n" + "fantastic Stopping -- Ubuntu 24.04 LTS\n", "table_list_multiple"}, {&table_formatter, @@ -916,6 +943,17 @@ const std::vector orderable_list_info_formatter_outputs{ "Load: --\n" "Disk usage: --\n" "Memory usage: --\n" + "Mounts: --\n\n" + "Name: fantastic\n" + "State: Stopping\n" + "Snapshots: 0\n" + "IPv4: --\n" + "Release: --\n" + "Image hash: abcabcabc (Ubuntu 24.04 LTS)\n" + "CPU(s): --\n" + "Load: --\n" + "Disk usage: --\n" + "Memory usage: --\n" "Mounts: --\n", "table_info_multiple_instances"}, {&table_formatter, @@ -974,6 +1012,17 @@ const std::vector orderable_list_info_formatter_outputs{ "Disk usage: --\n" "Memory usage: --\n" "Mounts: --\n\n" + "Name: fantastic\n" + "State: Stopping\n" + "Snapshots: 0\n" + "IPv4: --\n" + "Release: --\n" + "Image hash: abcabcabc (Ubuntu 24.04 LTS)\n" + "CPU(s): --\n" + "Load: --\n" + "Disk usage: --\n" + "Memory usage: --\n" + "Mounts: --\n\n" "Snapshot: snapshot2\n" "Instance: bogus-instance\n" "CPU(s): 2\n" @@ -1013,6 +1062,17 @@ const std::vector orderable_list_info_formatter_outputs{ "Disk usage: --\n" "Memory usage: --\n" "Mounts: --\n\n" + "Name: fantastic\n" + "State: Stopping\n" + "Snapshots: 0\n" + "IPv4: --\n" + "Release: --\n" + "Image hash: abcabcabc (Ubuntu 24.04 LTS)\n" + "CPU(s): --\n" + "Load: --\n" + "Disk usage: --\n" + "Memory usage: --\n" + "Mounts: --\n\n" "Snapshot: snapshot1\n" "Instance: bogus-instance\n" "CPU(s): 2\n" @@ -1057,7 +1117,8 @@ const std::vector orderable_list_info_formatter_outputs{ &multiple_instances_list_reply, "Name,State,IPv4,IPv6,Release,AllIPv4\n" "bogus-instance,Running,10.21.124.56,,Ubuntu 16.04 LTS,\"10.21.124.56\"\n" - "bombastic,Stopped,,,Ubuntu 18.04 LTS,\"\"\n", + "bombastic,Stopped,,,Ubuntu 18.04 LTS,\"\"\n" + "fantastic,Stopping,,,Ubuntu 24.04 LTS,\"\"\n", "csv_list_multiple"}, {&csv_formatter, &unsorted_list_reply, @@ -1114,7 +1175,8 @@ const std::vector orderable_list_info_formatter_outputs{ "LTS,1797c5c82016c1e65f4008fcf89deae3a044ef76087a9ec5b907c6d64a3609ac,16.04 LTS,0.03 0.10 " "0.15,1932735284,6764573492,38797312,1610612736,/home/user/source => " "source,10.21.124.56,4,1\nbombastic,Stopped,,,," - "ab5191cc172564e7cc0eafd397312a32598823e645279c820f0935393aead509,18.04 LTS,,,,,,,,,3\n", + "ab5191cc172564e7cc0eafd397312a32598823e645279c820f0935393aead509,18.04 LTS,,,,,,,,,3\nfantastic,Stopping,,,," + "abcabcabc,24.04 LTS,,,,,,,,,0\n", "csv_info_multiple_instances"}, {&yaml_formatter, &empty_list_reply, "\n", "yaml_list_empty"}, @@ -1138,7 +1200,12 @@ const std::vector orderable_list_info_formatter_outputs{ " - state: Stopped\n" " ipv4:\n" " []\n" - " release: Ubuntu 18.04 LTS\n", + " release: Ubuntu 18.04 LTS\n" + "fantastic:\n" + " - state: Stopping\n" + " ipv4:\n" + " []\n" + " release: Ubuntu 24.04 LTS\n", "yaml_list_multiple"}, {&yaml_formatter, &unsorted_list_reply, @@ -1279,6 +1346,23 @@ const std::vector orderable_list_info_formatter_outputs{ " total: ~\n" " ipv4:\n" " []\n" + " mounts: ~\n" + "fantastic:\n" + " - state: Stopping\n" + " snapshot_count: 0\n" + " image_hash: abcabcabc\n" + " image_release: 24.04 LTS\n" + " release: ~\n" + " cpu_count: ~\n" + " disks:\n" + " - sda1:\n" + " used: ~\n" + " total: ~\n" + " memory:\n" + " usage: ~\n" + " total: ~\n" + " ipv4:\n" + " []\n" " mounts: ~\n", "yaml_info_multiple_instances"}, {&yaml_formatter, @@ -1361,6 +1445,23 @@ const std::vector orderable_list_info_formatter_outputs{ " ipv4:\n" " []\n" " mounts: ~\n" + "fantastic:\n" + " - state: Stopping\n" + " snapshot_count: 0\n" + " image_hash: abcabcabc\n" + " image_release: 24.04 LTS\n" + " release: ~\n" + " cpu_count: ~\n" + " disks:\n" + " - sda1:\n" + " used: ~\n" + " total: ~\n" + " memory:\n" + " usage: ~\n" + " total: ~\n" + " ipv4:\n" + " []\n" + " mounts: ~\n" "bogus-instance:\n" " - snapshots:\n" " - snapshot2:\n" @@ -1456,6 +1557,23 @@ const std::vector orderable_list_info_formatter_outputs{ " ipv4:\n" " []\n" " mounts: ~\n" + "fantastic:\n" + " - state: Stopping\n" + " snapshot_count: 0\n" + " image_hash: abcabcabc\n" + " image_release: 24.04 LTS\n" + " release: ~\n" + " cpu_count: ~\n" + " disks:\n" + " - sda1:\n" + " used: ~\n" + " total: ~\n" + " memory:\n" + " usage: ~\n" + " total: ~\n" + " ipv4:\n" + " []\n" + " mounts: ~\n" "messier-87:\n" " - snapshots:\n" " - black-hole:\n" @@ -1513,6 +1631,13 @@ const std::vector non_orderable_list_info_formatter_outputs{ " \"name\": \"bombastic\",\n" " \"release\": \"Ubuntu 18.04 LTS\",\n" " \"state\": \"Stopped\"\n" + " },\n" + " {\n" + " \"ipv4\": [\n" + " ],\n" + " \"name\": \"fantastic\",\n" + " \"release\": \"Ubuntu 24.04 LTS\",\n" + " \"state\": \"Stopping\"\n" " }\n" " ]\n" "}\n", @@ -1692,6 +1817,26 @@ const std::vector non_orderable_list_info_formatter_outputs{ " \"release\": \"\",\n" " \"snapshot_count\": \"3\",\n" " \"state\": \"Stopped\"\n" + " },\n" + " \"fantastic\": {\n" + " \"cpu_count\": \"\",\n" + " \"disks\": {\n" + " \"sda1\": {\n" + " }\n" + " },\n" + " \"image_hash\": \"abcabcabc\",\n" + " \"image_release\": \"24.04 LTS\",\n" + " \"ipv4\": [\n" + " ],\n" + " \"load\": [\n" + " ],\n" + " \"memory\": {\n" + " },\n" + " \"mounts\": {\n" + " },\n" + " \"release\": \"\",\n" + " \"snapshot_count\": \"0\",\n" + " \"state\": \"Stopping\"\n" " }\n" " }\n" "}\n", @@ -1831,6 +1976,26 @@ const std::vector non_orderable_list_info_formatter_outputs{ " \"release\": \"\",\n" " \"snapshot_count\": \"3\",\n" " \"state\": \"Stopped\"\n" + " },\n" + " \"fantastic\": {\n" + " \"cpu_count\": \"\",\n" + " \"disks\": {\n" + " \"sda1\": {\n" + " }\n" + " },\n" + " \"image_hash\": \"abcabcabc\",\n" + " \"image_release\": \"24.04 LTS\",\n" + " \"ipv4\": [\n" + " ],\n" + " \"load\": [\n" + " ],\n" + " \"memory\": {\n" + " },\n" + " \"mounts\": {\n" + " },\n" + " \"release\": \"\",\n" + " \"snapshot_count\": \"0\",\n" + " \"state\": \"Stopping\"\n" " }\n" " }\n" "}\n", @@ -1934,6 +2099,26 @@ const std::vector non_orderable_list_info_formatter_outputs{ " \"snapshot_count\": \"3\",\n" " \"state\": \"Stopped\"\n" " },\n" + " \"fantastic\": {\n" + " \"cpu_count\": \"\",\n" + " \"disks\": {\n" + " \"sda1\": {\n" + " }\n" + " },\n" + " \"image_hash\": \"abcabcabc\",\n" + " \"image_release\": \"24.04 LTS\",\n" + " \"ipv4\": [\n" + " ],\n" + " \"load\": [\n" + " ],\n" + " \"memory\": {\n" + " },\n" + " \"mounts\": {\n" + " },\n" + " \"release\": \"\",\n" + " \"snapshot_count\": \"0\",\n" + " \"state\": \"Stopping\"\n" + " },\n" " \"messier-87\": {\n" " \"snapshots\": {\n" " \"black-hole\": {\n" From a3631ab88482a5ee149f5aa40d13baa8dc645f91 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 23 Apr 2024 12:31:55 +0100 Subject: [PATCH 44/49] [shutdown] Lock when cancelling delayed_shutdown --- src/daemon/delayed_shutdown_timer.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/daemon/delayed_shutdown_timer.cpp b/src/daemon/delayed_shutdown_timer.cpp index 6cb470a62f..f64cdd9e24 100644 --- a/src/daemon/delayed_shutdown_timer.cpp +++ b/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"); } }); From d759e58bcd2b28662163ee687dbc2cb5dfc85a56 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 23 Apr 2024 19:00:51 +0100 Subject: [PATCH 45/49] [qemu] Lock and log stopping state --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index a457065f17..639fec17f9 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -445,8 +445,10 @@ void mp::QemuVirtualMachine::on_error() void mp::QemuVirtualMachine::on_stopping() { - drop_ssh_session(); + std::unique_lock lock{state_mutex}; + mpl::log(mpl::Level::trace, vm_name, "VM stopping"); state = State::stopping; + drop_ssh_session(); update_state(); } From 4faa19abf22509e03bd283a3dab44164c8c050d4 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 26 Apr 2024 20:35:06 +0100 Subject: [PATCH 46/49] [lxd] Drive LXD instances into Stopping state --- src/platform/backends/lxd/lxd_virtual_machine.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index cecc328770..58015e7254 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -285,8 +285,10 @@ void mp::LXDVirtualMachine::shutdown(const bool force) return; } - request_state("stop", {{"force", force}}); + state = State::stopping; + update_state(); + request_state("stop", {{"force", force}}); state = State::stopped; if (present_state == State::starting) From 211504876406cf81c6f31d173687ff7d8476cb4f Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Fri, 26 Apr 2024 20:39:15 +0100 Subject: [PATCH 47/49] [libvirt] Drive libvirt VMs into stopping state --- .../backends/libvirt/libvirt_virtual_machine.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index 6a0905afd6..c389249028 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -227,8 +227,8 @@ auto refresh_instance_state_for_domain(virDomainPtr domain, const mp::VirtualMac 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; } @@ -374,6 +374,9 @@ void mp::LibVirtVirtualMachine::shutdown(const bool force) 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) @@ -383,9 +386,6 @@ void mp::LibVirtVirtualMachine::shutdown(const bool force) 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) { From 5a768d067fcddc11edae0f323a88b5ea817a4c47 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 30 Apr 2024 18:22:26 +0100 Subject: [PATCH 48/49] [qemu] Drive QEMU VMs into the stopping state --- .../backends/qemu/qemu_virtual_machine.cpp | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 639fec17f9..2900319fe8 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -349,9 +349,16 @@ void mp::QemuVirtualMachine::start() void mp::QemuVirtualMachine::shutdown(const bool force) { + std::unique_lock lock{state_mutex}; + auto initial_state = state; + if (force) { - state = State::off; + state = State::stopping; + update_state(); + lock.unlock(); // The whole method should be locked, but this is the best we can do without reworking stopping + // while starting: we'd need to lock recursively, which is incompatible with condition_variable + mpl::log(mpl::Level::info, vm_name, "Forcing shutdown"); if (vm_process) @@ -370,17 +377,23 @@ void mp::QemuVirtualMachine::shutdown(const bool force) } else { + // TODO@no-merge unduplicate & otherwise streamline code after rebasing on stable base + state = State::stopping; + update_state(); + lock.unlock(); + drop_ssh_session(); - if ((state == State::running || state == State::delayed_shutdown || state == State::unknown) && vm_process && - vm_process->running()) + if ((initial_state == State::running || initial_state == State::delayed_shutdown || + initial_state == State::unknown) && + vm_process && vm_process->running()) { vm_process->write(qmp_execute_json("system_powerdown")); vm_process->wait_for_finished(timeout); } else { - if (state == State::starting) + if (initial_state == State::starting) update_shutdown_status = false; if (vm_process) @@ -446,10 +459,15 @@ void mp::QemuVirtualMachine::on_error() void mp::QemuVirtualMachine::on_stopping() { std::unique_lock lock{state_mutex}; - mpl::log(mpl::Level::trace, vm_name, "VM stopping"); - state = State::stopping; - drop_ssh_session(); - update_state(); + if (state != State::stopping) + { + mpl::log(mpl::Level::trace, vm_name, "VM stopping"); + state = State::stopping; + drop_ssh_session(); + update_state(); + } + else + mpl::log(mpl::Level::trace, vm_name, "VM already stopping"); } void mp::QemuVirtualMachine::on_shutdown() From 89b4ab1da049a71065f45dd85003ad4d77c46fd8 Mon Sep 17 00:00:00 2001 From: Ricardo Abreu Date: Tue, 30 Apr 2024 18:29:34 +0100 Subject: [PATCH 49/49] [qemu] Ignore shutdown in more inapplicable states --- src/platform/backends/qemu/qemu_virtual_machine.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index 2900319fe8..3ce7544cff 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -371,9 +371,10 @@ void mp::QemuVirtualMachine::shutdown(const bool force) mpl::log(mpl::Level::info, vm_name, "No process to kill"); } } - else if (state == State::suspended) + else if (state == State::off || state == State::stopped || state == State::suspended || state == State::stopping) { - mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); + // TODO@no-merge use an util to get string from state enum (reuse where needed, e.g. CLI format utils) + mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while stopped/stopping/suspended")); } else {