Commit a0431e06 authored by Nicholas Hollingum's avatar Nicholas Hollingum Committed by Commit Bot

Borealis: Support invalidating the context

When shutting down the VM we must invalidate the current context. We
support that in this CL by unconditionally requesting concierge to shut
the vm down (which is fine to do even if the vm is not running). Any
queued callbacks are invoked with the new kCancelled status.

Bug: b/172006764
Change-Id: I8e9d5817728ede618967392959ec78b6f7d595f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513807
Commit-Queue: Nic Hollingum <hollingum@google.com>
Reviewed-by: default avatarDaniel Ng <danielng@google.com>
Cr-Commit-Position: refs/heads/master@{#823897}
parent 68ad76f4
...@@ -19,6 +19,7 @@ class BorealisContextManager : public KeyedService { ...@@ -19,6 +19,7 @@ class BorealisContextManager : public KeyedService {
// A list of possible outcomes for an attempt to startup borealis. // A list of possible outcomes for an attempt to startup borealis.
enum class Status { enum class Status {
kSuccess, kSuccess,
kCancelled,
kMountFailed, kMountFailed,
kDiskImageFailed, kDiskImageFailed,
kStartVmFailed, kStartVmFailed,
...@@ -67,6 +68,11 @@ class BorealisContextManager : public KeyedService { ...@@ -67,6 +68,11 @@ class BorealisContextManager : public KeyedService {
// Starts the Borealis VM and/or runs the callback when it is running. // Starts the Borealis VM and/or runs the callback when it is running.
virtual void StartBorealis(ResultCallback callback) = 0; virtual void StartBorealis(ResultCallback callback) = 0;
// Stop the current running state, re-initializing the context manager
// to the state it was in prior to being started. All pending callbacks are
// invoked with kCancelled status.
virtual void ShutDownBorealis() = 0;
}; };
} // namespace borealis } // namespace borealis
......
...@@ -11,6 +11,8 @@ ...@@ -11,6 +11,8 @@
#include "chrome/browser/chromeos/borealis/borealis_context.h" #include "chrome/browser/chromeos/borealis/borealis_context.h"
#include "chrome/browser/chromeos/borealis/borealis_context_manager.h" #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
#include "chrome/browser/chromeos/borealis/borealis_task.h" #include "chrome/browser/chromeos/borealis/borealis_task.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chromeos/dbus/dbus_thread_manager.h"
namespace { namespace {
...@@ -23,6 +25,8 @@ std::ostream& operator<<(std::ostream& stream, ...@@ -23,6 +25,8 @@ std::ostream& operator<<(std::ostream& stream,
switch (status) { switch (status) {
case borealis::BorealisContextManager::Status::kSuccess: case borealis::BorealisContextManager::Status::kSuccess:
return stream << "Success"; return stream << "Success";
case borealis::BorealisContextManager::Status::kCancelled:
return stream << "Cancelled";
case borealis::BorealisContextManager::Status::kMountFailed: case borealis::BorealisContextManager::Status::kMountFailed:
return stream << "Mount Failed"; return stream << "Mount Failed";
case borealis::BorealisContextManager::Status::kDiskImageFailed: case borealis::BorealisContextManager::Status::kDiskImageFailed:
...@@ -57,6 +61,30 @@ void BorealisContextManagerImpl::StartBorealis(ResultCallback callback) { ...@@ -57,6 +61,30 @@ void BorealisContextManagerImpl::StartBorealis(ResultCallback callback) {
} }
} }
void BorealisContextManagerImpl::ShutDownBorealis() {
// TODO(b/172178036): This could have been a task-sequence but that
// abstraction is proving insufficient.
vm_tools::concierge::StopVmRequest request;
request.set_owner_id(
chromeos::ProfileHelper::GetUserIdHashFromProfile(profile_));
request.set_name(context_->vm_name());
chromeos::DBusThreadManager::Get()->GetConciergeClient()->StopVm(
std::move(request),
base::BindOnce(
[](base::Optional<vm_tools::concierge::StopVmResponse> response) {
// We don't have a good way to deal with a vm failing to stop (and
// this would be a very rare occurrence anyway). We log an error if
// it actually wasn't successful.
if (!response.has_value()) {
LOG(ERROR) << "Failed to stop Borealis VM: No response";
} else if (!response.value().success()) {
LOG(ERROR) << "Failed to stop Borealis VM: "
<< response.value().failure_reason();
}
}));
Complete(BorealisContextManager::Status::kCancelled, "shut down");
}
base::queue<std::unique_ptr<BorealisTask>> base::queue<std::unique_ptr<BorealisTask>>
BorealisContextManagerImpl::GetTasks() { BorealisContextManagerImpl::GetTasks() {
base::queue<std::unique_ptr<BorealisTask>> task_queue; base::queue<std::unique_ptr<BorealisTask>> task_queue;
...@@ -74,8 +102,7 @@ void BorealisContextManagerImpl::AddCallback(ResultCallback callback) { ...@@ -74,8 +102,7 @@ void BorealisContextManagerImpl::AddCallback(ResultCallback callback) {
void BorealisContextManagerImpl::NextTask() { void BorealisContextManagerImpl::NextTask() {
if (task_queue_.empty()) { if (task_queue_.empty()) {
startup_status_ = Status::kSuccess; Complete(Status::kSuccess, "");
OnQueueComplete();
return; return;
} }
task_queue_.front()->Run( task_queue_.front()->Run(
...@@ -86,23 +113,31 @@ void BorealisContextManagerImpl::NextTask() { ...@@ -86,23 +113,31 @@ void BorealisContextManagerImpl::NextTask() {
void BorealisContextManagerImpl::TaskCallback(Status status, void BorealisContextManagerImpl::TaskCallback(Status status,
std::string error) { std::string error) {
task_queue_.pop(); task_queue_.pop();
startup_status_ = status; if (status == Status::kSuccess) {
if (startup_status_ == Status::kSuccess) {
NextTask(); NextTask();
return; return;
} }
startup_error_ = error; LOG(ERROR) << "Startup failed: failure=" << status << " message=" << error;
LOG(ERROR) << "Startup failed: failure=" << startup_status_ Complete(status, std::move(error));
<< " message=" << startup_error_;
OnQueueComplete();
} }
void BorealisContextManagerImpl::OnQueueComplete() { void BorealisContextManagerImpl::Complete(BorealisContextManager::Status status,
std::string error_or_empty) {
startup_status_ = status;
startup_error_ = error_or_empty;
while (!callback_queue_.empty()) { while (!callback_queue_.empty()) {
ResultCallback callback = std::move(callback_queue_.front()); ResultCallback callback = std::move(callback_queue_.front());
callback_queue_.pop(); callback_queue_.pop();
std::move(callback).Run(GetResult()); std::move(callback).Run(GetResult());
} }
if (startup_status_ == BorealisContextManager::Status::kSuccess)
return;
task_queue_ = {};
// TODO(b/172178467): handle races better when doing this.
context_.reset();
} }
BorealisContextManager::Result BorealisContextManagerImpl::GetResult() { BorealisContextManager::Result BorealisContextManagerImpl::GetResult() {
......
...@@ -29,6 +29,7 @@ class BorealisContextManagerImpl : public BorealisContextManager { ...@@ -29,6 +29,7 @@ class BorealisContextManagerImpl : public BorealisContextManager {
// BorealisContextManager: // BorealisContextManager:
void StartBorealis(ResultCallback callback) override; void StartBorealis(ResultCallback callback) override;
void ShutDownBorealis() override;
// Public due to testing. // Public due to testing.
virtual base::queue<std::unique_ptr<BorealisTask>> GetTasks(); virtual base::queue<std::unique_ptr<BorealisTask>> GetTasks();
...@@ -37,7 +38,12 @@ class BorealisContextManagerImpl : public BorealisContextManager { ...@@ -37,7 +38,12 @@ class BorealisContextManagerImpl : public BorealisContextManager {
void AddCallback(ResultCallback callback); void AddCallback(ResultCallback callback);
void NextTask(); void NextTask();
void TaskCallback(Status status, std::string error); void TaskCallback(Status status, std::string error);
void OnQueueComplete();
// Completes the startup with the given |status| and error messgae, invoking
// all callbacks with the result. For any status except kSuccess the state of
// the system will be as though StartBorealis() had not been called.
void Complete(BorealisContextManager::Status status,
std::string error_or_empty);
// Returns the result of the startup (i.e. the context if it succeeds, or an // Returns the result of the startup (i.e. the context if it succeeds, or an
// error if it doesn't). // error if it doesn't).
......
...@@ -6,13 +6,16 @@ ...@@ -6,13 +6,16 @@
#include <memory> #include <memory>
#include "base/bind_helpers.h"
#include "base/containers/queue.h" #include "base/containers/queue.h"
#include "base/test/bind_test_util.h"
#include "chrome/browser/chromeos/borealis/borealis_context_manager.h" #include "chrome/browser/chromeos/borealis/borealis_context_manager.h"
#include "chrome/browser/chromeos/borealis/borealis_context_manager_factory.h" #include "chrome/browser/chromeos/borealis/borealis_context_manager_factory.h"
#include "chrome/browser/chromeos/borealis/borealis_task.h" #include "chrome/browser/chromeos/borealis/borealis_task.h"
#include "chrome/browser/chromeos/login/users/mock_user_manager.h" #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_concierge_client.h"
#include "components/user_manager/scoped_user_manager.h" #include "components/user_manager/scoped_user_manager.h"
#include "content/public/test/browser_task_environment.h" #include "content/public/test/browser_task_environment.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
...@@ -200,6 +203,44 @@ TEST_F(BorealisContextManagerTest, ...@@ -200,6 +203,44 @@ TEST_F(BorealisContextManagerTest,
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
} }
class NeverCompletingContextManager : public BorealisContextManagerImpl {
public:
explicit NeverCompletingContextManager(Profile* profile)
: BorealisContextManagerImpl(profile) {}
private:
class NeverCompletingTask : public BorealisTask {
public:
void Run(BorealisContext* context,
CompletionStatusCallback callback) override {}
};
base::queue<std::unique_ptr<BorealisTask>> GetTasks() override {
base::queue<std::unique_ptr<BorealisTask>> queue;
queue.push(std::make_unique<NeverCompletingTask>());
return queue;
}
};
TEST_F(BorealisContextManagerTest, ShutDownCancelsRequestsAndTerminatesVm) {
testing::StrictMock<ResultCallbackHandler> callback_expectation;
EXPECT_CALL(callback_expectation, Callback(testing::_))
.WillOnce(testing::Invoke([](BorealisContextManager::Result result) {
EXPECT_FALSE(result.Ok());
EXPECT_EQ(result.Failure(), BorealisContextManager::Status::kCancelled);
}));
NeverCompletingContextManager context_manager(profile_.get());
context_manager.StartBorealis(callback_expectation.GetCallback());
context_manager.ShutDownBorealis();
task_environment_.RunUntilIdle();
chromeos::FakeConciergeClient* fake_concierge_client =
static_cast<chromeos::FakeConciergeClient*>(
chromeos::DBusThreadManager::Get()->GetConciergeClient());
EXPECT_TRUE(fake_concierge_client->stop_vm_called());
}
class BorealisContextManagerFactoryTest : public testing::Test { class BorealisContextManagerFactoryTest : public testing::Test {
public: public:
BorealisContextManagerFactoryTest() = default; BorealisContextManagerFactoryTest() = default;
......
...@@ -52,7 +52,11 @@ class BorealisContextManagerMock : public borealis::BorealisContextManager { ...@@ -52,7 +52,11 @@ class BorealisContextManagerMock : public borealis::BorealisContextManager {
BorealisContextManagerMock& operator=(const BorealisContextManagerMock&) = BorealisContextManagerMock& operator=(const BorealisContextManagerMock&) =
delete; delete;
MOCK_METHOD1(StartBorealis, void(BorealisContextManager::ResultCallback)); MOCK_METHOD(void,
StartBorealis,
(BorealisContextManager::ResultCallback),
());
MOCK_METHOD(void, ShutDownBorealis, (), ());
}; };
class BorealisInstallerViewBrowserTest : public DialogBrowserTest { class BorealisInstallerViewBrowserTest : public DialogBrowserTest {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment