update_engine: Use FileStream in FilesystemVerifierAction.

Instead of dealing manually with the read from the partition while
verifying the filesystem, this patch uses chromeos::FileStream to do
that.

The base::MessageLoopForIO can't wait on regular files because libevent
will fail when trying to watch for such file descriptor. While
chromeos::Stream will still attempt to wait on the file descriptor in
such situation, that will be fixed in chromeos::Streams in a different
CL.

BUG=chromium:499886
TEST=Unittests still pass.

Change-Id: Icd6a031ac1137b1b5746a5c3413506ece79223cc
Reviewed-on: https://chromium-review.googlesource.com/290542
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Trybot-Ready: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/filesystem_verifier_action.cc b/filesystem_verifier_action.cc
index 5c352b0..d3e9fe3 100644
--- a/filesystem_verifier_action.cc
+++ b/filesystem_verifier_action.cc
@@ -14,13 +14,12 @@
 #include <string>
 
 #include <base/bind.h>
-#include <base/posix/eintr_wrapper.h>
+#include <chromeos/streams/file_stream.h>
 
 #include "update_engine/hardware_interface.h"
 #include "update_engine/system_state.h"
 #include "update_engine/utils.h"
 
-using chromeos::MessageLoop;
 using std::string;
 
 namespace chromeos_update_engine {
@@ -97,23 +96,23 @@
       break;
   }
 
-  src_fd_ = HANDLE_EINTR(open(target_path.c_str(), O_RDONLY));
-  if (src_fd_ < 0) {
-    PLOG(ERROR) << "Unable to open " << target_path << " for reading";
+  chromeos::ErrorPtr error;
+  src_stream_ = chromeos::FileStream::Open(
+      base::FilePath(target_path),
+      chromeos::Stream::AccessMode::READ,
+      chromeos::FileStream::Disposition::OPEN_EXISTING,
+      &error);
+
+  if (!src_stream_) {
+    LOG(ERROR) << "Unable to open " << target_path << " for reading";
     return;
   }
 
-  DetermineFilesystemSize(src_fd_);
+  DetermineFilesystemSize(target_path);
   buffer_.resize(kReadFileBufferSize);
 
   // Start the first read.
-  read_task_ = MessageLoop::current()->WatchFileDescriptor(
-      FROM_HERE,
-      src_fd_,
-      MessageLoop::WatchMode::kWatchRead,
-      true,  // persistent
-      base::Bind(&FilesystemVerifierAction::OnReadReadyCallback,
-                 base::Unretained(this)));
+  ScheduleRead();
 
   abort_action_completer.set_should_complete(false);
 }
@@ -124,19 +123,11 @@
 }
 
 bool FilesystemVerifierAction::IsCleanupPending() const {
-  return (src_fd_ != -1);
+  return src_stream_ != nullptr;
 }
 
 void FilesystemVerifierAction::Cleanup(ErrorCode code) {
-  MessageLoop::current()->CancelTask(read_task_);
-  read_task_ = MessageLoop::kTaskIdNull;
-
-  if (src_fd_ != -1) {
-    if (IGNORE_EINTR(close(src_fd_)) != 0) {
-      PLOG(ERROR) << "Error closing fd " << src_fd_;
-    }
-    src_fd_ = -1;
-  }
+  src_stream_.reset();
   // This memory is not used anymore.
   buffer_.clear();
 
@@ -147,74 +138,97 @@
   processor_->ActionComplete(this, code);
 }
 
