Skip to content

Commit 1f60a7d

Browse files
committed
Add a new flag --extract_super_image_fragments to cvd fetch.
This enables a new flow to download the super image components out of the target_files zip, using a path that only downloads the fragments of the zip necessary. Downloading the super image fragments is comparable to downloading the image zip, but these files can be used to build a super image ourselves with any last-minute modifications we need. Some performance and correctness improvements in the partial zip download are included in this change: increasing the chunk size for partial downloads, and working around libzip doing read-all behavior from its read calls. The partial zip download takes twice the time of the full image zip download, so hopefully more performance improvements are possible afterward. This change also supersedes #1558 . Bug: b/439877215
1 parent d706c54 commit 1f60a7d

File tree

14 files changed

+142
-20
lines changed

14 files changed

+142
-20
lines changed

base/cvd/cuttlefish/host/commands/cvd/fetch/BUILD.bazel

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,16 @@ cf_cc_library(
138138
"//cuttlefish/host/libs/web:android_build",
139139
"//cuttlefish/host/libs/web:android_build_api",
140140
"//cuttlefish/host/libs/web:android_build_string",
141+
"//cuttlefish/host/libs/web:build_api_zip",
141142
"//cuttlefish/host/libs/web:build_zip_name",
142143
"//cuttlefish/host/libs/web:chrome_os_build_string",
143144
"//cuttlefish/host/libs/web:luci_build_api",
144145
"//cuttlefish/host/libs/web/http_client:curl_global_init",
146+
"//cuttlefish/host/libs/zip:zip_cc",
147+
"//cuttlefish/host/libs/zip:zip_file",
148+
"//cuttlefish/host/libs/zip:zip_string",
145149
"//libbase",
150+
"@abseil-cpp//absl/strings",
146151
],
147152
)
148153

base/cvd/cuttlefish/host/commands/cvd/fetch/download_flags.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ DownloadFlags DownloadFlags::Create(const VectorFlags& flags, const int index) {
2727
.download_target_files_zip =
2828
GetOptional(flags.download_target_files_zip, index)
2929
.value_or(kDefaultDownloadTargetFilesZip),
30+
.extract_super_image_fragments =
31+
GetOptional(flags.extract_super_image_fragments, index)
32+
.value_or(kDefaultExtractSuperImageFragments),
3033
};
3134
}
3235

base/cvd/cuttlefish/host/commands/cvd/fetch/download_flags.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ struct DownloadFlags {
2424

2525
bool download_img_zip;
2626
bool download_target_files_zip;
27+
bool extract_super_image_fragments;
2728
};
2829

2930
} // namespace cuttlefish

base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <vector>
2929

3030
#include <android-base/strings.h>
31+
#include "absl/strings/str_split.h"
3132

3233
#include "cuttlefish/common/libs/utils/archive.h"
3334
#include "cuttlefish/common/libs/utils/contains.h"
@@ -48,10 +49,14 @@
4849
#include "cuttlefish/host/libs/web/android_build.h"
4950
#include "cuttlefish/host/libs/web/android_build_api.h"
5051
#include "cuttlefish/host/libs/web/android_build_string.h"
52+
#include "cuttlefish/host/libs/web/build_api_zip.h"
5153
#include "cuttlefish/host/libs/web/build_zip_name.h"
5254
#include "cuttlefish/host/libs/web/chrome_os_build_string.h"
5355
#include "cuttlefish/host/libs/web/http_client/curl_global_init.h"
5456
#include "cuttlefish/host/libs/web/luci_build_api.h"
57+
#include "cuttlefish/host/libs/zip/zip_cc.h"
58+
#include "cuttlefish/host/libs/zip/zip_file.h"
59+
#include "cuttlefish/host/libs/zip/zip_string.h"
5560

