diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index df956254a1c..5960a695bb8 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -1034,6 +1034,8 @@ mp::InstanceStatus::Status grpc_instance_status_for(const mp::VirtualMachine::St return mp::InstanceStatus::SUSPENDING; case mp::VirtualMachine::State::suspended: return mp::InstanceStatus::SUSPENDED; + case mp::VirtualMachine::State::unavailable: + return mp::InstanceStatus::UNAVAILABLE; case mp::VirtualMachine::State::unknown: default: return mp::InstanceStatus::UNKNOWN; @@ -2083,6 +2085,12 @@ try continue; } + if (vm->current_state() == VirtualMachine::State::unavailable) + { + add_fmt_to(errors, "instance '{}' is not available", name); + continue; + } + auto& vm_mounts = mounts[name]; if (vm_mounts.find(target_path) != vm_mounts.end()) { @@ -2259,9 +2267,13 @@ try continue; } case VirtualMachine::State::suspending: + case VirtualMachine::State::unavailable: + // TODO: format State directly fmt::format_to(std::back_inserter(start_errors), - "Cannot start the instance '{}' while suspending.", - name); + "Cannot start the instance '{}' while {}.", + name, + vm.current_state() == VirtualMachine::State::suspending ? "suspending" + : "unavailable"); continue; case VirtualMachine::State::delayed_shutdown: delayed_shutdown_instances.erase(name); @@ -2364,6 +2376,14 @@ try if (status.ok()) { status = cmd_vms(instance_selection.operative_selection, [this](auto& vm) { + if (vm.current_state() == VirtualMachine::State::unavailable) + { + mpl::log(mpl::Level::info, + vm.vm_name, + "Ignoring suspend since instance is unavailable."); + return grpc::Status::OK; + } + stop_mounts(vm.vm_name); vm.suspend(); @@ -2485,6 +2505,14 @@ try { const auto& instance_name = vm_it->first; + if (vm_it->second->current_state() == VirtualMachine::State::unavailable) + { + mpl::log(mpl::Level::info, + instance_name, + "Ignoring delete since instance is unavailable."); + continue; + } + auto snapshot_pick_it = instance_snapshots_map.find(instance_name); const auto& [pick, all] = snapshot_pick_it == instance_snapshots_map.end() ? SnapshotPick{{}, true} @@ -2533,12 +2561,19 @@ try const auto target_path = QDir::cleanPath(QString::fromStdString(path_entry.target_path())).toStdString(); - if (operative_instances.find(name) == operative_instances.end()) + auto vm = operative_instances.find(name); + if (vm == operative_instances.end()) { add_fmt_to(errors, "instance '{}' does not exist", name); continue; } + if (vm->second->current_state() == VirtualMachine::State::unavailable) + { + mpl::log(mpl::Level::info, name, "Ignoring umount since instance unavailable."); + continue; + } + auto& vm_spec_mounts = vm_instance_specs[name].mounts; auto& vm_mounts = mounts[name]; diff --git a/src/platform/backends/hyperv/hyperv_virtual_machine.cpp b/src/platform/backends/hyperv/hyperv_virtual_machine.cpp index f0dca2c39b4..3229bfac75d 100644 --- a/src/platform/backends/hyperv/hyperv_virtual_machine.cpp +++ b/src/platform/backends/hyperv/hyperv_virtual_machine.cpp @@ -408,9 +408,12 @@ void mp::HyperVVirtualMachine::suspend() update_state(); } } - else if (present_state == State::stopped) + else if (present_state == State::stopped || present_state == State::unavailable) { - mpl::info(vm_name, "Ignoring suspend issued while stopped"); + // TODO: format state directly + mpl::info(vm_name, + "Ignoring suspend since instance is {}", + (present_state == State::unavailable) ? "unavailable" : "stopped"); } monitor->on_suspend(); diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index ed53cf6ab60..69100293731 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -426,9 +426,10 @@ void mp::QemuVirtualMachine::suspend() vm_process.reset(nullptr); } - else if (state == State::off || state == State::suspended) + else if (state == State::off || state == State::suspended || state == State::unavailable) { - mpl::info(vm_name, "Ignoring suspend issued while stopped/suspended"); + // TODO: format state directly + mpl::info(vm_name, "Ignoring suspend issued while stopped/suspended/unavailable"); monitor->on_suspend(); } } diff --git a/src/platform/backends/shared/base_availability_zone.cpp b/src/platform/backends/shared/base_availability_zone.cpp index 4c983af15c3..7cd17fee068 100644 --- a/src/platform/backends/shared/base_availability_zone.cpp +++ b/src/platform/backends/shared/base_availability_zone.cpp @@ -23,6 +23,8 @@ #include +#include + #include namespace mpl = multipass::logging; @@ -115,10 +117,42 @@ void BaseAvailabilityZone::set_available(const bool new_available) return; m.available = new_available; - serialize(); + auto serialize_guard = sg::make_scope_guard([this]() noexcept { + try + { + serialize(); + } + catch (const std::exception& e) + { + mpl::error(m.name, "Failed to serialize availability zone: {}", e.what()); + } + }); - for (auto& vm : m.vms) - vm.get().set_available(m.available); + try + { + for (auto& vm : m.vms) + vm.get().set_available(new_available); + } + catch (...) + { + // if an error occurs fallback to available. + m.available = true; + + // make sure nothing is still unavailable. + for (auto& vm : m.vms) + { + // setting the state here breaks encapsulation, but it's already broken. + std::unique_lock vm_lock{vm.get().state_mutex}; + if (vm.get().current_state() == VirtualMachine::State::unavailable) + { + vm.get().state = VirtualMachine::State::off; + vm.get().update_state(); + } + } + + // rethrow the error so something else can deal with it. + throw; + } } void BaseAvailabilityZone::add_vm(VirtualMachine& vm) diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index b45e4013774..575cd5dd3c0 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -200,9 +200,12 @@ std::string mp::BaseVirtualMachine::get_instance_id_from_the_cloud_init() const void mp::BaseVirtualMachine::check_state_for_shutdown(ShutdownPolicy shutdown_policy) { // A mutex should already be locked by the caller here - if (state == State::off || state == State::stopped) + if (state == State::off || state == State::stopped || state == State::unavailable) { - throw VMStateIdempotentException{"Ignoring shutdown since instance is already stopped."}; + // TODO: format state directly + throw VMStateIdempotentException{ + fmt::format("Ignoring shutdown since instance is {}.", + (state == State::unavailable) ? "unavailable" : "already stopped")}; } if (shutdown_policy == ShutdownPolicy::Poweroff) @@ -238,6 +241,33 @@ void mp::BaseVirtualMachine::check_state_for_shutdown(ShutdownPolicy shutdown_po } } +void mp::BaseVirtualMachine::set_available(bool available) +{ + // Ignore idempotent calls + if (available == (state != State::unavailable)) + return; + + if (available) + { + state = State::off; + update_state(); + if (was_running) + { + start(); + + // normally the daemon sets the state to running... + state = State::running; + update_state(); + } + return; + } + + was_running = state == State::running || state == State::starting || state == State::restarting; + shutdown(ShutdownPolicy::Poweroff); + state = State::unavailable; + update_state(); +} + std::string mp::BaseVirtualMachine::ssh_exec(const std::string& cmd, bool whisper) { std::unique_lock lock{state_mutex}; diff --git a/src/platform/backends/shared/base_virtual_machine.h b/src/platform/backends/shared/base_virtual_machine.h index bea848dd3d2..33ad0c66f34 100644 --- a/src/platform/backends/shared/base_virtual_machine.h +++ b/src/platform/backends/shared/base_virtual_machine.h @@ -54,12 +54,7 @@ class BaseVirtualMachine : public VirtualMachine virtual std::string ssh_exec(const std::string& cmd, bool whisper = false) override; - void set_available(bool available) override - { - // TODO make vm unavailable by force stopping if running or available by starting again if - // it was running - throw NotImplementedOnThisBackendException("unavailability"); - } + void set_available(bool available) override; void wait_until_ssh_up(std::chrono::milliseconds timeout) override; void wait_for_cloud_init(std::chrono::milliseconds timeout) override; @@ -181,6 +176,7 @@ class BaseVirtualMachine : public VirtualMachine std::shared_ptr head_snapshot = nullptr; int snapshot_count = 0; // tracks the number of snapshots ever taken (regardless of deletes) mutable std::recursive_mutex snapshot_mutex; + bool was_running{false}; }; } // namespace multipass diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index d2bdba6c810..a553edf9902 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -408,7 +408,7 @@ mp::VirtualMachineFactory::UPtr mp::platform::vm_backend(const mp::Path& data_di #if VIRTUALBOX_ENABLED if (driver == QStringLiteral("virtualbox")) - return std::make_unique(data_dir); + return std::make_unique(data_dir, az_manager); #endif throw std::runtime_error(fmt::format("Unsupported virtualization driver: {}", driver)); diff --git a/src/rpc/multipass.proto b/src/rpc/multipass.proto index d634d004d99..2c2aed41bcf 100644 --- a/src/rpc/multipass.proto +++ b/src/rpc/multipass.proto @@ -208,6 +208,7 @@ message InstanceStatus { DELAYED_SHUTDOWN = 6; SUSPENDING = 7; SUSPENDED = 8; + UNAVAILABLE = 9; } Status status = 1; } diff --git a/tests/test_base_virtual_machine.cpp b/tests/test_base_virtual_machine.cpp index 6d3136a68ab..ed4f4c9c575 100644 --- a/tests/test_base_virtual_machine.cpp +++ b/tests/test_base_virtual_machine.cpp @@ -82,6 +82,10 @@ struct MockBaseVirtualMachine : public mpt::MockVirtualMachineT())); + MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(*this, + set_available, + mp::BaseVirtualMachine, + (A())); } MOCK_METHOD(std::shared_ptr, @@ -1441,4 +1445,37 @@ TEST_F(BaseVM, sshExecRethrowsSSHExceptionsWhenConnected) mpt::match_what(HasSubstr("intentional"))); } +TEST_F(BaseVM, setUnavailableShutsdownRunning) +{ + ON_CALL(vm, current_state).WillByDefault(Return(St::running)); + EXPECT_CALL(vm, shutdown(mp::VirtualMachine::ShutdownPolicy::Poweroff)).Times(1); + + vm.set_available(false); +} + +TEST_F(BaseVM, setAvailableRestartsRunning) +{ + StubBaseVirtualMachine base_vm(zone, St::running); + + base_vm.set_available(false); + ASSERT_EQ(base_vm.current_state(), St::unavailable); + + base_vm.set_available(false); + ASSERT_EQ(base_vm.current_state(), St::unavailable); + + base_vm.set_available(true); + EXPECT_EQ(base_vm.current_state(), St::running); +} + +TEST_F(BaseVM, setAvailableKeepsOffOff) +{ + StubBaseVirtualMachine base_vm(zone, St::off); + + base_vm.set_available(false); + ASSERT_EQ(base_vm.current_state(), St::unavailable); + + base_vm.set_available(true); + EXPECT_EQ(base_vm.current_state(), St::off); +} + } // namespace diff --git a/tests/test_daemon.cpp b/tests/test_daemon.cpp index 133c64a2639..8b812b8f694 100644 --- a/tests/test_daemon.cpp +++ b/tests/test_daemon.cpp @@ -438,7 +438,9 @@ TEST_F(Daemon, ensureThatOnRestartFutureCompletes) // This VM was running before, but not now. auto mock_vm = std::make_unique>("yakety-yak"); - EXPECT_CALL(*mock_vm, current_state).WillOnce(Return(mp::VirtualMachine::State::stopped)); + EXPECT_CALL(*mock_vm, current_state) + .Times(1) + .WillRepeatedly(Return(mp::VirtualMachine::State::stopped)); EXPECT_CALL(*mock_vm, start).Times(1); mp::Signal sig; @@ -471,7 +473,9 @@ TEST_F(Daemon, startsPreviouslyRunningVmsBack) // This VM was running before, but not now. auto mock_vm = std::make_unique>(vm_props.name); - EXPECT_CALL(*mock_vm, current_state).WillOnce(Return(mp::VirtualMachine::State::stopped)); + EXPECT_CALL(*mock_vm, current_state) + .Times(1) + .WillRepeatedly(Return(mp::VirtualMachine::State::stopped)); EXPECT_CALL(*mock_vm, start).Times(1); EXPECT_CALL(*mock_vm, update_state).Times(1); EXPECT_CALL(*mock_vm, wait_until_ssh_up).Times(1); @@ -492,7 +496,9 @@ TEST_F(Daemon, callsOnRestartForAlreadyRunningVmsOnConstruction) // This VM was running before, but not now. auto mock_vm = std::make_unique>(vm_props.name); - EXPECT_CALL(*mock_vm, current_state).WillOnce(Return(mp::VirtualMachine::State::running)); + EXPECT_CALL(*mock_vm, current_state) + .Times(1) + .WillRepeatedly(Return(mp::VirtualMachine::State::running)); EXPECT_CALL(*mock_vm, start).Times(0); EXPECT_CALL(*mock_vm, update_state).Times(1); EXPECT_CALL(*mock_vm, wait_until_ssh_up).Times(1); @@ -513,7 +519,9 @@ TEST_F(Daemon, callsOnRestartForAlreadyStartingVmsOnConstruction) // This VM was running before, but not now. auto mock_vm = std::make_unique>(vm_props.name); - EXPECT_CALL(*mock_vm, current_state).WillOnce(Return(mp::VirtualMachine::State::starting)); + EXPECT_CALL(*mock_vm, current_state) + .Times(1) + .WillRepeatedly(Return(mp::VirtualMachine::State::starting)); EXPECT_CALL(*mock_vm, start).Times(0); EXPECT_CALL(*mock_vm, update_state).Times(1); EXPECT_CALL(*mock_vm, wait_until_ssh_up).Times(1);