crostini: Add ResizeCrostiniDisk to crostini_disk.

Bug: chromium:858815
Test: Unit tests, manually call and check disk size changes
Change-Id: I68bacf5d6865e35f6280ee3e8e8fabeb27a52de9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2081722
Auto-Submit: David Munro <davidmunro@google.com>
Commit-Queue: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747593}

chrome/browser/chromeos/crostini/crostini_disk.cc

@@ -38,7 +38,6 @@ CrostiniDiskInfo& CrostiniDiskInfo::operator=(CrostiniDiskInfo&&) = default;
 CrostiniDiskInfo::~CrostiniDiskInfo() = default;
 
 namespace disk {
-
 void GetDiskInfo(OnceDiskInfoCallback callback,
                  Profile* profile,
                  std::string vm_name) {
@@ -78,6 +77,7 @@ void OnVMRunning(OnceDiskInfoCallback callback,
     vm_tools::concierge::ListVmDisksRequest request;
     request.set_cryptohome_id(CryptohomeIdForProfile(profile));
     request.set_storage_location(vm_tools::concierge::STORAGE_CRYPTOHOME_ROOT);
+    request.set_vm_name(vm_name);
     GetConciergeClient()->ListVmDisks(
         std::move(request), base::BindOnce(&OnListVmDisks, std::move(callback),
                                            std::move(vm_name), free_space));
@@ -118,6 +118,11 @@ void OnListVmDisks(
     std::move(callback).Run(std::move(disk_info));
     return;
   }
+  if (image->min_size() == 0) {
+    LOG(ERROR) << "Unable to get minimum disk size";
+    std::move(callback).Run(nullptr);
+    return;
+  }
   disk_info->is_user_chosen_size = image->user_chosen_size();
   disk_info->can_resize =
       image->image_type() == vm_tools::concierge::DiskImageType::DISK_IMAGE_RAW;
@@ -143,7 +148,15 @@ std::vector<crostini::mojom::DiskSliderTickPtr> GetTicks(
     int64_t current,
     int64_t max,
     int* out_default_index) {
-  if (min > current || current > max) {
+  if (current < min) {
+    // btrfs is conservative, sometimes it won't let us resize to what the user
+    // currently has. In those cases act like the current size is the same as
+    // the minimum.
+    VLOG(1) << "Minimum size is larger than the current, setting current = min";
+    current = min;
+  }
+  if (current > max) {
+    LOG(ERROR) << "current > max";
     return {};
   }
   std::vector<int64_t> values = GetTicksForDiskSize(min, max);
@@ -167,5 +180,67 @@ std::vector<crostini::mojom::DiskSliderTickPtr> GetTicks(
   }
   return ticks;
 }
+
+class Observer : public chromeos::ConciergeClient::DiskImageObserver {
+ public:
+  Observer(std::string uuid, base::OnceCallback<void(bool)> callback)
+      : uuid_(std::move(uuid)), callback_(std::move(callback)) {}
+  ~Observer() override { GetConciergeClient()->RemoveDiskImageObserver(this); }
+  void OnDiskImageProgress(
+      const vm_tools::concierge::DiskImageStatusResponse& signal) override {
+    if (signal.command_uuid() != uuid_) {
+      return;
+    }
+    switch (signal.status()) {
+      case vm_tools::concierge::DiskImageStatus::DISK_STATUS_IN_PROGRESS:
+        break;
+      case vm_tools::concierge::DiskImageStatus::DISK_STATUS_RESIZED:
+        std::move(callback_).Run(true);
+        break;
+      default:
+        LOG(ERROR) << "Failed or unrecognised status when resizing: "
+                   << signal.status() << " " << signal.failure_reason();
+        std::move(callback_).Run(false);
+        delete this;
+    }
+  }
+
+ private:
+  std::string uuid_;
+  base::OnceCallback<void(bool)> callback_;
+};
+
+void ResizeCrostiniDisk(Profile* profile,
+                        std::string vm_name,
+                        uint64_t size_bytes,
+                        base::OnceCallback<void(bool)> callback) {
+  vm_tools::concierge::ResizeDiskImageRequest request;
+  request.set_cryptohome_id(CryptohomeIdForProfile(profile));
+  request.set_vm_name(vm_name);
+  request.set_disk_size(size_bytes);
+
+  GetConciergeClient()->ResizeDiskImage(
+      request, base::BindOnce(&OnResize, std::move(callback)));
+}
+
+void OnResize(
+    base::OnceCallback<void(bool)> callback,
+    base::Optional<vm_tools::concierge::ResizeDiskImageResponse> response) {
+  if (!response) {
+    LOG(ERROR) << "Got null response from concierge";
+    std::move(callback).Run(false);
+  } else if (response->status() ==
+             vm_tools::concierge::DiskImageStatus::DISK_STATUS_RESIZED) {
+    std::move(callback).Run(true);
+  } else if (response->status() ==
+             vm_tools::concierge::DiskImageStatus::DISK_STATUS_IN_PROGRESS) {
+    GetConciergeClient()->AddDiskImageObserver(
+        new Observer(response->command_uuid(), std::move(callback)));
+  } else {
+    LOG(ERROR) << "Got unexpected or error status from concierge: "
+               << response->status();
+    std::move(callback).Run(false);
+  }
+}
 }  // namespace disk
 }  // namespace crostini

chrome/browser/chromeos/crostini/crostini_disk.h

@@ -75,6 +75,14 @@ std::vector<crostini::mojom::DiskSliderTickPtr> GetTicks(
     int64_t max,
     int* out_default_index);
 
+void ResizeCrostiniDisk(Profile* profile,
+                        std::string vm_name,
+                        uint64_t size_bytes,
+                        base::OnceCallback<void(bool)> callback);
+
+void OnResize(
+    base::OnceCallback<void(bool)> callback,
+    base::Optional<vm_tools::concierge::ResizeDiskImageResponse> response);
 }  // namespace disk
 }  // namespace crostini
 #endif  // CHROME_BROWSER_CHROMEOS_CROSTINI_CROSTINI_DISK_H_