5661
namespace cuttlefish {
5762
namespace {
@@ -237,19 +242,58 @@ Result<void> FetchDefaultTarget(BuildApi& build_api, const Builds& builds,
237242
trace.CompletePhase("Desparse image files");
238243
}
239244

245+
std::string target_files_name =
246+
GetBuildZipName(*builds.default_build, "target_files");
247+
std::optional<std::string> target_files;
240248
if (builds.system || flags.download_target_files_zip) {
241249
LOG(INFO) << "Downloading target files zip for " << *builds.default_build;
242-
std::string target_files_name =
243-
GetBuildZipName(*builds.default_build, "target_files");
244-
std::string target_files = CF_EXPECT(build_api.DownloadFile(
250+
target_files = CF_EXPECT(build_api.DownloadFile(
245251
*builds.default_build, target_directories.default_target_files,
246252
target_files_name));
247253
trace.CompletePhase("Download Target Files");
248254
LOG(INFO) << "Adding target files for default build";
249-
CF_EXPECT(config.AddFilesToConfig(FileSource::DEFAULT_BUILD,
250-
default_build_id, default_build_target,
251-
{target_files}, target_directories.root));
255+
CF_EXPECT(config.AddFilesToConfig(
256+
FileSource::DEFAULT_BUILD, default_build_id, default_build_target,
257+
{*target_files}, target_directories.root));
258+
}
259+
260+
if (flags.extract_super_image_fragments) {
261+
ReadableZip target_files_zip =
262+
target_files.has_value()
263+
? CF_EXPECT(ZipOpenRead(*target_files))
264+
: CF_EXPECT(
265+
OpenZip(build_api, *builds.default_build, target_files_name));
266+
ReadableZipSource ab_partitions_source =
267+
CF_EXPECT(target_files_zip.GetFile("META/ab_partitions.txt"));
268+
std::string ab_partitions_contents =
269+
CF_EXPECT(ReadToString(ab_partitions_source));
270+
271+
std::string ab_partitions_file =
272+
target_directories.default_target_files + "/ab_partitions.txt";
273+
CF_EXPECT(android::base::WriteStringToFile(ab_partitions_contents,
274+
ab_partitions_file));
275+
CF_EXPECT(config.AddFilesToConfig(
276+
FileSource::DEFAULT_BUILD, default_build_id, default_build_target,
277+
{ab_partitions_file}, target_directories.default_target_files));
278+
279+
std::vector<std::string> ab_files =
280+
absl::StrSplit(ab_partitions_contents, '\n');
281+
ab_files.emplace_back("super_empty");
282+
for (const std::string& ab_file : ab_files) {
283+
if (ab_file.empty()) {
284+
continue;
285+
}
286+
std::string output = fmt::format(
287+
"{}/{}.img", target_directories.default_target_files, ab_file);
288+
CF_EXPECTF(ExtractFile(target_files_zip,
289+
fmt::format("IMAGES/{}.img", ab_file), output),
290+
"Failed to extract {}", output);
291+
CF_EXPECT(config.AddFilesToConfig(
292+
FileSource::DEFAULT_BUILD, default_build_id, default_build_target,
293+
{output}, target_directories.default_target_files));
294+
}
252295
}
296+
253297
return {};
254298
}
255299

base/cvd/cuttlefish/host/commands/cvd/fetch/vector_flags.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ std::vector<Flag> VectorFlags::Flags() {
6868
this->download_target_files_zip,
6969
kDefaultDownloadTargetFilesZip)
7070
.Help("Whether to fetch the -target_files-*.zip file."));
71+
flags.emplace_back(
72+
GflagsCompatFlag("extract_super_image_fragments",
73+
this->extract_super_image_fragments,
74+
kDefaultExtractSuperImageFragments)
75+
.Help("Whether to fetch the -target_files-*.zip file."));
7176

7277
return flags;
7378
}

base/cvd/cuttlefish/host/commands/cvd/fetch/vector_flags.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ namespace cuttlefish {
2727

2828
inline constexpr bool kDefaultDownloadImgZip = true;
2929
inline constexpr bool kDefaultDownloadTargetFilesZip = false;
30+
// TODO: schuffelen - Enable this by default.
31+
inline constexpr bool kDefaultExtractSuperImageFragments = false;
3032

3133
struct VectorFlags {
3234
std::vector<Flag> Flags();
@@ -44,6 +46,7 @@ struct VectorFlags {
4446
std::vector<bool> download_img_zip;
4547
std::vector<bool> download_target_files_zip;
4648
std::vector<std::string> boot_artifact;
49+
std::vector<bool> extract_super_image_fragments;
4750
};
4851

4952
} // namespace cuttlefish

base/cvd/cuttlefish/host/libs/web/build_api_zip.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ Result<ReadableZip> OpenZip(BuildApi& build_api, const Build& build,
3131
SeekableZipSource source = CF_EXPECT(build_api.FileReader(build, name));
3232

3333
SeekableZipSource buffered =
34-
CF_EXPECT(BufferZipSource(std::move(source), 1 << 16));
34+
CF_EXPECT(BufferZipSource(std::move(source), 1 << 26));
3535

3636
return CF_EXPECT(ReadableZip::FromSource(std::move(buffered)));
3737
}

base/cvd/cuttlefish/host/libs/zip/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ cf_cc_library(
1313
deps = [
1414
"//cuttlefish/common/libs/utils:result",
1515
"//cuttlefish/host/libs/zip:zip_cc",
16+
"//libbase",
1617
],
1718
)
1819

@@ -107,6 +108,7 @@ cf_cc_library(
107108
"//cuttlefish/common/libs/utils:result",
108109
"//cuttlefish/host/libs/web/http_client",
109110
"//cuttlefish/host/libs/zip:zip_cc",
111+
"//libbase",
110112
"@abseil-cpp//absl/strings",
111113
"@fmt",
112114
],

base/cvd/cuttlefish/host/libs/zip/buffered_zip_source.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#include <utility>
2626
#include <vector>
2727

28+
#include "android-base/logging.h"
29+
2830
#include "cuttlefish/common/libs/utils/result.h"
2931
#include "cuttlefish/host/libs/zip/zip_cc.h"
3032

@@ -71,6 +73,7 @@ class BufferedZipSourceCallbacks : public SeekableZipSourceCallback {
7173
if (!reader_->SeekFromStart(offset_).ok()) {
7274
return false;
7375
}
76+
LOG(VERBOSE) << "Bypassing buffer, reading " << len;
7477
Result<uint64_t> data_read = reader_->Read(data, len);
7578
if (data_read.ok()) {
7679
offset_ += *data_read;
@@ -97,6 +100,7 @@ class BufferedZipSourceCallbacks : public SeekableZipSourceCallback {
97100
if (!reader_->SeekFromStart(offset_).ok()) {
98101
return -1;
99102
}
103+
LOG(VERBOSE) << "Filling buffer with " << buffer_fill;
100104
Result<size_t> inner_read = reader_->Read(buffer_.data(), buffer_fill);
101105
if (!inner_read.ok()) {
102106
return -1;
@@ -106,9 +110,14 @@ class BufferedZipSourceCallbacks : public SeekableZipSourceCallback {
106110
return Read(data, len);
107111
}
108112
uint64_t Size() override { return size_; }
109-
bool SetOffset(int64_t offset) override {
110-
offset_ = offset;
111-
buffer_remaining_ = 0;
113+
bool SetOffset(int64_t new_offset) override {
114+
if (new_offset >= offset_ && new_offset < offset_ + buffer_remaining_) {
115+
offset_in_buffer_ += new_offset - offset_;
116+
buffer_remaining_ -= new_offset - offset_;
117+
} else {
118+
buffer_remaining_ = 0;
119+
}
120+
offset_ = new_offset;
112121
return true;
113122
}
114123
int64_t Offset() override { return offset_; }

base/cvd/cuttlefish/host/libs/zip/cached_zip_source.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class CachedZipSourceCallbacks : public SeekableZipSourceCallback {
4747
}
4848

4949
int64_t Read(char* data, uint64_t len) override {
50+
LOG(VERBOSE) << "Reading " << len;
5051
if (Result<void> res = source_.Seek(offset_); !res.ok()) {
5152
LOG(ERROR) << res.error().FormatForEnv();
5253
return -1;
@@ -64,6 +65,7 @@ class CachedZipSourceCallbacks : public SeekableZipSourceCallback {
6465
uint64_t Size() override { return size_; }
6566

6667
bool SetOffset(int64_t offset) override {
68+
LOG(VERBOSE) << "Setting offset to " << offset;
6769
offset_ = offset;
6870
return true;
6971
}
@@ -113,7 +115,7 @@ Result<SeekableZipSource> CacheZipSource(SeekableZipSource inner,
113115
std::move(reader));
114116

115117
LazilyLoadedFile file = CF_EXPECT(LazilyLoadedFile::Create(
116-
std::move(file_path), std::move(file_callbacks)));
118+
std::move(file_path), size, std::move(file_callbacks)));
117119

118120
CachedZipSourceCallbacks callbacks(std::move(file), size);
119121

0 commit comments

Comments
 (0)