[paver] Don't attempt to bind FVM if it's already bound.
This eliminates a devhost deadlock which occurs when attempting to wait
for bind to finish.
Tested: Paved astro, NUC, and vim2
Change-Id: I97a8c61ea9f52b7d9c74320ba744113593bd541f
diff --git a/zircon/system/ulib/paver/BUILD.gn b/zircon/system/ulib/paver/BUILD.gn
index 4ee2857..ef45b15 100644
--- a/zircon/system/ulib/paver/BUILD.gn
+++ b/zircon/system/ulib/paver/BUILD.gn
@@ -55,6 +55,7 @@
output_name = "paver-test"
sources = [
"test/device-partitioner-test.cpp",
+ "test/fvm-test.cpp",
"test/paversvc-test.cpp",
"test/stream-reader-test.cpp",
"test/test-utils.cpp",
diff --git a/zircon/system/ulib/paver/include/lib/paver/paver.h b/zircon/system/ulib/paver/include/lib/paver/paver.h
index 994366e..a35423c 100644
--- a/zircon/system/ulib/paver/include/lib/paver/paver.h
+++ b/zircon/system/ulib/paver/include/lib/paver/paver.h
@@ -13,6 +13,18 @@
namespace paver {
+// Options for locating an FVM within a partition.
+enum class BindOption {
+ // Bind to the FVM, if it exists already.
+ TryBind,
+ // Reformat the partition, regardless of if it already exists as an FVM.
+ Reformat,
+};
+
+// Public for testing.
+fbl::unique_fd FvmPartitionFormat(fbl::unique_fd partition_fd, size_t slice_size,
+ BindOption option);
+
class Paver {
public:
// Writes a kernel or verified boot metadata payload to the appropriate
diff --git a/zircon/system/ulib/paver/paver.cpp b/zircon/system/ulib/paver/paver.cpp
index 9db609b..651bcc9 100644
--- a/zircon/system/ulib/paver/paver.cpp
+++ b/zircon/system/ulib/paver/paver.cpp
@@ -393,6 +393,16 @@
return fbl::unique_fd();
}
+ // We assume the FVM will either have completed binding, or is not bound at all. This is ensured
+ // by the paver always waiting for the FVM to bind after invoking ControllerBind.
+ char fvm_path[PATH_MAX];
+ snprintf(fvm_path, sizeof(fvm_path), "%s/fvm", path);
+
+ fbl::unique_fd fvm(open(fvm_path, O_RDWR));
+ if (fvm) {
+ return fvm;
+ }
+
fdio_t* io = fdio_unsafe_fd_to_io(partition_fd.get());
if (io == nullptr) {
ERROR("Failed to convert to io\n");
@@ -411,8 +421,6 @@
return fbl::unique_fd();
}
- char fvm_path[PATH_MAX];
- snprintf(fvm_path, sizeof(fvm_path), "%s/fvm", path);
if (wait_for_device(fvm_path, timeout.get()) != ZX_OK) {
ERROR("Error waiting for fvm driver to bind\n");
return fbl::unique_fd();
@@ -420,13 +428,7 @@
return fbl::unique_fd(open(fvm_path, O_RDWR));
}
-// Options for locating an FVM within a partition.
-enum class BindOption {
- // Bind to the FVM, if it exists already.
- TryBind,
- // Reformat the partition, regardless of if it already exists as an FVM.
- Reformat,
-};
+} // namespace
// Formats the FVM within the provided partition if it is not already formatted.
//
@@ -487,6 +489,8 @@
return TryBindToFvmDriver(partition_fd, zx::sec(3));
}
+namespace {
+
// Formats a block device as a zxcrypt volume.
//
// On success, returns a file descriptor to an FVM.
@@ -1018,8 +1022,8 @@
memset(buffer.get(), 0, remaining_bytes);
status = payload_vmo.write(buffer.get(), payload_size, remaining_bytes);
if (status != ZX_OK) {
- ERROR("Failed to write padding to vmo\n");
- return status;
+ ERROR("Failed to write padding to vmo\n");
+ return status;
}
payload_size += remaining_bytes;
}
@@ -1084,7 +1088,7 @@
}
zx_status_t Paver::WriteAsset(fuchsia_paver_Configuration configuration, fuchsia_paver_Asset asset,
- const fuchsia_mem_Buffer& payload) {
+ const fuchsia_mem_Buffer& payload) {
if (!InitializePartitioner()) {
return ZX_ERR_BAD_STATE;
}
diff --git a/zircon/system/ulib/paver/test/fvm-test.cpp b/zircon/system/ulib/paver/test/fvm-test.cpp
new file mode 100644
index 0000000..debcaaf
--- /dev/null
+++ b/zircon/system/ulib/paver/test/fvm-test.cpp
@@ -0,0 +1,66 @@
+// Copyright 2019 The Fuchsia Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <lib/paver/paver.h>
+
+#include <fs-management/fvm.h>
+#include <zxtest/zxtest.h>
+
+#include "test/test-utils.h"
+
+namespace {
+
+constexpr size_t kSliceSize = kBlockSize * 2;
+constexpr uint8_t kFvmType[GPT_GUID_LEN] = GUID_FVM_VALUE;
+
+} // namespace
+
+class FvmTest : public zxtest::Test {
+public:
+ FvmTest() {
+ BlockDevice::Create(kFvmType, &device_);
+ ASSERT_TRUE(device_);
+ }
+
+ int borrow_fd() {
+ return device_->fd();
+ }
+
+ fbl::unique_fd fd() {
+ return fbl::unique_fd(dup(device_->fd()));
+ }
+
+private:
+ std::unique_ptr<BlockDevice> device_;
+};
+
+TEST_F(FvmTest, FormatFvmEmpty) {
+ fbl::unique_fd fvm_part = FvmPartitionFormat(fd(), kSliceSize, paver::BindOption::Reformat);
+ ASSERT_TRUE(fvm_part.is_valid());
+}
+
+TEST_F(FvmTest, TryBindEmpty) {
+ fbl::unique_fd fvm_part = FvmPartitionFormat(fd(), kSliceSize, paver::BindOption::TryBind);
+ ASSERT_TRUE(fvm_part.is_valid());
+}
+
+TEST_F(FvmTest, TryBindAlreadyFormatted) {
+ ASSERT_OK(fvm_init(borrow_fd(), kSliceSize));
+ fbl::unique_fd fvm_part = FvmPartitionFormat(fd(), kSliceSize, paver::BindOption::TryBind);
+ ASSERT_TRUE(fvm_part.is_valid());
+}
+
+TEST_F(FvmTest, TryBindAlreadyBound) {
+ fbl::unique_fd fvm_part = FvmPartitionFormat(fd(), kSliceSize, paver::BindOption::Reformat);
+ ASSERT_TRUE(fvm_part.is_valid());
+
+ fvm_part = FvmPartitionFormat(fd(), kSliceSize, paver::BindOption::TryBind);
+ ASSERT_TRUE(fvm_part.is_valid());
+}
+
+TEST_F(FvmTest, TryBindAlreadyFormattedWrongSliceSize) {
+ ASSERT_OK(fvm_init(borrow_fd(), kSliceSize * 2));
+ fbl::unique_fd fvm_part = FvmPartitionFormat(fd(), kSliceSize, paver::BindOption::TryBind);
+ ASSERT_TRUE(fvm_part.is_valid());
+}
diff --git a/zircon/system/ulib/paver/test/test-utils.h b/zircon/system/ulib/paver/test/test-utils.h
index 903782d..a38a7de 100644
--- a/zircon/system/ulib/paver/test/test-utils.h
+++ b/zircon/system/ulib/paver/test/test-utils.h
@@ -13,7 +13,7 @@
#include <zxtest/zxtest.h>
constexpr uint64_t kBlockSize = 0x1000;
-constexpr uint64_t kBlockCount = 0x10;
+constexpr uint64_t kBlockCount = 0x100;
constexpr uint32_t kOobSize = 8;
constexpr uint32_t kPageSize = 1024;
@@ -33,6 +33,9 @@
ramdisk_destroy(client_);
}
+ // Does not transfer ownership of the file descriptor.
+ int fd() { return ramdisk_get_block_fd(client_); }
+
private:
BlockDevice(ramdisk_client_t* client)
: client_(client) {}
