From cc047d6cf8be9bf453ff94acd83ae65483e0e67b Mon Sep 17 00:00:00 2001 From: "A. Cody Schuffelen" Date: Thu, 19 Mar 2026 00:46:28 +0000 Subject: [PATCH 1/3] Add WriteExact and WriteExactBinary to IO library #gemini Bug: b/494034973 --- base/cvd/cuttlefish/io/BUILD.bazel | 11 ++++++++ base/cvd/cuttlefish/io/write_exact.cc | 36 +++++++++++++++++++++++++++ base/cvd/cuttlefish/io/write_exact.h | 34 +++++++++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 base/cvd/cuttlefish/io/write_exact.cc create mode 100644 base/cvd/cuttlefish/io/write_exact.h diff --git a/base/cvd/cuttlefish/io/BUILD.bazel b/base/cvd/cuttlefish/io/BUILD.bazel index 1c0ac254964..dd18198a280 100644 --- a/base/cvd/cuttlefish/io/BUILD.bazel +++ b/base/cvd/cuttlefish/io/BUILD.bazel @@ -359,3 +359,14 @@ cf_cc_library( "//cuttlefish/result:result_type", ], ) + +cf_cc_library( + name = "write_exact", + srcs = ["write_exact.cc"], + hdrs = ["write_exact.h"], + deps = [ + "//cuttlefish/io", + "//cuttlefish/result:expect", + "//cuttlefish/result:result_type", + ], +) diff --git a/base/cvd/cuttlefish/io/write_exact.cc b/base/cvd/cuttlefish/io/write_exact.cc new file mode 100644 index 00000000000..3d7e62ceec9 --- /dev/null +++ b/base/cvd/cuttlefish/io/write_exact.cc @@ -0,0 +1,36 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "cuttlefish/io/write_exact.h" + +#include + +#include "cuttlefish/io/io.h" +#include "cuttlefish/result/expect.h" +#include "cuttlefish/result/result_type.h" + +namespace cuttlefish { + +Result WriteExact(Writer& writer, const char* buf, size_t size) { + while (size > 0) { + size_t data_written = CF_EXPECT(writer.Write((const void*)buf, size)); + CF_EXPECT_GT(data_written, 0, "Write returned 0 before completing"); + buf += data_written; + size -= data_written; + } + return {}; +} + +} // namespace cuttlefish diff --git a/base/cvd/cuttlefish/io/write_exact.h b/base/cvd/cuttlefish/io/write_exact.h new file mode 100644 index 00000000000..a1553a57b33 --- /dev/null +++ b/base/cvd/cuttlefish/io/write_exact.h @@ -0,0 +1,34 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include + +#include "cuttlefish/io/io.h" +#include "cuttlefish/result/expect.h" +#include "cuttlefish/result/result_type.h" + +namespace cuttlefish { + +Result WriteExact(Writer&, const char* buf, size_t size); + +template +Result WriteExactBinary(Writer& writer, const T& data) { + const char* const data_char = reinterpret_cast(&data); + return WriteExact(writer, data_char, sizeof(data)); +} + +} // namespace cuttlefish From 88195f14a364b8d286cab3097667714b7ded1625 Mon Sep 17 00:00:00 2001 From: "A. Cody Schuffelen" Date: Thu, 19 Mar 2026 00:46:41 +0000 Subject: [PATCH 2/3] Implement Lz4LegacyWriter for LZ4 Legacy frame compression This adds Lz4LegacyWriter, which provides support for creating LZ4 Legacy frames used by the Linux kernel. It handles initial magic numbers, block-based compression (up to 8MB), and frame termination. Frame termination is triggered when a write call of size less than or equal to the block size is made. Added unit tests to verify round-trip compression/decompression and frame closure behavior. #gemini Bug: b/494034973 Test: bazel test //cuttlefish/io:lz4_legacy_test --- base/cvd/cuttlefish/io/BUILD.bazel | 17 +++ base/cvd/cuttlefish/io/lz4_legacy.cc | 44 ++++++- base/cvd/cuttlefish/io/lz4_legacy.h | 11 ++ base/cvd/cuttlefish/io/lz4_legacy_test.cc | 137 ++++++++++++++++++++++ 4 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 base/cvd/cuttlefish/io/lz4_legacy_test.cc diff --git a/base/cvd/cuttlefish/io/BUILD.bazel b/base/cvd/cuttlefish/io/BUILD.bazel index dd18198a280..902d44cd2fa 100644 --- a/base/cvd/cuttlefish/io/BUILD.bazel +++ b/base/cvd/cuttlefish/io/BUILD.bazel @@ -255,12 +255,29 @@ cf_cc_library( deps = [ "//cuttlefish/io", "//cuttlefish/io:read_exact", + "//cuttlefish/io:write_exact", "//cuttlefish/result:expect", "//cuttlefish/result:result_type", "@lz4", ], ) +cf_cc_test( + name = "lz4_legacy_test", + srcs = ["lz4_legacy_test.cc"], + deps = [ + "//cuttlefish/io:filesystem", + "//cuttlefish/io:in_memory", + "//cuttlefish/io:lz4_legacy", + "//cuttlefish/io:read_exact", + "//cuttlefish/io:string", + "//cuttlefish/io:write_exact", + "//cuttlefish/result:result_matchers", + "@googletest//:gtest", + "@googletest//:gtest_main", + ], +) + cf_cc_library( name = "native_filesystem", srcs = ["native_filesystem.cc"], diff --git a/base/cvd/cuttlefish/io/lz4_legacy.cc b/base/cvd/cuttlefish/io/lz4_legacy.cc index 264561eba1c..4f5a2158b8b 100644 --- a/base/cvd/cuttlefish/io/lz4_legacy.cc +++ b/base/cvd/cuttlefish/io/lz4_legacy.cc @@ -18,13 +18,16 @@ #include #include +#include #include #include +#include #include "lz4.h" #include "cuttlefish/io/io.h" #include "cuttlefish/io/read_exact.h" +#include "cuttlefish/io/write_exact.h" #include "cuttlefish/result/expect.h" #include "cuttlefish/result/result_type.h" @@ -32,7 +35,6 @@ namespace cuttlefish { namespace { constexpr uint32_t kLz4LegacyFrameMagic = 0x184C2102; -constexpr uint32_t kLz4LegacyFrameBlockSize = 8 << 20; class Lz4LegacyReaderImpl : public Reader { public: @@ -77,6 +79,39 @@ class Lz4LegacyReaderImpl : public Reader { std::vector decompressed_; }; +class Lz4LegacyWriterImpl : public Writer { + public: + Lz4LegacyWriterImpl(std::unique_ptr sink) : sink_(std::move(sink)) {} + + Result Write(const void* buf, uint64_t count) override { + CF_EXPECT(!footer_written_, "Write called after LZ4 frame was closed"); + uint64_t to_write = std::min(count, kLz4LegacyFrameBlockSize); + + if (to_write > 0) { + compressed_.resize(LZ4_compressBound(to_write)); + int compressed_size = LZ4_compress_default( + reinterpret_cast(buf), compressed_.data(), to_write, + compressed_.size()); + CF_EXPECT_GT(compressed_size, 0, "LZ4 compression failed"); + + CF_EXPECT(WriteExactBinary(*sink_, htole32(compressed_size))); + CF_EXPECT(WriteExact(*sink_, compressed_.data(), compressed_size)); + } + + if (count <= kLz4LegacyFrameBlockSize) { + CF_EXPECT(WriteExactBinary(*sink_, 0)); + footer_written_ = true; + } + + return to_write; + } + + private: + std::unique_ptr sink_; + std::vector compressed_; + bool footer_written_ = false; +}; + } // namespace Result> Lz4LegacyReader( @@ -87,4 +122,11 @@ Result> Lz4LegacyReader( return std::make_unique(std::move(source)); } +Result> Lz4LegacyWriter(std::unique_ptr sink) { + CF_EXPECT(sink.get()); + const uint32_t magic_le = htole32(kLz4LegacyFrameMagic); + CF_EXPECT(WriteExactBinary(*sink, magic_le)); + return std::make_unique(std::move(sink)); +} + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/io/lz4_legacy.h b/base/cvd/cuttlefish/io/lz4_legacy.h index 0f9fe1fb1ee..4a9a7f8ff92 100644 --- a/base/cvd/cuttlefish/io/lz4_legacy.h +++ b/base/cvd/cuttlefish/io/lz4_legacy.h @@ -22,10 +22,21 @@ namespace cuttlefish { +constexpr uint32_t kLz4LegacyFrameBlockSize = 8 << 20; + // Handles the LZ4 Legacy frame format, used by the linux kernel. // // https://github.com/lz4/lz4/blob/5c4c1fb2354133e1f3b087a341576985f8114bd5/doc/lz4_Frame_format.md#legacy-frame Result> Lz4LegacyReader(std::unique_ptr); +// LZ4 Legacy frames are terminated by a 4-byte 0 length block. This +// implementation handles this by assuming that any write call with a size less +// than or equal to the block size (8 MB) is intended to be the last block in +// the frame. Subsequent writes will fail after the last block is written. +// +// Because of this, callers should prefer WriteExact with this writer instead +// of Copy, to avoid premature termination from small writes. +Result> Lz4LegacyWriter(std::unique_ptr); + } // namespace cuttlefish diff --git a/base/cvd/cuttlefish/io/lz4_legacy_test.cc b/base/cvd/cuttlefish/io/lz4_legacy_test.cc new file mode 100644 index 00000000000..7de706f53c0 --- /dev/null +++ b/base/cvd/cuttlefish/io/lz4_legacy_test.cc @@ -0,0 +1,137 @@ +// +// Copyright (C) 2026 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "cuttlefish/io/lz4_legacy.h" + +#include +#include +#include + +#include + +#include "cuttlefish/io/filesystem.h" +#include "cuttlefish/io/in_memory.h" +#include "cuttlefish/io/read_exact.h" +#include "cuttlefish/io/string.h" +#include "cuttlefish/io/write_exact.h" +#include "cuttlefish/result/result_matchers.h" + +namespace cuttlefish { +namespace { + +static constexpr std::string_view kTestFile = "filename.lz4"; + +TEST(Lz4LegacyTest, RoundTrip) { + std::string original_data = + "This is some test data to compress and decompress."; + static constexpr int kTimesToDuplicate = 10; + for (int i = 0; i < kTimesToDuplicate; ++i) { + original_data += original_data; + } + + std::unique_ptr fs = InMemoryFilesystem(); + + Result> writer_sink = + fs->CreateFile(kTestFile); + ASSERT_THAT(writer_sink, IsOk()); + Result> writer = + Lz4LegacyWriter(std::move(*writer_sink)); + ASSERT_THAT(writer, IsOk()); + + EXPECT_THAT((*writer)->Write(original_data.data(), original_data.size()), + IsOkAndValue(original_data.size())); + + Result> reader_source = + fs->OpenReadOnly(kTestFile); + ASSERT_THAT(reader_source, IsOk()); + Result> reader = + Lz4LegacyReader(std::move(*reader_source)); + ASSERT_THAT(reader, IsOk()); + + Result decompressed_data = ReadToString(**reader); + EXPECT_THAT(decompressed_data, IsOkAndValue(original_data)); +} + +TEST(Lz4LegacyTest, MultipleBlocks) { + static constexpr size_t kTotalSize = 10 << 20; + const std::string original_data(kTotalSize, 'a'); + + std::unique_ptr fs = InMemoryFilesystem(); + + Result> writer_sink = + fs->CreateFile(kTestFile); + ASSERT_THAT(writer_sink, IsOk()); + Result> writer = + Lz4LegacyWriter(std::move(*writer_sink)); + ASSERT_THAT(writer, IsOk()); + + EXPECT_THAT(WriteExact(**writer, original_data.data(), original_data.size()), + IsOk()); + + Result> reader_source = + fs->OpenReadOnly(kTestFile); + ASSERT_THAT(reader_source, IsOk()); + Result> reader = + Lz4LegacyReader(std::move(*reader_source)); + ASSERT_THAT(reader, IsOk()); + + Result decompressed_data = ReadToString(**reader); + EXPECT_THAT(decompressed_data, IsOkAndValue(original_data)); +} + +TEST(Lz4LegacyTest, RejectsWriteAfterFooter) { + Result> writer = Lz4LegacyWriter(InMemoryIo()); + ASSERT_THAT(writer, IsOk()); + + // A small write triggers the footer + std::string data = "some data"; + EXPECT_THAT((*writer)->Write(data.data(), data.size()), + IsOkAndValue(data.size())); + + // Future writes should fail + EXPECT_THAT((*writer)->Write(data.data(), data.size()), IsError()); +} + +TEST(Lz4LegacyTest, ExactlyBlockSizeTriggersFooter) { + std::unique_ptr fs = InMemoryFilesystem(); + + Result> writer_sink = + fs->CreateFile(kTestFile); + ASSERT_THAT(writer_sink, IsOk()); + Result> writer = + Lz4LegacyWriter(std::move(*writer_sink)); + ASSERT_THAT(writer, IsOk()); + + std::string data(kLz4LegacyFrameBlockSize, 'x'); + EXPECT_THAT((*writer)->Write(data.data(), data.size()), + IsOkAndValue(kLz4LegacyFrameBlockSize)); + + // It should be closed now; try adding a bit more. + EXPECT_THAT((*writer)->Write("more", 4), IsError()); + + // Verify it can be read back + Result> reader_source = + fs->OpenReadOnly(kTestFile); + ASSERT_THAT(reader_source, IsOk()); + Result> reader = + Lz4LegacyReader(std::move(*reader_source)); + ASSERT_THAT(reader, IsOk()); + + Result decompressed_data = ReadToString(**reader); + EXPECT_THAT(decompressed_data, IsOkAndValue(data)); +} + +} // namespace +} // namespace cuttlefish From d104b08af80f437f5325160e46c19e0fca15d90a Mon Sep 17 00:00:00 2001 From: "A. Cody Schuffelen" Date: Thu, 19 Mar 2026 00:47:16 +0000 Subject: [PATCH 3/3] Refactor RunLz4 in boot_image_utils to use internal Lz4LegacyWriter This replaces the external dependency on the lz4 host binary with the internal Lz4LegacyWriter, avoiding a subprocess call. #gemini Bug: b/494034973 Test: Create device with kernel replacement --- .../host/commands/assemble_cvd/BUILD.bazel | 2 ++ .../commands/assemble_cvd/boot_image_utils.cc | 25 ++++++++----------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/base/cvd/cuttlefish/host/commands/assemble_cvd/BUILD.bazel b/base/cvd/cuttlefish/host/commands/assemble_cvd/BUILD.bazel index 0e49395dda2..f6b36573004 100644 --- a/base/cvd/cuttlefish/host/commands/assemble_cvd/BUILD.bazel +++ b/base/cvd/cuttlefish/host/commands/assemble_cvd/BUILD.bazel @@ -129,6 +129,8 @@ cf_cc_library( "//cuttlefish/io:lz4_legacy", "//cuttlefish/io:native_filesystem", "//cuttlefish/io:shared_fd", + "//cuttlefish/io:string", + "//cuttlefish/io:write_exact", "//cuttlefish/result", "//libbase", "@abseil-cpp//absl/log", diff --git a/base/cvd/cuttlefish/host/commands/assemble_cvd/boot_image_utils.cc b/base/cvd/cuttlefish/host/commands/assemble_cvd/boot_image_utils.cc index 102953f6533..e3fe7e00fe1 100644 --- a/base/cvd/cuttlefish/host/commands/assemble_cvd/boot_image_utils.cc +++ b/base/cvd/cuttlefish/host/commands/assemble_cvd/boot_image_utils.cc @@ -48,6 +48,8 @@ #include "cuttlefish/io/lz4_legacy.h" #include "cuttlefish/io/native_filesystem.h" #include "cuttlefish/io/shared_fd.h" +#include "cuttlefish/io/string.h" +#include "cuttlefish/io/write_exact.h" #include "cuttlefish/result/result.h" namespace cuttlefish { @@ -75,21 +77,14 @@ Result RunMkBootFs(const std::string& input_dir, } Result RunLz4(const std::string& input, const std::string& output) { - SharedFD output_fd = SharedFD::Open(output, O_CREAT | O_RDWR | O_TRUNC, 0644); - CF_EXPECTF(output_fd->IsOpen(), "Failed to open '{}': '{}'", output, - output_fd->StrError()); - int success = Command(HostBinaryPath("lz4")) - .AddParameter("-c") - .AddParameter("-l") - .AddParameter("-12") - .AddParameter("--favor-decSpeed") - .AddParameter(input) - .RedirectStdIO(Subprocess::StdIOChannel::kStdOut, output_fd) - .Start() - .Wait(); - CF_EXPECT_EQ( - success, 0, - "`lz4` failed to transform '" << input << "' to '" << output << "'"); + NativeFilesystem fs; + std::unique_ptr input_reader = CF_EXPECT(fs.OpenReadOnly(input)); + (void)fs.DeleteFile(output); + std::unique_ptr output_writer = CF_EXPECT(fs.CreateFile(output)); + std::unique_ptr lz4_writer = + CF_EXPECT(Lz4LegacyWriter(std::move(output_writer))); + std::string input_data = CF_EXPECT(ReadToString(*input_reader)); + CF_EXPECT(WriteExact(*lz4_writer, input_data.data(), input_data.size())); return {}; }