diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index df956254a1..fb9bc63b70 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,12 @@ try continue; } case VirtualMachine::State::suspending: + case VirtualMachine::State::unavailable: 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 +2375,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 +2504,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 +2560,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]; @@ -3730,6 +3764,7 @@ error_string mp::Daemon::async_wait_for_ssh_and_start_mounts_for( return fmt::to_string(errors); } const auto vm = it->second; + vm->wait_until_ssh_up(timeout); if (std::is_same::value) diff --git a/src/platform/backends/hyperv/hyperv_virtual_machine.cpp b/src/platform/backends/hyperv/hyperv_virtual_machine.cpp index f0dca2c39b..a41df3718d 100644 --- a/src/platform/backends/hyperv/hyperv_virtual_machine.cpp +++ b/src/platform/backends/hyperv/hyperv_virtual_machine.cpp @@ -412,6 +412,10 @@ void mp::HyperVVirtualMachine::suspend() { mpl::info(vm_name, "Ignoring suspend issued while stopped"); } + else if (present_state == State::unavailable) + { + mpl::log(mpl::Level::info, vm_name, "Ignoring suspend since instance is unavailable"); + } monitor->on_suspend(); } diff --git a/src/platform/backends/qemu/qemu_virtual_machine.cpp b/src/platform/backends/qemu/qemu_virtual_machine.cpp index ed53cf6ab6..77d8879b3c 100644 --- a/src/platform/backends/qemu/qemu_virtual_machine.cpp +++ b/src/platform/backends/qemu/qemu_virtual_machine.cpp @@ -426,9 +426,11 @@ 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"); + mpl::log(mpl::Level::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 4c983af15c..d58c60b48a 100644 --- a/src/platform/backends/shared/base_availability_zone.cpp +++ b/src/platform/backends/shared/base_availability_zone.cpp @@ -117,8 +117,32 @@ void BaseAvailabilityZone::set_available(const bool new_available) m.available = new_available; serialize(); - 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; + serialize(); + + // 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 b45e401377..93f252c210 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -204,6 +204,10 @@ void mp::BaseVirtualMachine::check_state_for_shutdown(ShutdownPolicy shutdown_po { throw VMStateIdempotentException{"Ignoring shutdown since instance is already stopped."}; } + if (state == State::unavailable) + { + throw VMStateIdempotentException{"Ignoring shutdown since instance is unavailable."}; + } if (shutdown_policy == ShutdownPolicy::Poweroff) { @@ -238,6 +242,29 @@ void mp::BaseVirtualMachine::check_state_for_shutdown(ShutdownPolicy shutdown_po } } +void mp::BaseVirtualMachine::set_available(bool available) +{ + 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 bea848dd3d..33ad0c66f3 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 d2bdba6c81..a553edf990 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 d634d004d9..2c2aed41bc 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 6d3136a68a..12e86a636d 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,34 @@ 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(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 133c64a263..8b812b8f69 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);