chrome/browser/chromeos/crostini/crostini_disk_unittest.cc

@@ -8,8 +8,11 @@
 #include <utility>
 
 #include "base/test/bind_test_util.h"
+#include "base/test/task_environment.h"
 #include "chrome/browser/chromeos/crostini/crostini_types.mojom.h"
 #include "chromeos/dbus/concierge/concierge_service.pb.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/fake_concierge_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace crostini {
@@ -28,8 +31,8 @@ class CrostiniDiskTest : public testing::Test {
       base::Optional<vm_tools::concierge::ListVmDisksResponse>
           list_disks_response) {
     std::unique_ptr<CrostiniDiskInfo> result;
-    auto store =
-        base::BindLambdaForTesting([&](std::unique_ptr<CrostiniDiskInfo> info) {
+    auto store = base::BindLambdaForTesting(
+        [&result](std::unique_ptr<CrostiniDiskInfo> info) {
           result = std::move(info);
         });
 
@@ -40,6 +43,39 @@ class CrostiniDiskTest : public testing::Test {
   }
 };
 
+class CrostiniDiskTestDbus : public CrostiniDiskTest {
+ public:
+  CrostiniDiskTestDbus() {
+    chromeos::DBusThreadManager::Initialize();
+    fake_concierge_client_ = static_cast<chromeos::FakeConciergeClient*>(
+        chromeos::DBusThreadManager::Get()->GetConciergeClient());
+  }
+  ~CrostiniDiskTestDbus() override { chromeos::DBusThreadManager::Shutdown(); }
+
+ protected:
+  // A wrapper for ResizeCrostiniDisk which returns the result.
+  bool OnResizeWithResult(Profile* profile,
+                          const char* vm_name,
+                          int64_t size_bytes) {
+    bool result;
+    auto store =
+        base::BindLambdaForTesting([&result, &run_loop = run_loop_](bool info) {
+          result = std::move(info);
+          run_loop.QuitClosure().Run();
+        });
+
+    ResizeCrostiniDisk(profile, vm_name, size_bytes, std::move(store));
+    run_loop_.Run();
+    return result;
+  }
+
+  base::test::SingleThreadTaskEnvironment task_environment;
+
+  base::RunLoop run_loop_;
+  // Owned by chromeos::DBusThreadManager
+  chromeos::FakeConciergeClient* fake_concierge_client_;
+};
+
 TEST_F(CrostiniDiskTest, NonResizeableDiskReturnsEarly) {
   vm_tools::concierge::ListVmDisksResponse response;
   response.set_success(true);
@@ -80,8 +116,9 @@ TEST_F(CrostiniDiskTest, IsUserChosenSizeIsReportedCorrectly) {
   image->set_name("vm_name");
   image->set_image_type(vm_tools::concierge::DiskImageType::DISK_IMAGE_RAW);
   image->set_user_chosen_size(true);
+  image->set_min_size(1);
 
-  auto disk_info_user_size = OnListVmDisksWithResult("vm_name", 0, response);
+  auto disk_info_user_size = OnListVmDisksWithResult("vm_name", 1, response);
 
   ASSERT_TRUE(disk_info_user_size);
   EXPECT_TRUE(disk_info_user_size->can_resize);
@@ -90,7 +127,7 @@ TEST_F(CrostiniDiskTest, IsUserChosenSizeIsReportedCorrectly) {
   image->set_user_chosen_size(false);
 
   auto disk_info_not_user_size =
-      OnListVmDisksWithResult("vm_name", 0, response);
+      OnListVmDisksWithResult("vm_name", 1, response);
 
   ASSERT_TRUE(disk_info_not_user_size);
   EXPECT_TRUE(disk_info_not_user_size->can_resize);
@@ -156,5 +193,54 @@ TEST_F(CrostiniDiskTest, VMRunningFailureIsHandled) {
               CrostiniResult::VM_START_FAILED);
   EXPECT_FALSE(disk_info);
 }
+
+TEST_F(CrostiniDiskTestDbus, DiskResizeImmediateFailureReportsFailure) {
+  vm_tools::concierge::ResizeDiskImageResponse response;
+  response.set_status(vm_tools::concierge::DiskImageStatus::DISK_STATUS_FAILED);
+  fake_concierge_client_->set_resize_disk_image_response(response);
+
+  auto result = OnResizeWithResult(nullptr, "vm_name", 12345);
+
+  EXPECT_EQ(result, false);
+}
+
+TEST_F(CrostiniDiskTestDbus, DiskResizeEventualFailureReportsFailure) {
+  vm_tools::concierge::ResizeDiskImageResponse response;
+  vm_tools::concierge::DiskImageStatusResponse in_progress;
+  vm_tools::concierge::DiskImageStatusResponse failed;
+  response.set_status(
+      vm_tools::concierge::DiskImageStatus::DISK_STATUS_IN_PROGRESS);
+  in_progress.set_status(
+      vm_tools::concierge::DiskImageStatus::DISK_STATUS_IN_PROGRESS);
+  failed.set_status(vm_tools::concierge::DiskImageStatus::DISK_STATUS_FAILED);
+  fake_concierge_client_->set_resize_disk_image_response(response);
+  std::vector<vm_tools::concierge::DiskImageStatusResponse> signals{in_progress,
+                                                                    failed};
+  fake_concierge_client_->set_disk_image_status_signals(signals);
+
+  auto result = OnResizeWithResult(nullptr, "vm_name", 12345);
+
+  EXPECT_EQ(result, false);
+}
+
+TEST_F(CrostiniDiskTestDbus, DiskResizeEventualSuccessReportsSuccess) {
+  vm_tools::concierge::ResizeDiskImageResponse response;
+  vm_tools::concierge::DiskImageStatusResponse in_progress;
+  vm_tools::concierge::DiskImageStatusResponse resized;
+  response.set_status(
+      vm_tools::concierge::DiskImageStatus::DISK_STATUS_IN_PROGRESS);
+  in_progress.set_status(
+      vm_tools::concierge::DiskImageStatus::DISK_STATUS_IN_PROGRESS);
+  resized.set_status(vm_tools::concierge::DiskImageStatus::DISK_STATUS_RESIZED);
+  fake_concierge_client_->set_resize_disk_image_response(response);
+  std::vector<vm_tools::concierge::DiskImageStatusResponse> signals{in_progress,
+                                                                    resized};
+  fake_concierge_client_->set_disk_image_status_signals(signals);
+
+  auto result = OnResizeWithResult(nullptr, "vm_name", 12345);
+
+  EXPECT_EQ(result, true);
+}
+
 }  // namespace disk
 }  // namespace crostini

chromeos/dbus/fake_concierge_client.cc

@@ -92,8 +92,10 @@ void FakeConciergeClient::ImportDiskImage(
   import_disk_image_called_ = true;
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::BindOnce(&FakeConciergeClient::FakeImportCallbacks,
-                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+      base::BindOnce(std::move(callback), import_disk_image_response_));
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&FakeConciergeClient::NotifyDiskImageProgress,
+                                weak_ptr_factory_.GetWeakPtr()));
 }
 
 void FakeConciergeClient::CancelDiskImageOperation(
@@ -106,9 +108,7 @@ void FakeConciergeClient::CancelDiskImageOperation(
       base::BindOnce(std::move(callback), cancel_disk_image_response_));
 }
 