-void FilesystemVerifierAction::OnReadReadyCallback() {
+void FilesystemVerifierAction::ScheduleRead() {
   size_t bytes_to_read = std::min(static_cast<int64_t>(buffer_.size()),
                                   remaining_size_);
-
-  ssize_t bytes_read = 0;
-  if (bytes_to_read) {
-    bytes_read = HANDLE_EINTR(read(src_fd_, buffer_.data(), bytes_to_read));
-  }
-  if (bytes_read < 0) {
-    PLOG(ERROR) << "Read failed";
-    failed_ = true;
-  } else if (bytes_read == 0) {
-    read_done_ = true;
-  } else {
-    remaining_size_ -= bytes_read;
-  }
-
-  if (bytes_read > 0) {
-    CHECK(!read_done_);
-    if (!hasher_.Update(buffer_.data(), bytes_read)) {
-      LOG(ERROR) << "Unable to update the hash.";
-      failed_ = true;
-    }
-  }
-
-  CheckTerminationConditions();
-}
-
-void FilesystemVerifierAction::CheckTerminationConditions() {
-  if (failed_ || cancelled_) {
-    Cleanup(ErrorCode::kError);
+  if (!bytes_to_read) {
+    OnReadDoneCallback(0);
     return;
   }
 
-  if (read_done_) {
-    // We're done!
-    ErrorCode code = ErrorCode::kSuccess;
-    if (hasher_.Finalize()) {
-      LOG(INFO) << "Hash: " << hasher_.hash();
-      switch (partition_type_) {
-        case PartitionType::kRootfs:
-          if (install_plan_.rootfs_hash != hasher_.raw_hash()) {
-            code = ErrorCode::kNewRootfsVerificationError;
-            LOG(ERROR) << "New rootfs verification failed.";
-          }
-          break;
-        case PartitionType::kKernel:
-          if (install_plan_.kernel_hash != hasher_.raw_hash()) {
-            code = ErrorCode::kNewKernelVerificationError;
-            LOG(ERROR) << "New kernel verification failed.";
-          }
-          break;
-        case PartitionType::kSourceRootfs:
-          install_plan_.source_rootfs_hash = hasher_.raw_hash();
-          break;
-        case PartitionType::kSourceKernel:
-          install_plan_.source_kernel_hash = hasher_.raw_hash();
-          break;
-      }
-    } else {
-      LOG(ERROR) << "Unable to finalize the hash.";
-      code = ErrorCode::kError;
-    }
-    Cleanup(code);
+  bool read_async_ok = src_stream_->ReadAsync(
+    buffer_.data(),
+    bytes_to_read,
+    base::Bind(&FilesystemVerifierAction::OnReadDoneCallback,
+               base::Unretained(this)),
+    base::Bind(&FilesystemVerifierAction::OnReadErrorCallback,
+               base::Unretained(this)),
+    nullptr);
+
+  if (!read_async_ok) {
+    LOG(ERROR) << "Unable to schedule an asynchronous read from the stream.";
+    Cleanup(ErrorCode::kError);
   }
 }
 
-void FilesystemVerifierAction::DetermineFilesystemSize(int fd) {
+void FilesystemVerifierAction::OnReadDoneCallback(size_t bytes_read) {
+  if (bytes_read == 0) {
+    read_done_ = true;
+  } else {
+    remaining_size_ -= bytes_read;
+    CHECK(!read_done_);
+    if (!hasher_.Update(buffer_.data(), bytes_read)) {
+      LOG(ERROR) << "Unable to update the hash.";
+      Cleanup(ErrorCode::kError);
+      return;
+    }
+  }
+
+  // We either terminate the action or have more data to read.
+  if (!CheckTerminationConditions())
+    ScheduleRead();
+}
+
+void FilesystemVerifierAction::OnReadErrorCallback(
+      const chromeos::Error* error) {
+  // TODO(deymo): Transform the read-error into an specific ErrorCode.
+  LOG(ERROR) << "Asynchronous read failed.";
+  Cleanup(ErrorCode::kError);
+}
+
+bool FilesystemVerifierAction::CheckTerminationConditions() {
+  if (cancelled_) {
+    Cleanup(ErrorCode::kError);
+    return true;
+  }
+
+  if (!read_done_)
+    return false;
+
+  // We're done!
+  ErrorCode code = ErrorCode::kSuccess;
+  if (hasher_.Finalize()) {
+    LOG(INFO) << "Hash: " << hasher_.hash();
+    switch (partition_type_) {
+      case PartitionType::kRootfs:
+        if (install_plan_.rootfs_hash != hasher_.raw_hash()) {
+          code = ErrorCode::kNewRootfsVerificationError;
+          LOG(ERROR) << "New rootfs verification failed.";
+        }
+        break;
+      case PartitionType::kKernel:
+        if (install_plan_.kernel_hash != hasher_.raw_hash()) {
+          code = ErrorCode::kNewKernelVerificationError;
+          LOG(ERROR) << "New kernel verification failed.";
+        }
+        break;
+      case PartitionType::kSourceRootfs:
+        install_plan_.source_rootfs_hash = hasher_.raw_hash();
+        break;
+      case PartitionType::kSourceKernel:
+        install_plan_.source_kernel_hash = hasher_.raw_hash();
+        break;
+    }
+  } else {
+    LOG(ERROR) << "Unable to finalize the hash.";
+    code = ErrorCode::kError;
+  }
+  Cleanup(code);
+  return true;
+}
+
+void FilesystemVerifierAction::DetermineFilesystemSize(
+    const std::string& path) {
   switch (partition_type_) {
     case PartitionType::kRootfs:
       remaining_size_ = install_plan_.rootfs_size;
@@ -227,7 +241,7 @@
     case PartitionType::kSourceRootfs:
       {
         int block_count = 0, block_size = 0;
-        if (utils::GetFilesystemSizeFromFD(fd, &block_count, &block_size)) {
+        if (utils::GetFilesystemSize(path, &block_count, &block_size)) {
           remaining_size_ = static_cast<int64_t>(block_count) * block_size;
           LOG(INFO) << "Filesystem size: " << remaining_size_ << " bytes ("
                     << block_count << "x" << block_size << ").";
@@ -237,7 +251,6 @@
     default:
       break;
   }
-  return;
 }
 
 }  // namespace chromeos_update_engine
diff --git a/filesystem_verifier_action.h b/filesystem_verifier_action.h
index 69e4972..731790a 100644
--- a/filesystem_verifier_action.h
+++ b/filesystem_verifier_action.h
@@ -11,7 +11,7 @@
 #include <string>
 #include <vector>
 
-#include <chromeos/message_loops/message_loop.h>
+#include <chromeos/streams/stream.h>
 #include <gtest/gtest_prod.h>  // for FRIEND_TEST
 
 #include "update_engine/action.h"
@@ -56,13 +56,17 @@
   FRIEND_TEST(FilesystemVerifierActionTest,
               RunAsRootDetermineFilesystemSizeTest);
 
-  // Callback from the main loop when there's data to read from the file
-  // descriptor.
-  void OnReadReadyCallback();
+  // Schedules the asynchronous read of the filesystem.
+  void ScheduleRead();
 
-  // Based on the state of the read buffers, terminates read process and the
-  // action.
-  void CheckTerminationConditions();
+  // Called from the main loop when a single read from |src_stream_| succeeds or
+  // fails, calling OnReadDoneCallback() and OnReadErrorCallback() respectively.
+  void OnReadDoneCallback(size_t bytes_read);
+  void OnReadErrorCallback(const chromeos::Error* error);
+
+  // Based on the state of the read buffer, terminates read process and the
+  // action. Return whether the action was terminated.
+  bool CheckTerminationConditions();
 
   // Cleans up all the variables we use for async operations and tells the
   // ActionProcessor we're done w/ |code| as passed in. |cancelled_| should be
@@ -72,23 +76,18 @@
   // Determine, if possible, the source file system size to avoid copying the
   // whole partition. Currently this supports only the root file system assuming
   // it's ext3-compatible.
-  void DetermineFilesystemSize(int fd);
+  void DetermineFilesystemSize(const std::string& path);
 
   // The type of the partition that we are verifying.
   PartitionType partition_type_;
 
-  // If non-null, this is the GUnixInputStream object for the opened source
-  // partition.
-  int src_fd_{-1};
+  // If not null, the FileStream used to read from the device.
+  chromeos::StreamPtr src_stream_;
 
   // Buffer for storing data we read.
   chromeos::Blob buffer_;
 
-  // The task id for the the in-flight async call.
-  chromeos::MessageLoop::TaskId read_task_{chromeos::MessageLoop::kTaskIdNull};
-
   bool read_done_{false};  // true if reached EOF on the input stream.
-  bool failed_{false};  // true if the action has failed.
   bool cancelled_{false};  // true if the action has been cancelled.
 
   // The install plan we're passed in via the input pipe.
diff --git a/filesystem_verifier_action_unittest.cc b/filesystem_verifier_action_unittest.cc
index da0fd95..66f8571 100644
--- a/filesystem_verifier_action_unittest.cc
+++ b/filesystem_verifier_action_unittest.cc
@@ -352,6 +352,8 @@
 TEST_F(FilesystemVerifierActionTest, RunAsRootTerminateEarlyTest) {
   ASSERT_EQ(0, getuid());
   EXPECT_TRUE(DoTest(true, false, PartitionType::kKernel));
+  // TerminateEarlyTest may leak some null callbacks from the Stream class.
+  while (loop_.RunOnce(false)) {}
 }
 
 TEST_F(FilesystemVerifierActionTest, RunAsRootDetermineFilesystemSizeTest) {
@@ -362,19 +364,18 @@
   // Extend the "partition" holding the file system from 10MiB to 20MiB.
   EXPECT_EQ(0, truncate(img.c_str(), 20 * 1024 * 1024));
 
-  for (int i = 0; i < 2; ++i) {
-    PartitionType fs_type =
-        i ? PartitionType::kSourceKernel : PartitionType::kSourceRootfs;
-    FilesystemVerifierAction action(&fake_system_state_, fs_type);
+  {
+    FilesystemVerifierAction action(&fake_system_state_,
+                                    PartitionType::kSourceKernel);
     EXPECT_EQ(kint64max, action.remaining_size_);
-    {
-      int fd = HANDLE_EINTR(open(img.c_str(), O_RDONLY));
-      EXPECT_GT(fd, 0);
-      ScopedFdCloser fd_closer(&fd);
-      action.DetermineFilesystemSize(fd);
-    }
-    EXPECT_EQ(i ? kint64max : 10 * 1024 * 1024,
-              action.remaining_size_);
+    action.DetermineFilesystemSize(img);
+    EXPECT_EQ(kint64max, action.remaining_size_);
+  }
+  {
+    FilesystemVerifierAction action(&fake_system_state_,
+                                    PartitionType::kSourceRootfs);
+    action.DetermineFilesystemSize(img);
+    EXPECT_EQ(10 * 1024 * 1024, action.remaining_size_);
   }
 }