From fd605e5480b220dbe0f9d4a32cb66bb0dbddf918 Mon Sep 17 00:00:00 2001 From: "bors[bot]" <26634292+bors[bot]@users.noreply.github.com> Date: Wed, 6 Jul 2022 14:38:25 +0000 Subject: [PATCH] Merge #2647 2647: [daemon] Move starting instances back to main thread r=luis4a0 a=townsend2010 Fixes #2646 Co-authored-by: Chris Townsend --- src/daemon/daemon.cpp | 63 ++++++++++++++++++++----------------- src/daemon/daemon.h | 9 +++--- tests/test_daemon_start.cpp | 3 +- 3 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/daemon/daemon.cpp b/src/daemon/daemon.cpp index 240644e62e..38dc3de9ba 100644 --- a/src/daemon/daemon.cpp +++ b/src/daemon/daemon.cpp @@ -1706,9 +1706,33 @@ try // clang-format on } } + fmt::memory_buffer start_errors; + for (const auto& name : vms) + { + auto& vm = vm_instances.find(name)->second; + auto state = vm->current_state(); + if (state == VirtualMachine::State::unknown) + { + auto error_string = fmt::format("Instance \'{}\' is already running, but in an unknown state", name); + mpl::log(mpl::Level::warning, category, error_string); + fmt::format_to(start_errors, error_string); + continue; + } + else if (state == VirtualMachine::State::suspending) + { + fmt::format_to(start_errors, "Cannot start the instance \'{}\' while suspending", name); + continue; + } + else if (state != VirtualMachine::State::running && state != VirtualMachine::State::starting && + state != VirtualMachine::State::restarting) + { + vm->start(); + } + } + auto future_watcher = create_future_watcher(); future_watcher->setFuture(QtConcurrent::run(this, &Daemon::async_wait_for_ready_all, server, vms, - timeout, status_promise, true)); + timeout, status_promise, fmt::to_string(start_errors))); } catch (const std::exception& e) { @@ -1817,7 +1841,7 @@ try // clang-format on auto future_watcher = create_future_watcher(); future_watcher->setFuture(QtConcurrent::run(this, &Daemon::async_wait_for_ready_all, server, - instances, timeout, status_promise, false)); + instances, timeout, status_promise, std::string())); } catch (const std::exception& e) { @@ -2077,7 +2101,8 @@ void mp::Daemon::on_restart(const std::string& name) { auto future_watcher = create_future_watcher(); future_watcher->setFuture(QtConcurrent::run(this, &Daemon::async_wait_for_ready_all, nullptr, - std::vector{name}, mp::default_timeout, nullptr, false)); + std::vector{name}, mp::default_timeout, nullptr, + std::string())); } void mp::Daemon::persist_state_for(const std::string& name, const VirtualMachine::State& state) @@ -2307,6 +2332,8 @@ void mp::Daemon::create_vm(const CreateRequest* request, grpc::ServerWriterInter reply.set_create_message("Starting " + name); server->Write(reply); + vm_instances[name]->start(); + auto future_watcher = create_future_watcher([this, server, name] { LaunchReply reply; reply.set_vm_instance_name(name); @@ -2315,7 +2342,7 @@ void mp::Daemon::create_vm(const CreateRequest* request, grpc::ServerWriterInter }); future_watcher->setFuture(QtConcurrent::run(this, &Daemon::async_wait_for_ready_all, server, std::vector{name}, timeout, - status_promise, true)); + status_promise, std::string())); } else { @@ -2620,9 +2647,11 @@ template mp::Daemon::AsyncOperationStatus mp::Daemon::async_wait_for_ready_all(grpc::ServerWriterInterface* server, const std::vector& vms, const std::chrono::seconds& timeout, std::promise* status_promise, - const bool start_vm) + const std::string& start_errors) { fmt::memory_buffer errors; + fmt::format_to(errors, "{}", start_errors); + QFutureSynchronizer start_synchronizer; { std::lock_guard lock{start_mutex}; @@ -2634,30 +2663,6 @@ mp::Daemon::async_wait_for_ready_all(grpc::ServerWriterInterface* server, } else { - if (start_vm) - { - auto it = vm_instances.find(name); - auto state = it->second->current_state(); - if (state == VirtualMachine::State::unknown) - { - auto error_string = - fmt::format("Instance \'{}\' is already running, but in an unknown state", name); - mpl::log(mpl::Level::warning, category, error_string); - fmt::format_to(errors, error_string); - continue; - } - else if (state == VirtualMachine::State::suspending) - { - fmt::format_to(errors, "Cannot start the instance while suspending"); - continue; - } - else if (state != VirtualMachine::State::running && state != VirtualMachine::State::starting && - state != VirtualMachine::State::restarting) - { - it->second->start(); - } - } - auto future = QtConcurrent::run(this, &Daemon::async_wait_for_ssh_and_start_mounts_for, name, timeout, server); async_running_futures[name] = future; diff --git a/src/daemon/daemon.h b/src/daemon/daemon.h index d94dadde89..b12a77f862 100644 --- a/src/daemon/daemon.h +++ b/src/daemon/daemon.h @@ -23,6 +23,7 @@ #include "vm_specs.h" #include +//#include #include #include #include @@ -147,10 +148,10 @@ public slots: std::string async_wait_for_ssh_and_start_mounts_for(const std::string& name, const std::chrono::seconds& timeout, grpc::ServerWriterInterface* server); template - AsyncOperationStatus async_wait_for_ready_all(grpc::ServerWriterInterface* server, - const std::vector& vms, - const std::chrono::seconds& timeout, - std::promise* status_promise, const bool start_vm); + AsyncOperationStatus + async_wait_for_ready_all(grpc::ServerWriterInterface* server, const std::vector& vms, + const std::chrono::seconds& timeout, std::promise* status_promise, + const std::string& errors); void finish_async_operation(QFuture async_future); QFutureWatcher* create_future_watcher(std::function const& finished_op = []() {}); diff --git a/tests/test_daemon_start.cpp b/tests/test_daemon_start.cpp index 88a92e5420..1d12da7f2e 100644 --- a/tests/test_daemon_start.cpp +++ b/tests/test_daemon_start.cpp @@ -132,5 +132,6 @@ TEST_F(TestDaemonStart, suspendingStateDoesNotStartHasError) EXPECT_FALSE(status.ok()); - EXPECT_THAT(status.error_message(), HasSubstr("Cannot start the instance while suspending")); + EXPECT_THAT(status.error_message(), + HasSubstr(fmt::format("Cannot start the instance \'{}\' while suspending", mock_instance_name))); }