diff --git a/include/multipass/exceptions/virtual_machine_state_exceptions.h b/include/multipass/exceptions/virtual_machine_state_exceptions.h new file mode 100644 index 0000000000..a4eec1d2c6 --- /dev/null +++ b/include/multipass/exceptions/virtual_machine_state_exceptions.h @@ -0,0 +1,44 @@ +/* + * Copyright (C) Canonical, Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +#ifndef MULTIPASS_VIRTUAL_MACHINE_STATE_EXCEPTIONS_H +#define MULTIPASS_VIRTUAL_MACHINE_STATE_EXCEPTIONS_H + +#include +#include + +namespace multipass +{ +class VMStateIdempotentException : public std::runtime_error +{ +public: + explicit VMStateIdempotentException(const std::string& msg) : runtime_error{msg} + { + } +}; + +class VMStateInvalidException : public std::runtime_error +{ +public: + explicit VMStateInvalidException(const std::string& msg) : runtime_error{msg} + { + } +}; + +} // namespace multipass + +#endif // MULTIPASS_VIRTUAL_MACHINE_STATE_EXCEPTIONS_H diff --git a/include/multipass/virtual_machine.h b/include/multipass/virtual_machine.h index 5032aa0b93..2c02e14eaa 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; diff --git a/src/client/cli/cmd/stop.cpp b/src/client/cli/cmd/stop.cpp index 534ffc65ce..0482bc4387 100644 --- a/src/client/cli/cmd/stop.cpp +++ b/src/client/cli/cmd/stop.cpp @@ -62,8 +62,8 @@ QString cmd::Stop::short_help() const QString cmd::Stop::description() const { - return QStringLiteral("Stop the named instances, if running. Exits with\n" - "return code 0 if successful."); + return QStringLiteral("Stop the named instances. Exits with return code 0 \n" + "if successful."); } mp::ParseCode cmd::Stop::parse_args(mp::ArgParser* parser) @@ -84,7 +84,10 @@ 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 the instance to shut down immediately. Warning: This could potentially " + "corrupt a running instance, so use with caution."); + parser->addOptions({all_option, time_option, cancel_option, force_option}); auto status = parser->commandParse(this); if (status != ParseCode::Ok) @@ -105,6 +108,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("+")) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 99092cdf20..a38552498e 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -2132,10 +2132,13 @@ try // clang-format on std::function operation; if (request->cancel_shutdown()) - operation = std::bind(&Daemon::cancel_vm_shutdown, this, std::placeholders::_1); + operation = [this](const VirtualMachine& vm) { return this->cancel_vm_shutdown(vm); }; + else if (request->force_stop()) + operation = [this](VirtualMachine& vm) { return this->switch_off_vm(vm); }; else - operation = std::bind(&Daemon::shutdown_vm, this, std::placeholders::_1, - std::chrono::minutes(request->time_minutes())); + operation = [this, delay_minutes = std::chrono::minutes(request->time_minutes())](VirtualMachine& vm) { + return this->shutdown_vm(vm, delay_minutes); + }; status = cmd_vms(instance_selection.operative_selection, operation); } @@ -3071,7 +3074,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) { @@ -3116,12 +3119,14 @@ grpc::Status mp::Daemon::reboot_vm(VirtualMachine& vm) grpc::Status mp::Daemon::shutdown_vm(VirtualMachine& vm, const std::chrono::milliseconds delay) { const auto& name = vm.vm_name; - const auto& state = vm.current_state(); + const auto& current_state = vm.current_state(); using St = VirtualMachine::State; const auto skip_states = {St::off, St::stopped, St::suspended}; - if (std::none_of(cbegin(skip_states), cend(skip_states), [&state](const auto& st) { return state == st; })) + if (std::none_of(cbegin(skip_states), cend(skip_states), [¤t_state](const auto& skip_state) { + return current_state == skip_state; + })) { delayed_shutdown_instances.erase(name); @@ -3140,6 +3145,28 @@ 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& current_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), [¤t_state](const auto& skip_state) { + return current_state == skip_state; + })) + { + 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 a5ae50b03e..6556052f63 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); diff --git a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp index 53a1d8e390..bc0dab2e66 100644 --- a/src/platform/backends/libvirt/libvirt_virtual_machine.cpp +++ b/src/platform/backends/libvirt/libvirt_virtual_machine.cpp @@ -18,6 +18,7 @@ #include "libvirt_virtual_machine.h" #include +#include #include #include #include @@ -345,41 +346,50 @@ 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); + std::unique_lock lock{state_mutex}; + auto domain = checked_vm_domain(); + state = refresh_instance_state_for_domain(domain.get(), state, libvirt_wrapper); - if (state == State::suspended) + + try { - mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); + check_state_for_shutdown(force); } - else + catch (const VMStateIdempotentException& e) { - drop_ssh_session(); + mpl::log(mpl::Level::info, vm_name, e.what()); + return; + } - if (state == State::running || state == State::delayed_shutdown || state == State::unknown) - { - 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) + if (force) // TODO delete suspension state if it exists + { + mpl::log(mpl::Level::info, vm_name, "Forcing shutdown"); + + libvirt_wrapper->virDomainDestroy(domain.get()); + + if (state == State::starting || state == State::restarting) { libvirt_wrapper->virDomainDestroy(domain.get()); state_wait.wait(lock, [this] { return shutdown_while_starting; }); - update_state(); } } + else + { + drop_ssh_session(); - lock.unlock(); + if (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(); monitor->on_shutdown(); } 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; diff --git a/src/platform/backends/lxd/lxd_virtual_machine.cpp b/src/platform/backends/lxd/lxd_virtual_machine.cpp index a398fca1d4..f12e911d4c 100644 --- a/src/platform/backends/lxd/lxd_virtual_machine.cpp +++ b/src/platform/backends/lxd/lxd_virtual_machine.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -261,26 +262,25 @@ void mp::LXDVirtualMachine::start() update_state(); } -void mp::LXDVirtualMachine::shutdown() +void mp::LXDVirtualMachine::shutdown(bool force) { - std::unique_lock lock{state_mutex}; - auto present_state = current_state(); + std::unique_lock lock{state_mutex}; - if (present_state == State::stopped) + const auto present_state = current_state(); + + try { - mpl::log(mpl::Level::debug, vm_name, "Ignoring stop request since instance is already stopped"); - return; + check_state_for_shutdown(force); } - - if (present_state == State::suspended) + catch (const VMStateIdempotentException& e) { - mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); + mpl::log(mpl::Level::info, vm_name, e.what()); return; } - request_state("stop"); + request_state("stop", {{"force", force}}); - state = State::stopped; + state = State::off; if (present_state == State::starting) { @@ -407,9 +407,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 a4f293656d..2a13420a46 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; @@ -85,7 +86,7 @@ class LXDVirtualMachine : public BaseVirtualMachine const QUrl url() const; 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 21979c00ed..5fd6524fe0 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -56,7 +57,8 @@ constexpr auto mount_data_key = "mount_data"; constexpr auto mount_source_key = "source"; constexpr auto mount_arguments_key = "arguments"; -constexpr int timeout = 300000; // 5 minute timeout for shutdown/suspend +constexpr int shutdown_timeout = 300000; // unit: ms, 5 minute timeout for shutdown/suspend +constexpr int kill_process_timeout = 5000; // unit: ms, 5 seconds timeout for killing the process bool use_cdrom_set(const QJsonObject& metadata) { @@ -343,31 +345,59 @@ void mp::QemuVirtualMachine::start() vm_process->write(qmp_execute_json("qmp_capabilities")); } -void mp::QemuVirtualMachine::shutdown() +void mp::QemuVirtualMachine::shutdown(bool force) { - if (state == State::suspended) + std::unique_lock lock{state_mutex}; + + try { - mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended")); + check_state_for_shutdown(force); } - else + catch (const VMStateIdempotentException& e) { - drop_ssh_session(); + mpl::log(mpl::Level::info, vm_name, e.what()); + return; + } + + if (force) + { + mpl::log(mpl::Level::info, vm_name, "Forcing shutdown"); - if ((state == State::running || state == State::delayed_shutdown || state == State::unknown) && vm_process && - vm_process->running()) + if (vm_process) { - vm_process->write(qmp_execute_json("system_powerdown")); - vm_process->wait_for_finished(timeout); + mpl::log(mpl::Level::info, vm_name, "Killing process"); + lock.unlock(); + vm_process->kill(); + if (vm_process != nullptr && !vm_process->wait_for_finished(kill_process_timeout)) + { + throw std::runtime_error{ + fmt::format("The QEMU process did not finish within {} seconds after being killed", + kill_process_timeout)}; + } } else { - if (state == State::starting) - update_shutdown_status = false; + mpl::log(mpl::Level::debug, vm_name, "No process to kill"); + } - if (vm_process) - { - vm_process->kill(); - } + if (state == State::suspended || mp::backend::instance_image_has_snapshot(desc.image.image_path, suspend_tag)) + { + mpl::log(mpl::Level::info, vm_name, "Deleting suspend image"); + mp::backend::delete_instance_suspend_image(desc.image.image_path, suspend_tag); + } + + state = State::off; + } + else + { + lock.unlock(); + + drop_ssh_session(); + + if (vm_process && vm_process->running()) + { + vm_process->write(qmp_execute_json("system_powerdown")); + vm_process->wait_for_finished(shutdown_timeout); } } } @@ -385,7 +415,8 @@ void mp::QemuVirtualMachine::suspend() drop_ssh_session(); vm_process->write(hmc_to_qmp_json(QString{"savevm "} + suspend_tag)); - vm_process->wait_for_finished(timeout); + vm_process->wait_for_finished(shutdown_timeout); + vm_process.reset(nullptr); } else if (state == State::off || state == State::suspended) diff --git a/src/platform/backends/qemu/qemu_virtual_machine.h b/src/platform/backends/qemu/qemu_virtual_machine.h index cacddfc4bb..9457e78721 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; diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index a0982ca771..34c299debe 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -139,25 +140,22 @@ std::optional wait_until_ssh_up_helper(mp::VirtualMachine* virtu } } // namespace -namespace multipass -{ - -BaseVirtualMachine::BaseVirtualMachine(VirtualMachine::State state, - const std::string& vm_name, - const SSHKeyProvider& key_provider, - const Path& instance_dir) +mp::BaseVirtualMachine::BaseVirtualMachine(VirtualMachine::State state, + const std::string& vm_name, + const SSHKeyProvider& key_provider, + const Path& instance_dir) : VirtualMachine{state, vm_name, instance_dir}, key_provider{key_provider} { } -BaseVirtualMachine::BaseVirtualMachine(const std::string& vm_name, - const SSHKeyProvider& key_provider, - const Path& instance_dir) +mp::BaseVirtualMachine::BaseVirtualMachine(const std::string& vm_name, + const SSHKeyProvider& key_provider, + const Path& instance_dir) : VirtualMachine{vm_name, instance_dir}, key_provider{key_provider} { } -void BaseVirtualMachine::apply_extra_interfaces_and_instance_id_to_cloud_init( +void mp::BaseVirtualMachine::apply_extra_interfaces_and_instance_id_to_cloud_init( const std::string& default_mac_addr, const std::vector& extra_interfaces, const std::string& new_instance_id) const @@ -171,8 +169,8 @@ void BaseVirtualMachine::apply_extra_interfaces_and_instance_id_to_cloud_init( cloud_init_config_iso_file_path); } -void BaseVirtualMachine::add_extra_interface_to_instance_cloud_init(const std::string& default_mac_addr, - const NetworkInterface& extra_interface) const +void mp::BaseVirtualMachine::add_extra_interface_to_instance_cloud_init(const std::string& default_mac_addr, + const NetworkInterface& extra_interface) const { const std::filesystem::path cloud_init_config_iso_file_path = std::filesystem::path{instance_dir.absolutePath().toStdString()} / "cloud-init-config.iso"; @@ -182,7 +180,7 @@ void BaseVirtualMachine::add_extra_interface_to_instance_cloud_init(const std::s cloud_init_config_iso_file_path); } -std::string BaseVirtualMachine::get_instance_id_from_the_cloud_init() const +std::string mp::BaseVirtualMachine::get_instance_id_from_the_cloud_init() const { const std::filesystem::path cloud_init_config_iso_file_path = std::filesystem::path{instance_dir.absolutePath().toStdString()} / "cloud-init-config.iso"; @@ -190,7 +188,38 @@ std::string BaseVirtualMachine::get_instance_id_from_the_cloud_init() const return MP_CLOUD_INIT_FILE_OPS.get_instance_id_from_cloud_init(cloud_init_config_iso_file_path); } -std::string BaseVirtualMachine::ssh_exec(const std::string& cmd, bool whisper) +void mp::BaseVirtualMachine::check_state_for_shutdown(bool force) +{ + const std::string force_statement{"Use --force to override."}; + + // A mutex should already be locked by the caller here + if (state == State::off || state == State::stopped) + { + throw VMStateIdempotentException{"Ignoring shutdown since instance is already stopped."}; + } + + if (force) + { + return; + } + + if (state == State::suspending) + { + throw VMStateInvalidException{fmt::format("Cannot stop instance while suspending. {}", force_statement)}; + } + + if (state == State::suspended) + { + throw VMStateInvalidException{fmt::format("Cannot stop suspended instance. {}", force_statement)}; + } + + if (state == State::starting || state == State::restarting) + { + throw VMStateInvalidException{fmt::format("Cannot stop instance while starting. {}", force_statement)}; + } +} + +std::string mp::BaseVirtualMachine::ssh_exec(const std::string& cmd, bool whisper) { const std::unique_lock lock{state_mutex}; @@ -227,7 +256,7 @@ std::string BaseVirtualMachine::ssh_exec(const std::string& cmd, bool whisper) assert(false && "we should never reach here"); } -void BaseVirtualMachine::renew_ssh_session() +void mp::BaseVirtualMachine::renew_ssh_session() { if (!MP_UTILS.is_running(current_state())) // spend time updating state only if we need a new session throw SSHException{fmt::format("SSH unavailable on instance {}: not running", vm_name)}; @@ -239,14 +268,14 @@ void BaseVirtualMachine::renew_ssh_session() ssh_session.emplace(ssh_hostname(), ssh_port(), ssh_username(), key_provider); } -void BaseVirtualMachine::wait_until_ssh_up(std::chrono::milliseconds timeout) +void mp::BaseVirtualMachine::wait_until_ssh_up(std::chrono::milliseconds timeout) { drop_ssh_session(); ssh_session = wait_until_ssh_up_helper(this, timeout, key_provider); mpl::log(logging::Level::debug, vm_name, "Caching initial SSH session"); } -void BaseVirtualMachine::wait_for_cloud_init(std::chrono::milliseconds timeout) +void mp::BaseVirtualMachine::wait_for_cloud_init(std::chrono::milliseconds timeout) { auto action = [this] { ensure_vm_is_running(); @@ -270,7 +299,7 @@ void BaseVirtualMachine::wait_for_cloud_init(std::chrono::milliseconds timeout) mp::utils::try_action_for(on_timeout, timeout, action); } -std::vector BaseVirtualMachine::get_all_ipv4() +std::vector mp::BaseVirtualMachine::get_all_ipv4() { std::vector all_ipv4; @@ -303,7 +332,7 @@ std::vector BaseVirtualMachine::get_all_ipv4() return all_ipv4; } -auto BaseVirtualMachine::view_snapshots() const -> SnapshotVista +auto mp::BaseVirtualMachine::view_snapshots() const -> SnapshotVista { require_snapshots_support(); SnapshotVista ret; @@ -317,7 +346,7 @@ auto BaseVirtualMachine::view_snapshots() const -> SnapshotVista return ret; } -std::shared_ptr BaseVirtualMachine::get_snapshot(const std::string& name) const +std::shared_ptr mp::BaseVirtualMachine::get_snapshot(const std::string& name) const { require_snapshots_support(); const std::unique_lock lock{snapshot_mutex}; @@ -331,7 +360,7 @@ std::shared_ptr BaseVirtualMachine::get_snapshot(const std::stri } } -std::shared_ptr BaseVirtualMachine::get_snapshot(int index) const +std::shared_ptr mp::BaseVirtualMachine::get_snapshot(int index) const { require_snapshots_support(); const std::unique_lock lock{snapshot_mutex}; @@ -344,19 +373,19 @@ std::shared_ptr BaseVirtualMachine::get_snapshot(int index) cons fmt::format("No snapshot with given index in instance; instance name: {}; snapshot index: {}", vm_name, index)}; } -std::shared_ptr BaseVirtualMachine::get_snapshot(const std::string& name) +std::shared_ptr mp::BaseVirtualMachine::get_snapshot(const std::string& name) { return std::const_pointer_cast(std::as_const(*this).get_snapshot(name)); } -std::shared_ptr BaseVirtualMachine::get_snapshot(int index) +std::shared_ptr mp::BaseVirtualMachine::get_snapshot(int index) { return std::const_pointer_cast(std::as_const(*this).get_snapshot(index)); } -void BaseVirtualMachine::take_snapshot_rollback_helper(SnapshotMap::iterator it, - std::shared_ptr& old_head, - int old_count) +void mp::BaseVirtualMachine::take_snapshot_rollback_helper(SnapshotMap::iterator it, + std::shared_ptr& old_head, + int old_count) { if (old_head != head_snapshot) { @@ -375,7 +404,7 @@ void BaseVirtualMachine::take_snapshot_rollback_helper(SnapshotMap::iterator it, snapshots.erase(it); } -auto BaseVirtualMachine::make_take_snapshot_rollback(SnapshotMap::iterator it) +auto mp::BaseVirtualMachine::make_take_snapshot_rollback(SnapshotMap::iterator it) { return sg::make_scope_guard( // best effort to rollback [this, it = it, old_head = head_snapshot, old_count = snapshot_count]() mutable noexcept { @@ -383,9 +412,9 @@ auto BaseVirtualMachine::make_take_snapshot_rollback(SnapshotMap::iterator it) }); } -std::shared_ptr BaseVirtualMachine::take_snapshot(const VMSpecs& specs, - const std::string& snapshot_name, - const std::string& comment) +std::shared_ptr mp::BaseVirtualMachine::take_snapshot(const VMSpecs& specs, + const std::string& snapshot_name, + const std::string& comment) { require_snapshots_support(); @@ -417,7 +446,7 @@ std::shared_ptr BaseVirtualMachine::take_snapshot(const VMSpecs& return ret; } -bool BaseVirtualMachine::updated_deleted_head(std::shared_ptr& snapshot, const Path& head_path) +bool mp::BaseVirtualMachine::updated_deleted_head(std::shared_ptr& snapshot, const Path& head_path) { if (head_snapshot == snapshot) { @@ -429,16 +458,16 @@ bool BaseVirtualMachine::updated_deleted_head(std::shared_ptr& snapsho return false; } -auto BaseVirtualMachine::make_deleted_head_rollback(const Path& head_path, const bool& wrote_head) +auto mp::BaseVirtualMachine::make_deleted_head_rollback(const Path& head_path, const bool& wrote_head) { return sg::make_scope_guard([this, old_head = head_snapshot, &head_path, &wrote_head]() mutable noexcept { deleted_head_rollback_helper(head_path, wrote_head, old_head); }); } -void BaseVirtualMachine::deleted_head_rollback_helper(const Path& head_path, - const bool& wrote_head, - std::shared_ptr& old_head) +void mp::BaseVirtualMachine::deleted_head_rollback_helper(const Path& head_path, + const bool& wrote_head, + std::shared_ptr& old_head) { if (head_snapshot != old_head) { @@ -452,15 +481,15 @@ void BaseVirtualMachine::deleted_head_rollback_helper(const Path& head_path, } } -auto BaseVirtualMachine::make_parent_update_rollback(const std::shared_ptr& deleted_parent, - std::vector& updated_parents) const +auto mp::BaseVirtualMachine::make_parent_update_rollback(const std::shared_ptr& deleted_parent, + std::vector& updated_parents) const { return sg::make_scope_guard([this, &updated_parents, deleted_parent]() noexcept { top_catch_all(vm_name, &update_parents_rollback_helper, deleted_parent, updated_parents); }); } -void BaseVirtualMachine::delete_snapshot_helper(std::shared_ptr& snapshot) +void mp::BaseVirtualMachine::delete_snapshot_helper(std::shared_ptr& snapshot) { // Update head if deleted auto wrote_head = false; @@ -481,8 +510,8 @@ void BaseVirtualMachine::delete_snapshot_helper(std::shared_ptr& snaps rollback_head.dismiss(); } -void BaseVirtualMachine::update_parents(std::shared_ptr& deleted_parent, - std::vector& updated_parents) +void mp::BaseVirtualMachine::update_parents(std::shared_ptr& deleted_parent, + std::vector& updated_parents) { auto new_parent = deleted_parent->get_parent(); for (auto& [ignore, other] : snapshots) @@ -496,7 +525,7 @@ void BaseVirtualMachine::update_parents(std::shared_ptr& deleted_paren } template -auto BaseVirtualMachine::make_reinsert_guard(NodeT& snapshot_node) +auto mp::BaseVirtualMachine::make_reinsert_guard(NodeT& snapshot_node) { return sg::make_scope_guard([this, &snapshot_node]() noexcept { top_catch_all(vm_name, [this, &snapshot_node] { @@ -510,7 +539,7 @@ auto BaseVirtualMachine::make_reinsert_guard(NodeT& snapshot_node) ; } -void BaseVirtualMachine::rename_snapshot(const std::string& old_name, const std::string& new_name) +void mp::BaseVirtualMachine::rename_snapshot(const std::string& old_name, const std::string& new_name) { if (old_name == new_name) return; @@ -531,7 +560,7 @@ void BaseVirtualMachine::rename_snapshot(const std::string& old_name, const std: snapshot_node.mapped()->set_name(new_name); } -void BaseVirtualMachine::delete_snapshot(const std::string& name) +void mp::BaseVirtualMachine::delete_snapshot(const std::string& name) { const std::unique_lock lock{snapshot_mutex}; @@ -546,7 +575,7 @@ void BaseVirtualMachine::delete_snapshot(const std::string& name) mpl::log(mpl::Level::debug, vm_name, fmt::format("Snapshot deleted: {}", name)); } -void BaseVirtualMachine::load_snapshots() +void mp::BaseVirtualMachine::load_snapshots() { const std::unique_lock lock{snapshot_mutex}; @@ -560,7 +589,7 @@ void BaseVirtualMachine::load_snapshots() load_generic_snapshot_info(); } -std::vector BaseVirtualMachine::get_childrens_names(const Snapshot* parent) const +std::vector mp::BaseVirtualMachine::get_childrens_names(const Snapshot* parent) const { require_snapshots_support(); std::vector children; @@ -572,7 +601,7 @@ std::vector BaseVirtualMachine::get_childrens_names(const Snapshot* return children; } -void BaseVirtualMachine::load_generic_snapshot_info() +void mp::BaseVirtualMachine::load_generic_snapshot_info() { try { @@ -589,7 +618,7 @@ void BaseVirtualMachine::load_generic_snapshot_info() } template -void BaseVirtualMachine::log_latest_snapshot(LockT lock) const +void mp::BaseVirtualMachine::log_latest_snapshot(LockT lock) const { auto num_snapshots = static_cast(snapshots.size()); auto parent_name = head_snapshot->get_parents_name(); @@ -610,7 +639,7 @@ void BaseVirtualMachine::log_latest_snapshot(LockT lock) const } } -void BaseVirtualMachine::load_snapshot(const QString& filename) +void mp::BaseVirtualMachine::load_snapshot(const QString& filename) { auto snapshot = make_specific_snapshot(filename); const auto& name = snapshot->get_name(); @@ -623,19 +652,19 @@ void BaseVirtualMachine::load_snapshot(const QString& filename) } } -auto BaseVirtualMachine::make_common_file_rollback(const Path& file_path, - QFile& file, - const std::string& old_contents) const +auto mp::BaseVirtualMachine::make_common_file_rollback(const Path& file_path, + QFile& file, + const std::string& old_contents) const { return sg::make_scope_guard([this, &file_path, &file, old_contents, existed = file.exists()]() noexcept { common_file_rollback_helper(file_path, file, old_contents, existed); }); } -void BaseVirtualMachine::common_file_rollback_helper(const Path& file_path, - QFile& file, - const std::string& old_contents, - bool existed) const +void mp::BaseVirtualMachine::common_file_rollback_helper(const Path& file_path, + QFile& file, + const std::string& old_contents, + bool existed) const { // best effort, ignore returns if (!existed) @@ -646,7 +675,7 @@ void BaseVirtualMachine::common_file_rollback_helper(const Path& file_path, }); } -void BaseVirtualMachine::persist_generic_snapshot_info() const +void mp::BaseVirtualMachine::persist_generic_snapshot_info() const { assert(head_snapshot); @@ -667,18 +696,18 @@ void BaseVirtualMachine::persist_generic_snapshot_info() const head_file_rollback.dismiss(); } -void BaseVirtualMachine::persist_head_snapshot_index(const Path& head_path) const +void mp::BaseVirtualMachine::persist_head_snapshot_index(const Path& head_path) const { auto head_index = head_snapshot ? head_snapshot->get_index() : 0; MP_UTILS.make_file_with_content(head_path.toStdString(), std::to_string(head_index) + "\n", yes_overwrite); } -std::string BaseVirtualMachine::generate_snapshot_name() const +std::string mp::BaseVirtualMachine::generate_snapshot_name() const { return fmt::format("snapshot{}", snapshot_count + 1); } -auto BaseVirtualMachine::make_restore_rollback(const Path& head_path, VMSpecs& specs) +auto mp::BaseVirtualMachine::make_restore_rollback(const Path& head_path, VMSpecs& specs) { return sg::make_scope_guard([this, &head_path, old_head = head_snapshot, old_specs = specs, &specs]() noexcept { top_catch_all(vm_name, @@ -691,10 +720,10 @@ auto BaseVirtualMachine::make_restore_rollback(const Path& head_path, VMSpecs& s }); } -void BaseVirtualMachine::restore_rollback_helper(const Path& head_path, - const std::shared_ptr& old_head, - const VMSpecs& old_specs, - VMSpecs& specs) +void mp::BaseVirtualMachine::restore_rollback_helper(const Path& head_path, + const std::shared_ptr& old_head, + const VMSpecs& old_specs, + VMSpecs& specs) { // best effort only specs = old_specs; @@ -705,7 +734,7 @@ void BaseVirtualMachine::restore_rollback_helper(const Path& head_path, } } -void BaseVirtualMachine::restore_snapshot(const std::string& name, VMSpecs& specs) +void mp::BaseVirtualMachine::restore_snapshot(const std::string& name, VMSpecs& specs) { const std::unique_lock lock{snapshot_mutex}; @@ -745,21 +774,21 @@ void BaseVirtualMachine::restore_snapshot(const std::string& name, VMSpecs& spec rollback.dismiss(); } -std::shared_ptr BaseVirtualMachine::make_specific_snapshot(const std::string& /*snapshot_name*/, - const std::string& /*comment*/, - const std::string& /*instance_id*/, - const VMSpecs& /*specs*/, - std::shared_ptr /*parent*/) +std::shared_ptr mp::BaseVirtualMachine::make_specific_snapshot(const std::string& /*snapshot_name*/, + const std::string& /*comment*/, + const std::string& /*instance_id*/, + const VMSpecs& /*specs*/, + std::shared_ptr /*parent*/) { throw NotImplementedOnThisBackendException{"snapshots"}; } -std::shared_ptr BaseVirtualMachine::make_specific_snapshot(const QString& /*filename*/) +std::shared_ptr mp::BaseVirtualMachine::make_specific_snapshot(const QString& /*filename*/) { throw NotImplementedOnThisBackendException{"snapshots"}; } -void BaseVirtualMachine::drop_ssh_session() +void mp::BaseVirtualMachine::drop_ssh_session() { if (ssh_session) { @@ -767,5 +796,3 @@ void BaseVirtualMachine::drop_ssh_session() ssh_session.reset(); } } - -} // namespace multipass diff --git a/src/platform/backends/shared/base_virtual_machine.h b/src/platform/backends/shared/base_virtual_machine.h index f8cff6dcc7..8c36e14499 100644 --- a/src/platform/backends/shared/base_virtual_machine.h +++ b/src/platform/backends/shared/base_virtual_machine.h @@ -104,6 +104,8 @@ class BaseVirtualMachine : public VirtualMachine const std::string& new_instance_id) const; virtual std::string get_instance_id_from_the_cloud_init() const; + virtual void check_state_for_shutdown(bool force); + private: using SnapshotMap = std::unordered_map>; diff --git a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp index 9ce5c514e8..58d343effc 100644 --- a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp +++ b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.cpp @@ -102,3 +102,10 @@ bool mp::backend::instance_image_has_snapshot(const mp::Path& image_path, QStrin QRegularExpression regex{snapshot_tag.append(R"(\s)")}; return QString{process->read_all_standard_output()}.contains(regex); } + +void mp::backend::delete_instance_suspend_image(const Path& image_path, const QString& suspend_tag) +{ + checked_exec_qemu_img( + std::make_unique(QStringList{"snapshot", "-d", suspend_tag, image_path}, image_path), + "Failed to delete suspend image"); +} diff --git a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h index 89cb0dcf4b..c10b70c187 100644 --- a/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h +++ b/src/platform/backends/shared/qemu_img_utils/qemu_img_utils.h @@ -43,6 +43,7 @@ void resize_instance_image(const MemorySize& disk_space, const multipass::Path& Path convert_to_qcow_if_necessary(const Path& image_path); void amend_to_qcow2_v3(const Path& image_path); bool instance_image_has_snapshot(const Path& image_path, QString snapshot_tag); +void delete_instance_suspend_image(const Path& image_path, const QString& suspend_tag); } // namespace backend } // namespace multipass diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index 41bc54e907..330b5671c2 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -399,6 +399,7 @@ message StopRequest { int32 time_minutes = 2; bool cancel_shutdown = 3; int32 verbosity_level = 4; + bool force_stop = 5; } message StopReply { diff --git a/tests/libvirt/test_libvirt_backend.cpp b/tests/libvirt/test_libvirt_backend.cpp index de40040ce4..771a33cfcb 100644 --- a/tests/libvirt/test_libvirt_backend.cpp +++ b/tests/libvirt/test_libvirt_backend.cpp @@ -18,6 +18,7 @@ #include "tests/common.h" #include "tests/fake_handle.h" #include "tests/mock_backend_utils.h" +#include "tests/mock_logger.h" #include "tests/mock_ssh.h" #include "tests/mock_status_monitor.h" #include "tests/stub_ssh_key_provider.h" @@ -64,6 +65,8 @@ struct LibVirtBackend : public Test // This indicates that LibvirtWrapper should open the test executable std::string fake_libvirt_path{""}; + mpt::MockLogger::Scope logger_scope{mpt::MockLogger::inject()}; + mpt::MockBackend::GuardedMock backend_attr{mpt::MockBackend::inject()}; mpt::MockBackend* mock_backend = backend_attr.first; }; @@ -451,7 +454,7 @@ TEST_F(LibVirtBackend, shutdown_while_starting_throws_and_sets_correct_state) ASSERT_EQ(machine->state, mp::VirtualMachine::State::starting); - mp::AutoJoinThread thread = [&machine] { machine->shutdown(); }; + mp::AutoJoinThread thread = [&machine] { machine->shutdown(true); }; using namespace std::chrono_literals; while (!destroy_called) @@ -463,6 +466,54 @@ TEST_F(LibVirtBackend, shutdown_while_starting_throws_and_sets_correct_state) EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); } +TEST_F(LibVirtBackend, machineInOffStateLogsAndIgnoresShutdown) +{ + mp::LibVirtVirtualMachineFactory backend{data_dir.path(), fake_libvirt_path}; + NiceMock mock_monitor; + auto machine = backend.create_virtual_machine(default_description, key_provider, mock_monitor); + + EXPECT_THAT(machine->current_state(), Eq(mp::VirtualMachine::State::off)); + + backend.libvirt_wrapper->virDomainGetState = [](auto, auto state, auto, auto) { + *state = VIR_DOMAIN_SHUTOFF; + return 0; + }; + + logger_scope.mock_logger->screen_logs(mpl::Level::info); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Ignoring shutdown since instance is already stopped."); + + machine->shutdown(); + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); +} + +TEST_F(LibVirtBackend, machineNoForceCannotShutdownLogsAndThrows) +{ + const std::string error_msg{"Not working"}; + + mp::LibVirtVirtualMachineFactory backend{data_dir.path(), fake_libvirt_path}; + NiceMock mock_monitor; + auto machine = backend.create_virtual_machine(default_description, key_provider, mock_monitor); + + backend.libvirt_wrapper->virDomainGetState = [](auto, auto state, auto, auto) { + *state = VIR_DOMAIN_RUNNING; + return 0; + }; + + backend.libvirt_wrapper->virDomainShutdown = [](auto) { return -1; }; + + auto virGetLastErrorMessage = [&error_msg] { return error_msg.c_str(); }; + static auto static_virGetLastErrorMessage = virGetLastErrorMessage; + backend.libvirt_wrapper->virGetLastErrorMessage = [] { return static_virGetLastErrorMessage(); }; + + logger_scope.mock_logger->screen_logs(mpl::Level::warning); + logger_scope.mock_logger->expect_log(mpl::Level::warning, error_msg); + + MP_EXPECT_THROW_THAT(machine->shutdown(), + std::runtime_error, + mpt::match_what(AllOf(HasSubstr("pied-piper-valley"), HasSubstr(error_msg)))); +} + TEST_F(LibVirtBackend, lists_no_networks) { mp::LibVirtVirtualMachineFactory backend(data_dir.path(), fake_libvirt_path); diff --git a/tests/lxd/test_lxd_backend.cpp b/tests/lxd/test_lxd_backend.cpp index 268b853cdb..7f19e48e51 100644 --- a/tests/lxd/test_lxd_backend.cpp +++ b/tests/lxd/test_lxd_backend.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -1690,16 +1691,18 @@ TEST_F(LXDBackend, shutdown_while_stopped_does_nothing_and_logs_debug) EXPECT_CALL(mock_monitor, persist_state_for(_, _)); EXPECT_CALL( *logger_scope.mock_logger, - log(Eq(mpl::Level::debug), mpt::MockLogger::make_cstring_matcher(StrEq("pied-piper-valley")), - mpt::MockLogger::make_cstring_matcher(StrEq("Ignoring stop request since instance is already stopped")))); + log(Eq(mpl::Level::info), + mpt::MockLogger::make_cstring_matcher(StrEq("pied-piper-valley")), + mpt::MockLogger::make_cstring_matcher(StrEq("Ignoring shutdown since instance is already stopped.")))); machine.shutdown(); EXPECT_EQ(machine.current_state(), mp::VirtualMachine::State::stopped); } -TEST_F(LXDBackend, shutdown_while_frozen_does_nothing_and_logs_info) +TEST_F(LXDBackend, shutdown_while_frozen_throws_and_logs_info) { + const std::string error_msg{"Cannot stop suspended instance. Use --force to override."}; mpt::MockVMStatusMonitor mock_monitor; EXPECT_CALL(*mock_network_access_manager, createRequest(_, _, _)).WillRepeatedly([](auto, auto request, auto) { @@ -1726,11 +1729,8 @@ TEST_F(LXDBackend, shutdown_while_frozen_does_nothing_and_logs_info) ASSERT_EQ(machine.current_state(), mp::VirtualMachine::State::suspended); EXPECT_CALL(mock_monitor, persist_state_for(_, _)); - EXPECT_CALL(*logger_scope.mock_logger, - log(Eq(mpl::Level::info), mpt::MockLogger::make_cstring_matcher(StrEq("pied-piper-valley")), - mpt::MockLogger::make_cstring_matcher(StrEq("Ignoring shutdown issued while suspended")))); - machine.shutdown(); + MP_EXPECT_THROW_THAT(machine.shutdown(), mp::VMStateInvalidException, mpt::match_what(StrEq(error_msg))); EXPECT_EQ(machine.current_state(), mp::VirtualMachine::State::suspended); } @@ -1843,9 +1843,9 @@ TEST_F(LXDBackend, shutdown_while_starting_throws_and_sets_correct_state) ASSERT_EQ(machine.state, mp::VirtualMachine::State::starting); - mp::AutoJoinThread thread = [&machine] { machine.shutdown(); }; + mp::AutoJoinThread thread = [&machine] { machine.shutdown(true); }; // force shutdown - while (machine.state != mp::VirtualMachine::State::stopped) + while (machine.state != mp::VirtualMachine::State::off) std::this_thread::sleep_for(1ms); MP_EXPECT_THROW_THAT(machine.ensure_vm_is_running(1ms), mp::StartException, diff --git a/tests/mock_virtual_machine.h b/tests/mock_virtual_machine.h index ca5fb7f3e8..b5d1d44625 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/qemu/test_qemu_backend.cpp b/tests/qemu/test_qemu_backend.cpp index 8857baa732..5d3b54a2ca 100644 --- a/tests/qemu/test_qemu_backend.cpp +++ b/tests/qemu/test_qemu_backend.cpp @@ -38,6 +38,7 @@ #include #include +#include #include #include #include @@ -69,6 +70,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; @@ -171,6 +173,8 @@ struct QemuBackend : public mpt::TestWithMockedBinPath } }; + mpt::MockLogger::Scope logger_scope{mpt::MockLogger::inject()}; + mpt::SetEnvScope env_scope{"DISABLE_APPARMOR", "1"}; std::unique_ptr process_factory{mpt::MockProcessFactory::Inject()}; @@ -283,7 +287,7 @@ TEST_F(QemuBackend, throws_when_shutdown_while_starting) mp::AutoJoinThread thread{[&machine, vmproc] { ON_CALL(*vmproc, running()).WillByDefault(Return(false)); - machine->shutdown(); + machine->shutdown(true); }}; using namespace std::chrono_literals; @@ -368,6 +372,168 @@ TEST_F(QemuBackend, machine_unknown_state_properly_shuts_down) EXPECT_THAT(machine->current_state(), Eq(mp::VirtualMachine::State::off)); } +TEST_F(QemuBackend, suspendedStateNoForceShutdownThrows) +{ + const std::string error_msg{"Cannot stop suspended instance. Use --force to override."}; + + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + 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; + + MP_EXPECT_THROW_THAT(machine->shutdown(), mp::VMStateInvalidException, mpt::match_what(StrEq(error_msg))); + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::suspended); +} + +TEST_F(QemuBackend, suspendingStateNoForceShutdownThrows) +{ + const std::string error_msg{"Cannot stop instance while suspending. Use --force to override."}; + + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + 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::suspending; + + MP_EXPECT_THROW_THAT(machine->shutdown(), mp::VMStateInvalidException, mpt::match_what(StrEq(error_msg))); + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::suspending); +} + +TEST_F(QemuBackend, startingStateNoForceShutdownThrows) +{ + const std::string error_msg{"Cannot stop instance while starting. Use --force to override."}; + + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + 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::starting; + + MP_EXPECT_THROW_THAT(machine->shutdown(), mp::VMStateInvalidException, mpt::match_what(StrEq(error_msg))); + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::starting); +} + +TEST_F(QemuBackend, forceShutdownKillsProcessAndLogs) +{ + 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); + }); + + 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, "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"); + + 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, forceShutdownNoProcessLogs) +{ + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + logger_scope.mock_logger->screen_logs(mpl::Level::debug); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Forcing shutdown"); + logger_scope.mock_logger->expect_log(mpl::Level::debug, "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); + + machine->state = mp::VirtualMachine::State::unknown; + + machine->shutdown(true); // force shutdown + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); +} + +TEST_F(QemuBackend, forceShutdownSuspendDeletesSuspendImageAndOffState) +{ + EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { + return std::move(mock_qemu_platform); + }); + + auto factory = mpt::StubProcessFactory::Inject(); + + logger_scope.mock_logger->screen_logs(mpl::Level::debug); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Forcing shutdown"); + logger_scope.mock_logger->expect_log(mpl::Level::debug, "No process to kill"); + logger_scope.mock_logger->expect_log(mpl::Level::info, "Deleting suspend image"); + + 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(true); + + EXPECT_EQ(machine->current_state(), mp::VirtualMachine::State::off); + + auto processes = factory->process_list(); + EXPECT_TRUE(std::find_if(processes.cbegin(), + processes.cend(), + [](const mpt::StubProcessFactory::ProcessInfo& process_info) { + return process_info.command == "qemu-img" && process_info.arguments.contains("-d") && + process_info.arguments.contains(suspend_tag); + }) != processes.cend()); +} + TEST_F(QemuBackend, verify_dnsmasq_qemuimg_and_qemu_processes_created) { EXPECT_CALL(*mock_qemu_platform_factory, make_qemu_platform(_)).WillOnce([this](auto...) { @@ -715,7 +881,6 @@ TEST_F(QemuBackend, logsErrorOnFailureToConvertToQcow2V3UponConstruction) return handle_external_process_calls(process); }); - auto logger_scope = mpt::MockLogger::inject(); logger_scope.mock_logger->screen_logs(mpl::Level::error); logger_scope.mock_logger->expect_log(mpl::Level::error, "Failed to amend image to QCOW2 v3"); @@ -738,12 +903,12 @@ struct MockQemuVM : public mpt::MockVirtualMachineT TEST_F(QemuBackend, dropsSSHSessionWhenStopping) { NiceMock machine{"mock-qemu-vm", key_provider}; - machine.state = multipass::VirtualMachine::State::stopped; + machine.state = multipass::VirtualMachine::State::running; 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) diff --git a/tests/stub_virtual_machine.h b/tests/stub_virtual_machine.h index 03b107fab7..3789d272cc 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 { } diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 3479f958c0..9ea9c8282d 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -133,7 +133,7 @@ struct StubBaseVirtualMachine : public mp::BaseVirtualMachine state = St::running; } - void shutdown() override + void shutdown(bool force = false) override { state = St::off; } diff --git a/tests/test_cli_client.cpp b/tests/test_cli_client.cpp index 0751851d6c..41261fed65 100644 --- a/tests/test_cli_client.cpp +++ b/tests/test_cli_client.cpp @@ -2519,6 +2519,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) {