Add client methods for Concierge's ListVmDisks member

- Adds methods to concierge_client and crostini_manager for
accessing Concierge's ListVmDisks member.

- Changes argument naming in
c/b/chromeos/crostini/crostini_manager.h to match ditto.cc

- Fixes some linter errors

Bug: 822499
Change-Id: I981bb90a158bdb4d9cb6ebf3da1536c3ab6161c2
Reviewed-on: https://chromium-review.googlesource.com/1086429Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Ryo Hashimoto <hashimoto@chromium.org>
Commit-Queue: Renée Wright <rjwright@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564695}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -570,6 +570,26 @@ void CrostiniManager::DestroyDiskImage(
                      weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
 }
 
+void CrostiniManager::ListVmDisks(
+    // The cryptohome id for the user's encrypted storage.
+    const std::string& cryptohome_id,
+    ListVmDisksCallback callback) {
+  if (cryptohome_id.empty()) {
+    LOG(ERROR) << "Cryptohome id cannot be empty";
+    std::move(callback).Run(ConciergeClientResult::CLIENT_ERROR, 0);
+    return;
+  }
+
+  vm_tools::concierge::ListVmDisksRequest request;
+  request.set_cryptohome_id(std::move(cryptohome_id));
+  request.set_storage_location(vm_tools::concierge::STORAGE_CRYPTOHOME_ROOT);
+
+  GetConciergeClient()->ListVmDisks(
+      std::move(request),
+      base::BindOnce(&CrostiniManager::OnListVmDisks,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
 void CrostiniManager::StartTerminaVm(std::string owner_id,
                                      std::string name,
                                      const base::FilePath& disk_path,
@@ -847,6 +867,26 @@ void CrostiniManager::OnDestroyDiskImage(
   std::move(callback).Run(ConciergeClientResult::SUCCESS);
 }
 
+void CrostiniManager::OnListVmDisks(
+    ListVmDisksCallback callback,
+    base::Optional<vm_tools::concierge::ListVmDisksResponse> reply) {
+  if (!reply.has_value()) {
+    LOG(ERROR) << "Failed to get list of VM disks. Empty response.";
+    std::move(callback).Run(ConciergeClientResult::LIST_VM_DISKS_FAILED, 0);
+    return;
+  }
+  vm_tools::concierge::ListVmDisksResponse response = std::move(reply).value();
+
+  if (!response.success()) {
+    LOG(ERROR) << "Failed to list VM disks: " << response.failure_reason();
+    std::move(callback).Run(ConciergeClientResult::LIST_VM_DISKS_FAILED, 0);
+    return;
+  }
+
+  std::move(callback).Run(ConciergeClientResult::SUCCESS,
+                          response.total_size());
+}
+
 void CrostiniManager::OnStartTerminaVm(
     StartTerminaVmCallback callback,
     base::Optional<vm_tools::concierge::StartVmResponse> reply) {

chrome/browser/chromeos/crostini/crostini_manager.h

@@ -6,7 +6,10 @@
 #define CHROME_BROWSER_CHROMEOS_CROSTINI_CROSTINI_MANAGER_H_
 
 #include <map>
+#include <string>
+#include <tuple>
 #include <utility>
+#include <vector>
 
 #include "base/files/file_path.h"
 #include "base/memory/singleton.h"
@@ -30,6 +33,7 @@ enum class ConciergeClientResult {
   VM_START_FAILED,
   VM_STOP_FAILED,
   DESTROY_DISK_IMAGE_FAILED,
+  LIST_VM_DISKS_FAILED,
   CLIENT_ERROR,
   DISK_TYPE_ERROR,
   CONTAINER_START_FAILED,
@@ -71,6 +75,10 @@ class CrostiniManager : public chromeos::ConciergeClient::Observer,
                               const base::FilePath& disk_path)>;
   // The type of the callback for CrostiniManager::DestroyDiskImage.
   using DestroyDiskImageCallback = ConciergeClientCallback;
+  // The type of the callback for CrostiniManager::ListVmDisks.
+  using ListVmDisksCallback =
+      base::OnceCallback<void(ConciergeClientResult result,
+                              int64_t total_size)>;
   // The type of the callback for CrostiniManager::StopVm.
   using StopVmCallback = ConciergeClientCallback;
   // The type of the callback for CrostiniManager::StartContainer.
@@ -143,6 +151,11 @@ class CrostiniManager : public chromeos::ConciergeClient::Observer,
       vm_tools::concierge::StorageLocation storage_location,
       DestroyDiskImageCallback callback);
 
+  void ListVmDisks(
+      // The cryptohome id for the user's encrypted storage.
+      const std::string& cryptohome_id,
+      ListVmDisksCallback callback);
+
   // Checks the arguments for starting a Termina VM. Starts a Termina VM via
   // ConciergeClient::StartTerminaVm. |callback| is called if the arguments
   // are bad, or after the method call finishes.
@@ -253,24 +266,30 @@ class CrostiniManager : public chromeos::ConciergeClient::Observer,
   // service method finishes.
   void OnCreateDiskImage(
       CreateDiskImageCallback callback,
-      base::Optional<vm_tools::concierge::CreateDiskImageResponse> response);
+      base::Optional<vm_tools::concierge::CreateDiskImageResponse> reply);
 
   // Callback for ConciergeClient::DestroyDiskImage. Called after the Concierge
   // service method finishes.
   void OnDestroyDiskImage(
       DestroyDiskImageCallback callback,
-      base::Optional<vm_tools::concierge::DestroyDiskImageResponse> response);
+      base::Optional<vm_tools::concierge::DestroyDiskImageResponse> reply);
+
+  // Callback for ConciergeClient::ListVmDisks. Called after the Concierge
+  // service method finishes.
+  void OnListVmDisks(
+      ListVmDisksCallback callback,
+      base::Optional<vm_tools::concierge::ListVmDisksResponse> reply);
 
   // Callback for ConciergeClient::StartTerminaVm. Called after the Concierge
   // service method finishes.
   void OnStartTerminaVm(
       StartTerminaVmCallback callback,
-      base::Optional<vm_tools::concierge::StartVmResponse> response);
+      base::Optional<vm_tools::concierge::StartVmResponse> reply);
 
   // Callback for ConciergeClient::StopVm. Called after the Concierge
   // service method finishes.
   void OnStopVm(StopVmCallback callback,
-                base::Optional<vm_tools::concierge::StopVmResponse> response);
+                base::Optional<vm_tools::concierge::StopVmResponse> reply);
 
   // Callback for CrostiniClient::StartConcierge. Called after the
   // DebugDaemon service method finishes.
