Start TerminaVm with the correct number of CPUs

ARCVM already does this with SchedulerConfigurationManager's
observer, and this is Crostini port of the feature.

BUG=1011559
TEST=try, lscpu in guest returns 2 on eve

Change-Id: Ie94497ed803c2e10f17def26855107e0f1c336e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1972883Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726226}

chrome/browser/chromeos/crostini/crostini_installer_unittest.cc

@@ -15,6 +15,8 @@
 #include "chrome/browser/chromeos/crostini/crostini_installer_types.mojom.h"
 #include "chrome/browser/chromeos/crostini/crostini_installer_ui_delegate.h"
 #include "chrome/browser/chromeos/crostini/crostini_test_helper.h"
+#include "chrome/test/base/scoped_testing_local_state.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/concierge/concierge_service.pb.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -109,7 +111,9 @@ class CrostiniInstallerTest : public testing::Test {
     base::OnceClosure quit_closure_;
   };
 
-  CrostiniInstallerTest() = default;
+  CrostiniInstallerTest()
+      : local_state_(std::make_unique<ScopedTestingLocalState>(
+            TestingBrowserProcess::GetGlobal())) {}
 
   void SetUp() override {
     waiting_fake_concierge_client_ = new WaitingFakeConciergeClient;
@@ -128,9 +132,13 @@ class CrostiniInstallerTest : public testing::Test {
 
     crostini_installer_ = std::make_unique<CrostiniInstaller>(profile_.get());
     crostini_installer_->set_skip_launching_terminal_for_testing();
+
+    g_browser_process->platform_part()
+        ->InitializeSchedulerConfigurationManager();
   }
 
   void TearDown() override {
+    g_browser_process->platform_part()->ShutdownSchedulerConfigurationManager();
     crostini_installer_->Shutdown();
     crostini_installer_.reset();
     crostini_test_helper_.reset();
@@ -172,6 +180,8 @@ class CrostiniInstallerTest : public testing::Test {
   std::unique_ptr<CrostiniInstaller> crostini_installer_;
 
  private:
+  std::unique_ptr<ScopedTestingLocalState> local_state_;
+
   DISALLOW_COPY_AND_ASSIGN(CrostiniInstallerTest);
 };
 

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -17,6 +17,7 @@
 #include "base/no_destructor.h"
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
+#include "base/system/sys_info.h"
 #include "base/task/post_task.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
@@ -33,6 +34,7 @@
 #include "chrome/browser/chromeos/file_manager/volume_manager.h"
 #include "chrome/browser/chromeos/guest_os/guest_os_share_path.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
+#include "chrome/browser/chromeos/scheduler_configuration_manager.h"
 #include "chrome/browser/chromeos/usb/cros_usb_detector.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -123,7 +125,8 @@ CrostiniManager::RestartOptions& CrostiniManager::RestartOptions::operator=(
 class CrostiniManager::CrostiniRestarter
     : public base::RefCountedThreadSafe<CrostiniRestarter>,
       public crostini::VmShutdownObserver,
-      public chromeos::disks::DiskMountManager::Observer {
+      public chromeos::disks::DiskMountManager::Observer,
+      public chromeos::SchedulerConfigurationManagerBase::Observer {
  public:
   CrostiniRestarter(Profile* profile,
                     CrostiniManager* crostini_manager,
@@ -313,9 +316,34 @@ class CrostiniManager::CrostiniRestarter
       FinishRestart(CrostiniResult::CREATE_DISK_IMAGE_FAILED);
       return;
     }
+    disk_path_ = result_path;
+
+    auto* scheduler_configuration_manager =
+        g_browser_process->platform_part()->scheduler_configuration_manager();
+    base::Optional<std::pair<bool, size_t>> scheduler_configuration =
+        scheduler_configuration_manager->GetLastReply();
+    if (!scheduler_configuration) {
+      // Wait for the configuration to become available.
+      LOG(WARNING) << "Scheduler configuration is not yet ready";
+      scheduler_configuration_manager->AddObserver(this);
+      return;
+    }
+    OnConfigurationSet(scheduler_configuration->first,
+                       scheduler_configuration->second);
+  }
+
+  // chromeos::SchedulerConfigurationManagerBase::Observer:
+  void OnConfigurationSet(bool success, size_t num_cores_disabled) override {
+    // Note: On non-x86_64 devices, the configuration request to debugd always
+    // fails. It is WAI, and to support that case, don't log anything even when
+    // |success| is false. |num_cores_disabled| is always set regardless of
+    // whether the call is successful.
+    g_browser_process->platform_part()
+        ->scheduler_configuration_manager()
+        ->RemoveObserver(this);
     StartStage(mojom::InstallerState::kStartTerminaVm);
     crostini_manager_->StartTerminaVm(
-        vm_name_, result_path,
+        vm_name_, disk_path_, num_cores_disabled,
         base::BindOnce(&CrostiniRestarter::StartTerminaVmFinished, this));
   }
 
@@ -536,6 +564,7 @@ class CrostiniManager::CrostiniRestarter
   CrostiniManager* crostini_manager_;
 
   std::string vm_name_;
+  base::FilePath disk_path_;
   std::string container_name_;
   RestartOptions options_;
   std::string source_path_;
@@ -1002,6 +1031,7 @@ void CrostiniManager::ListVmDisks(ListVmDisksCallback callback) {
 
 void CrostiniManager::StartTerminaVm(std::string name,
                                      const base::FilePath& disk_path,
+                                     size_t num_cores_disabled,
                                      BoolCallback callback) {
   if (name.empty()) {
     LOG(ERROR) << "name is required";
@@ -1032,6 +1062,9 @@ void CrostiniManager::StartTerminaVm(std::string name,
   request.set_owner_id(owner_id_);
   if (base::FeatureList::IsEnabled(chromeos::features::kCrostiniGpuSupport))
     request.set_enable_gpu(true);
+  const int32_t cpus = base::SysInfo::NumberOfProcessors() - num_cores_disabled;
+  DCHECK_LT(0, cpus);
+  request.set_cpus(cpus);
 
   vm_tools::concierge::DiskImage* disk_image = request.add_disks();
   disk_image->set_path(std::move(disk_path_string));

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -237,6 +237,8 @@ class CrostiniManager : public KeyedService,
       std::string name,
       // Path to the disk image on the host.
       const base::FilePath& disk_path,
+      // The number of logical CPU cores that are currently disabled.
+      size_t num_cores_disabled,
       BoolCallback callback);
 
   // Checks the arguments for stopping a Termina VM. Stops the Termina VM via

chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

@@ -20,6 +20,8 @@
 #include "chrome/browser/chromeos/crostini/fake_crostini_features.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/test/base/scoped_testing_local_state.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/dbus/cicerone/cicerone_service.pb.h"
 #include "chromeos/dbus/concierge/concierge_service.pb.h"
@@ -139,7 +141,9 @@ class CrostiniManagerTest : public testing::Test {
   }
 
   CrostiniManagerTest()
-      : task_environment_(content::BrowserTaskEnvironment::REAL_IO_THREAD) {
+      : task_environment_(content::BrowserTaskEnvironment::REAL_IO_THREAD),
+        local_state_(std::make_unique<ScopedTestingLocalState>(
+            TestingBrowserProcess::GetGlobal())) {
     chromeos::DBusThreadManager::Initialize();
     fake_cicerone_client_ = static_cast<chromeos::FakeCiceroneClient*>(
         chromeos::DBusThreadManager::Get()->GetCiceroneClient());
@@ -170,9 +174,13 @@ class CrostiniManagerTest : public testing::Test {
     mojo::Remote<device::mojom::UsbDeviceManager> fake_usb_manager;
     fake_usb_manager_.AddReceiver(
         fake_usb_manager.BindNewPipeAndPassReceiver());
+
+    g_browser_process->platform_part()
+        ->InitializeSchedulerConfigurationManager();
   }
 
   void TearDown() override {
+    g_browser_process->platform_part()->ShutdownSchedulerConfigurationManager();
     scoped_user_manager_.reset();
     crostini_manager_->Shutdown();
     profile_.reset();
@@ -199,6 +207,8 @@ class CrostiniManagerTest : public testing::Test {
  private:
   content::BrowserTaskEnvironment task_environment_;
   std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
+  std::unique_ptr<ScopedTestingLocalState> local_state_;
+
   DISALLOW_COPY_AND_ASSIGN(CrostiniManagerTest);
 };
 
@@ -263,7 +273,8 @@ TEST_F(CrostiniManagerTest, StartTerminaVmNameError) {
   const base::FilePath& disk_path = base::FilePath(kVmName);
 
   crostini_manager()->StartTerminaVm(
-      "", disk_path, base::BindOnce(&ExpectFailure, run_loop()->QuitClosure()));
+      "", disk_path, 0,
+      base::BindOnce(&ExpectFailure, run_loop()->QuitClosure()));
   run_loop()->Run();
   EXPECT_FALSE(fake_concierge_client_->start_termina_vm_called());
 }
@@ -275,7 +286,7 @@ TEST_F(CrostiniManagerTest, StartTerminaVmAnomalyDetectorNotConnectedError) {
       false);
 
   crostini_manager()->StartTerminaVm(
-      kVmName, disk_path,
+      kVmName, disk_path, 0,
       base::BindOnce(&ExpectFailure, run_loop()->QuitClosure()));
   run_loop()->Run();
   EXPECT_FALSE(fake_concierge_client_->start_termina_vm_called());
@@ -285,7 +296,7 @@ TEST_F(CrostiniManagerTest, StartTerminaVmDiskPathError) {
   const base::FilePath& disk_path = base::FilePath();
 
   crostini_manager()->StartTerminaVm(
-      kVmName, disk_path,
+      kVmName, disk_path, 0,
       base::BindOnce(&ExpectFailure, run_loop()->QuitClosure()));
   run_loop()->Run();
   EXPECT_FALSE(fake_concierge_client_->start_termina_vm_called());
@@ -301,7 +312,7 @@ TEST_F(CrostiniManagerTest, StartTerminaVmMountError) {
   fake_concierge_client_->set_start_vm_response(response);
 
   crostini_manager()->StartTerminaVm(
-      kVmName, disk_path,
+      kVmName, disk_path, 0,
       base::BindOnce(&ExpectFailure, run_loop()->QuitClosure()));
   run_loop()->Run();
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
@@ -320,7 +331,7 @@ TEST_F(CrostiniManagerTest, StartTerminaVmMountErrorThenSuccess) {
   fake_concierge_client_->set_start_vm_response(response);
 
   crostini_manager()->StartTerminaVm(
-      kVmName, disk_path,
+      kVmName, disk_path, 0,
       base::BindOnce(&ExpectSuccess, run_loop()->QuitClosure()));
   run_loop()->Run();
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
@@ -333,7 +344,7 @@ TEST_F(CrostiniManagerTest, StartTerminaVmSuccess) {
   const base::FilePath& disk_path = base::FilePath(kVmName);
 
   crostini_manager()->StartTerminaVm(
-      kVmName, disk_path,
+      kVmName, disk_path, 0,
       base::BindOnce(&ExpectSuccess, run_loop()->QuitClosure()));
   run_loop()->Run();
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
@@ -346,7 +357,7 @@ TEST_F(CrostiniManagerTest, OnStartTremplinRecordsRunningVm) {
 
   // Start the Vm.
   crostini_manager()->StartTerminaVm(
-      kVmName, disk_path,
+      kVmName, disk_path, 0,
       base::BindOnce(&ExpectSuccess, run_loop()->QuitClosure()));
 
   // Check that the Vm start is not recorded until tremplin starts.
@@ -1073,7 +1084,7 @@ TEST_F(CrostiniManagerRestartTest, IsContainerRunningFalseIfVmNotStarted) {
 
   base::RunLoop run_loop2;
   crostini_manager()->StartTerminaVm(
-      kVmName, disk_path,
+      kVmName, disk_path, 0,
       base::BindOnce(&ExpectSuccess, run_loop2.QuitClosure()));
   run_loop2.Run();
   EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());

chrome/browser/chromeos/guest_os/guest_os_share_path_unittest.cc

@@ -20,6 +20,8 @@
 #include "chrome/browser/chromeos/guest_os/guest_os_pref_names.h"
 #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
 #include "chrome/common/chrome_features.h"
+#include "chrome/test/base/scoped_testing_local_state.h"
+#include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -200,7 +202,9 @@ class GuestOsSharePathTest : public testing::Test {
                         expected_failure_reason, success, failure_reason);
   }
 
-  GuestOsSharePathTest() {
+  GuestOsSharePathTest()
+      : local_state_(std::make_unique<ScopedTestingLocalState>(
+            TestingBrowserProcess::GetGlobal())) {
     chromeos::DBusThreadManager::Initialize();
     fake_concierge_client_ = static_cast<chromeos::FakeConciergeClient*>(
         chromeos::DBusThreadManager::Get()->GetConciergeClient());
@@ -254,9 +258,13 @@ class GuestOsSharePathTest : public testing::Test {
     // Create 'vm-running' VM instance which is running.
     crostini::CrostiniManager::GetForProfile(profile())->AddRunningVmForTesting(
         "vm-running");
+
+    g_browser_process->platform_part()
+        ->InitializeSchedulerConfigurationManager();
   }
 
   void TearDown() override {
+    g_browser_process->platform_part()->ShutdownSchedulerConfigurationManager();
     // Shutdown GuestOsSharePath to schedule FilePathWatchers to be destroyed,
     // then run thread bundle to ensure they are.
     guest_os_share_path_->Shutdown();
@@ -293,6 +301,8 @@ class GuestOsSharePathTest : public testing::Test {
   AccountId account_id_;
 
  private:
+  std::unique_ptr<ScopedTestingLocalState> local_state_;
+
   DISALLOW_COPY_AND_ASSIGN(GuestOsSharePathTest);
 };