-void FakeConciergeClient::FakeImportCallbacks(
-    DBusMethodCallback<vm_tools::concierge::ImportDiskImageResponse> callback) {
-  std::move(callback).Run(import_disk_image_response_);
+void FakeConciergeClient::NotifyDiskImageProgress() {
   // Trigger DiskImageStatus signals.
   for (auto const& signal : disk_image_status_signals_) {
     OnDiskImageProgress(signal);
@@ -275,6 +275,9 @@ void FakeConciergeClient::ResizeDiskImage(
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(std::move(callback), resize_disk_image_response_));
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(&FakeConciergeClient::NotifyDiskImageProgress,
+                                weak_ptr_factory_.GetWeakPtr()));
 }
 
 void FakeConciergeClient::NotifyVmStarted(

chromeos/dbus/fake_concierge_client.h

@@ -254,10 +254,8 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeConciergeClient
   void NotifyTremplinStarted(
       const vm_tools::cicerone::TremplinStartedSignal& signal);
 
-  // Fakes a sequence of progress callbacks.
-  void FakeImportCallbacks(
-      DBusMethodCallback<vm_tools::concierge::ImportDiskImageResponse>
-          callback);
+  // Notifies observers with a sequence of DiskImageStatus signals.
+  void NotifyDiskImageProgress();
   // Notifies observers with a DiskImageStatus signal.
   void OnDiskImageProgress(
       const vm_tools::concierge::DiskImageStatusResponse& signal);