@@ -287,25 +306,25 @@ class CrostiniManager : public chromeos::ConciergeClient::Observer,
       std::string vm_name,
       std::string container_name,
       StartContainerCallback callback,
-      base::Optional<vm_tools::concierge::StartContainerResponse> response);
+      base::Optional<vm_tools::concierge::StartContainerResponse> reply);
 
   // Callback for CrostiniManager::LaunchContainerApplication.
   void OnLaunchContainerApplication(
       LaunchContainerApplicationCallback callback,
       base::Optional<vm_tools::concierge::LaunchContainerApplicationResponse>
-          response);
+          reply);
 
   // Callback for CrostiniManager::GetContainerAppIcons. Called after the
   // Concierge service finishes.
   void OnGetContainerAppIcons(
       GetContainerAppIconsCallback callback,
-      base::Optional<vm_tools::concierge::ContainerAppIconResponse> response);
+      base::Optional<vm_tools::concierge::ContainerAppIconResponse> reply);
 
   // Callback for CrostiniManager::GetContainerSshKeys. Called after the
   // Concierge service finishes.
   void OnGetContainerSshKeys(
       GetContainerSshKeysCallback callback,
-      base::Optional<vm_tools::concierge::ContainerSshKeysResponse> response);
+      base::Optional<vm_tools::concierge::ContainerSshKeysResponse> reply);
 
   // Helper for CrostiniManager::CreateDiskImage. Separated so it can be run
   // off the main thread.

chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

@@ -41,6 +41,14 @@ class CrostiniManagerTest : public testing::Test {
     std::move(closure).Run();
   }
 
+  void ListVmDisksClientErrorCallback(base::OnceClosure closure,
+                                      ConciergeClientResult result,
+                                      int64_t total_size) {
+    EXPECT_FALSE(fake_concierge_client_->list_vm_disks_called());
+    EXPECT_EQ(result, ConciergeClientResult::CLIENT_ERROR);
+    std::move(closure).Run();
+  }
+
   void StartTerminaVmClientErrorCallback(base::OnceClosure closure,
                                          ConciergeClientResult result) {
     EXPECT_FALSE(fake_concierge_client_->start_termina_vm_called());
@@ -75,6 +83,13 @@ class CrostiniManagerTest : public testing::Test {
     std::move(closure).Run();
   }
 
+  void ListVmDisksSuccessCallback(base::OnceClosure closure,
+                                  ConciergeClientResult result,
+                                  int64_t total_size) {
+    EXPECT_TRUE(fake_concierge_client_->list_vm_disks_called());
+    std::move(closure).Run();
+  }
+
   void StartTerminaVmSuccessCallback(base::OnceClosure closure,
                                      ConciergeClientResult result) {
     EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
@@ -141,7 +156,7 @@ TEST_F(CrostiniManagerTest, CreateDiskImageNameError) {
   const base::FilePath& disk_path = base::FilePath("");
 
   CrostiniManager::GetInstance()->CreateDiskImage(
-      "i_dont_know_what_cryptohome_id_is", disk_path,
+      "a_cryptohome_id", disk_path,
       vm_tools::concierge::STORAGE_CRYPTOHOME_ROOT,
       base::BindOnce(&CrostiniManagerTest::CreateDiskImageClientErrorCallback,
                      base::Unretained(this), run_loop()->QuitClosure()));
@@ -162,7 +177,7 @@ TEST_F(CrostiniManagerTest, CreateDiskImageStorageLocationError) {
   const base::FilePath& disk_path = base::FilePath(kVmName);
 
   CrostiniManager::GetInstance()->CreateDiskImage(
-      "i_dont_know_what_cryptohome_id_is", disk_path,
+      "a_cryptohome_id", disk_path,
       vm_tools::concierge::StorageLocation_INT_MIN_SENTINEL_DO_NOT_USE_,
       base::BindOnce(&CrostiniManagerTest::CreateDiskImageClientErrorCallback,
                      base::Unretained(this), run_loop()->QuitClosure()));
@@ -173,7 +188,7 @@ TEST_F(CrostiniManagerTest, CreateDiskImageSuccess) {
   const base::FilePath& disk_path = base::FilePath(kVmName);
 
   CrostiniManager::GetInstance()->CreateDiskImage(
-      "i_dont_know_what_cryptohome_id_is", disk_path,
+      "a_cryptohome_id", disk_path,
       vm_tools::concierge::STORAGE_CRYPTOHOME_DOWNLOADS,
       base::BindOnce(&CrostiniManagerTest::CreateDiskImageSuccessCallback,
                      base::Unretained(this), run_loop()->QuitClosure()));
@@ -184,7 +199,7 @@ TEST_F(CrostiniManagerTest, DestroyDiskImageNameError) {
   const base::FilePath& disk_path = base::FilePath("");
 
   CrostiniManager::GetInstance()->DestroyDiskImage(
-      "i_dont_know_what_cryptohome_id_is", disk_path,
+      "a_cryptohome_id", disk_path,
       vm_tools::concierge::STORAGE_CRYPTOHOME_ROOT,
       base::BindOnce(&CrostiniManagerTest::DestroyDiskImageClientErrorCallback,
                      base::Unretained(this), run_loop()->QuitClosure()));
@@ -205,7 +220,7 @@ TEST_F(CrostiniManagerTest, DestroyDiskImageStorageLocationError) {
   const base::FilePath& disk_path = base::FilePath(kVmName);
 
   CrostiniManager::GetInstance()->DestroyDiskImage(
-      "i_dont_know_what_cryptohome_id_is", disk_path,
+      "a_cryptohome_id", disk_path,
       vm_tools::concierge::StorageLocation_INT_MIN_SENTINEL_DO_NOT_USE_,
       base::BindOnce(&CrostiniManagerTest::DestroyDiskImageClientErrorCallback,
                      base::Unretained(this), run_loop()->QuitClosure()));
@@ -216,13 +231,28 @@ TEST_F(CrostiniManagerTest, DestroyDiskImageSuccess) {
   const base::FilePath& disk_path = base::FilePath(kVmName);
 
   CrostiniManager::GetInstance()->DestroyDiskImage(
-      "i_dont_know_what_cryptohome_id_is", disk_path,
+      "a_cryptohome_id", disk_path,
       vm_tools::concierge::STORAGE_CRYPTOHOME_DOWNLOADS,
       base::BindOnce(&CrostiniManagerTest::DestroyDiskImageSuccessCallback,
                      base::Unretained(this), run_loop()->QuitClosure()));
   run_loop()->Run();
 }
 
+TEST_F(CrostiniManagerTest, ListVmDisksCryptohomeError) {
+  CrostiniManager::GetInstance()->ListVmDisks(
+      "", base::BindOnce(&CrostiniManagerTest::ListVmDisksClientErrorCallback,
+                         base::Unretained(this), run_loop()->QuitClosure()));
+  run_loop()->Run();
+}
+
+TEST_F(CrostiniManagerTest, ListVmDisksSuccess) {
+  CrostiniManager::GetInstance()->ListVmDisks(
+      "a_cryptohome_id",
+      base::BindOnce(&CrostiniManagerTest::ListVmDisksSuccessCallback,
+                     base::Unretained(this), run_loop()->QuitClosure()));
+  run_loop()->Run();
+}
+
 TEST_F(CrostiniManagerTest, StartTerminaVmOwnerIdError) {
   const base::FilePath& disk_path = base::FilePath(kVmName);
 

chromeos/dbus/concierge_client.cc

@@ -4,6 +4,9 @@
 
 #include "chromeos/dbus/concierge_client.h"
 
+#include <string>
+#include <utility>
+
 #include "base/bind.h"
 #include "base/observer_list.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -80,6 +83,26 @@ class ConciergeClientImpl : public ConciergeClient {
                        weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
+  void ListVmDisks(const vm_tools::concierge::ListVmDisksRequest& request,
+                   DBusMethodCallback<vm_tools::concierge::ListVmDisksResponse>
+                       callback) override {
+    dbus::MethodCall method_call(vm_tools::concierge::kVmConciergeInterface,
+                                 vm_tools::concierge::kListVmDisksMethod);
+    dbus::MessageWriter writer(&method_call);
+
+    if (!writer.AppendProtoAsArrayOfBytes(request)) {
+      LOG(ERROR) << "Failed to encode ListVmDisksRequest protobuf";
+      std::move(callback).Run(base::nullopt);
+      return;
+    }
+
+    concierge_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::BindOnce(&ConciergeClientImpl::OnDBusProtoResponse<
+                           vm_tools::concierge::ListVmDisksResponse>,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+  }
+
   void StartTerminaVm(const vm_tools::concierge::StartVmRequest& request,
                       DBusMethodCallback<vm_tools::concierge::StartVmResponse>
                           callback) override {

chromeos/dbus/concierge_client.h

@@ -62,6 +62,13 @@ class CHROMEOS_EXPORT ConciergeClient : public DBusClient {
       DBusMethodCallback<vm_tools::concierge::DestroyDiskImageResponse>
           callback) = 0;
 
+  // Lists the Termina VMs.
+  // |callback| is called after the method call finishes.
+  virtual void ListVmDisks(
+      const vm_tools::concierge::ListVmDisksRequest& request,
+      DBusMethodCallback<vm_tools::concierge::ListVmDisksResponse>
+          callback) = 0;
+
   // Starts a Termina VM if there is not alread one running.
   // |callback| is called after the method call finishes.
   virtual void StartTerminaVm(

chromeos/dbus/fake_concierge_client.cc

@@ -4,6 +4,8 @@
 
 #include "chromeos/dbus/fake_concierge_client.h"
 
+#include <utility>
+
 #include "base/threading/thread_task_runner_handle.h"
 
 namespace chromeos {
@@ -53,6 +55,16 @@ void FakeConciergeClient::DestroyDiskImage(
       FROM_HERE, base::BindOnce(std::move(callback), std::move(response)));
 }
 
+void FakeConciergeClient::ListVmDisks(
+    const vm_tools::concierge::ListVmDisksRequest& request,
+    DBusMethodCallback<vm_tools::concierge::ListVmDisksResponse> callback) {
+  list_vm_disks_called_ = true;
+  vm_tools::concierge::ListVmDisksResponse response;
+  response.set_success(true);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), std::move(response)));
+}
+
 void FakeConciergeClient::StartTerminaVm(
     const vm_tools::concierge::StartVmRequest& request,
     DBusMethodCallback<vm_tools::concierge::StartVmResponse> callback) {

chromeos/dbus/fake_concierge_client.h

@@ -31,7 +31,7 @@ class CHROMEOS_EXPORT FakeConciergeClient : public ConciergeClient {
   bool IsContainerStartupFailedSignalConnected() override;
 
   // Fake version of the method that creates a disk image for the Termina VM.
-  // Sets create_disk_image_called. |callback| is called after the method
+  // Sets create_disk_image_called_. |callback| is called after the method
   // call finishes.
   void CreateDiskImage(
       const vm_tools::concierge::CreateDiskImageRequest& request,
@@ -39,29 +39,37 @@ class CHROMEOS_EXPORT FakeConciergeClient : public ConciergeClient {
       override;
 
   // Fake version of the method that destroys a Termina VM and removes its disk
-  // image. Sets destroy_disk_image_called. |callback| is called after the
+  // image. Sets destroy_disk_image_called_. |callback| is called after the
   // method call finishes.
   void DestroyDiskImage(
       const vm_tools::concierge::DestroyDiskImageRequest& request,
       DBusMethodCallback<vm_tools::concierge::DestroyDiskImageResponse>
           callback) override;
 
+  // Fake version of the method that lists Termina VM disks. Sets
+  // list_vm_disks_called_. |callback| is called after the method call
+  // finishes.
+  void ListVmDisks(const vm_tools::concierge::ListVmDisksRequest& request,
+                   DBusMethodCallback<vm_tools::concierge::ListVmDisksResponse>
+                       callback) override;
+
   // Fake version of the method that starts a Termina VM. Sets
-  // start_termina_vm_called. |callback| is called after the method call
+  // start_termina_vm_called_. |callback| is called after the method call
   // finishes.
   void StartTerminaVm(const vm_tools::concierge::StartVmRequest& request,
                       DBusMethodCallback<vm_tools::concierge::StartVmResponse>
                           callback) override;
 
   // Fake version of the method that stops the named Termina VM if it is
-  // running. Sets stop_vm_called. |callback| is called after the method
+  // running. Sets stop_vm_called_. |callback| is called after the method
   // call finishes.
   void StopVm(const vm_tools::concierge::StopVmRequest& request,
               DBusMethodCallback<vm_tools::concierge::StopVmResponse> callback)
       override;
 
   // Fake version of the method that starts a Container inside an existing
-  // Termina VM. |callback| is called after the method call finishes.
+  // Termina VM. Sets start_container_called_. |callback| is called after the
+  // method call finishes.
   void StartContainer(
       const vm_tools::concierge::StartContainerRequest& request,
       DBusMethodCallback<vm_tools::concierge::StartContainerResponse> callback)
@@ -98,6 +106,8 @@ class CHROMEOS_EXPORT FakeConciergeClient : public ConciergeClient {
   bool create_disk_image_called() const { return create_disk_image_called_; }
   // Indicates whether DestroyDiskImage has been called
   bool destroy_disk_image_called() const { return destroy_disk_image_called_; }
+  // Indicates whether ListVmDisks has been called
+  bool list_vm_disks_called() const { return list_vm_disks_called_; }
   // Indicates whether StartTerminaVm has been called
   bool start_termina_vm_called() const { return start_termina_vm_called_; }
   // Indicates whether StopVm has been called
@@ -119,6 +129,7 @@ class CHROMEOS_EXPORT FakeConciergeClient : public ConciergeClient {
  private:
   bool create_disk_image_called_ = false;
   bool destroy_disk_image_called_ = false;
+  bool list_vm_disks_called_ = false;
   bool start_termina_vm_called_ = false;
   bool stop_vm_called_ = false;
   bool start_container_called_ = false;