Skip to content

Commit 06f9d77

Browse files
DatabeanDatabean
committed
Create a KernelPathFlag type to resolve ordering dependencies
https://github.com/google/android-cuttlefish/blob/403b3d6b44e149fe5156dc0150b6a801560bc9fe/base/cvd/cuttlefish/host/commands/assemble_cvd/assemble_cvd.cc#L516 https://github.com/google/android-cuttlefish/blob/403b3d6b44e149fe5156dc0150b6a801560bc9fe/base/cvd/cuttlefish/host/commands/assemble_cvd/disk_flags.cc#L74 https://github.com/google/android-cuttlefish/blob/403b3d6b44e149fe5156dc0150b6a801560bc9fe/base/cvd/cuttlefish/host/commands/assemble_cvd/disk_flags.cc#L436 `ExtractKernelParamsFromFetcherConfig` needs to be called before `FLAGS_kernel_path` can be accessed. This is unintuitive. With a custom type, access to the value can be gated behind correct initialization. Bug: b/430149667 Test: Fetch with `--kernel_build` Test: Launch with `--kernel_path` and `--initramfs_path`
1 parent 03f9157 commit 06f9d77

11 files changed

Lines changed: 163 additions & 45 deletions

File tree

‎base/cvd/cuttlefish/host/commands/assemble_cvd/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ cf_cc_binary(
3636
"//cuttlefish/host/commands/assemble_cvd/disk:factory_reset_protected",
3737
"//cuttlefish/host/commands/assemble_cvd/disk:metadata_image",
3838
"//cuttlefish/host/commands/assemble_cvd/disk:misc_image",
39+
"//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
3940
"//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
4041
"//cuttlefish/host/libs/command_util",
4142
"//cuttlefish/host/libs/config:ap_boot_flow",
@@ -160,6 +161,7 @@ cf_cc_library(
160161
"//cuttlefish/host/commands/assemble_cvd/disk:pstore",
161162
"//cuttlefish/host/commands/assemble_cvd/disk:sd_card",
162163
"//cuttlefish/host/commands/assemble_cvd/disk:vbmeta_enforce_minimum_size",
164+
"//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
163165
"//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
164166
"//cuttlefish/host/libs/avb",
165167
"//cuttlefish/host/libs/config:ap_boot_flow",
@@ -232,6 +234,7 @@ cf_cc_library(
232234
"//cuttlefish/host/commands/assemble_cvd:network_flags",
233235
"//cuttlefish/host/commands/assemble_cvd:touchpad",
234236
"//cuttlefish/host/commands/assemble_cvd/flags:display_proto",
237+
"//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
235238
"//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
236239
"//cuttlefish/host/libs/config:ap_boot_flow",
237240
"//cuttlefish/host/libs/config:config_constants",

‎base/cvd/cuttlefish/host/commands/assemble_cvd/assemble_cvd.cc‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "cuttlefish/host/commands/assemble_cvd/display.h"
4141
#include "cuttlefish/host/commands/assemble_cvd/flag_feature.h"
4242
#include "cuttlefish/host/commands/assemble_cvd/flags.h"
43+
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
4344
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
4445
#include "cuttlefish/host/commands/assemble_cvd/flags_defaults.h"
4546
#include "cuttlefish/host/commands/assemble_cvd/touchpad.h"
@@ -309,6 +310,7 @@ Result<SharedFD> SetLogger(std::string runtime_dir_parent) {
309310
Result<const CuttlefishConfig*> InitFilesystemAndCreateConfig(
310311
FetcherConfig fetcher_config, const std::vector<GuestConfig>& guest_configs,
311312
fruit::Injector<>& injector, SharedFD log,
313+
const KernelPathFlag& kernel_path,
312314
const SystemImageDirFlag& system_image_dir) {
313315
{
314316
// The config object is created here, but only exists in memory until the
@@ -317,7 +319,7 @@ Result<const CuttlefishConfig*> InitFilesystemAndCreateConfig(
317319
// disk.
318320
auto config = CF_EXPECT(InitializeCuttlefishConfiguration(
319321
FLAGS_instance_dir, guest_configs, injector,
320-
fetcher_config, system_image_dir),
322+
fetcher_config, kernel_path, system_image_dir),
321323
"cuttlefish configuration initialization failed");
322324

323325
const std::string snapshot_path = FLAGS_snapshot_path;
@@ -508,14 +510,9 @@ const std::string kKernelDefaultPath = "kernel";
508510
const std::string kInitramfsImg = "initramfs.img";
509511
static void ExtractKernelParamsFromFetcherConfig(
510512
const FetcherConfig& fetcher_config) {
511-
std::string discovered_kernel =
512-
fetcher_config.FindCvdFileWithSuffix(kKernelDefaultPath);
513513
std::string discovered_ramdisk =
514514
fetcher_config.FindCvdFileWithSuffix(kInitramfsImg);
515515

516-
SetCommandLineOptionWithMode("kernel_path", discovered_kernel.c_str(),
517-
google::FlagSettingMode::SET_FLAGS_DEFAULT);
518-
519516
SetCommandLineOptionWithMode("initramfs_path", discovered_ramdisk.c_str(),
520517
google::FlagSettingMode::SET_FLAGS_DEFAULT);
521518
}
@@ -618,6 +615,8 @@ Result<int> AssembleCvdMain(int argc, char** argv) {
618615
/* remove_flags */ false);
619616
}
620617

618+
KernelPathFlag kernel_path = KernelPathFlag::FromGlobalGflags(fetcher_config);
619+
621620
SystemImageDirFlag system_image_dir =
622621
CF_EXPECT(SystemImageDirFlag::FromGlobalGflags());
623622

@@ -649,13 +648,14 @@ Result<int> AssembleCvdMain(int argc, char** argv) {
649648
// gflags either consumes all arguments that start with - or leaves all of
650649
// them in place, and either errors out on unknown flags or accepts any flags.
651650

652-
auto guest_configs = CF_EXPECT(GetGuestConfigAndSetDefaults(system_image_dir),
653-
"Failed to parse arguments");
651+
auto guest_configs =
652+
CF_EXPECT(GetGuestConfigAndSetDefaults(kernel_path, system_image_dir),
653+
"Failed to parse arguments");
654654

655-
auto config = CF_EXPECT(
656-
InitFilesystemAndCreateConfig(std::move(fetcher_config), guest_configs,
657-
injector, log, system_image_dir),
658-
"Failed to create config");
655+
auto config = CF_EXPECT(InitFilesystemAndCreateConfig(
656+
std::move(fetcher_config), guest_configs,
657+
injector, log, kernel_path, system_image_dir),
658+
"Failed to create config");
659659

660660
std::cout << GetConfigFilePath(*config) << "\n";
661661
std::cout << std::flush;

‎base/cvd/cuttlefish/host/commands/assemble_cvd/assemble_cvd_flags.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@ DEFINE_string(refresh_rate_hz, "60", "Screen refresh rate in Hertz");
5858
DEFINE_string(overlays, "",
5959
"List of displays to overlay. Format is: 'vm_index:display_index "
6060
"vm_index2:display_index2 [...]'");
61-
DEFINE_vec(kernel_path, CF_DEFAULTS_KERNEL_PATH,
62-
"Path to the kernel. Overrides the one from the boot image");
6361
DEFINE_vec(initramfs_path, CF_DEFAULTS_INITRAMFS_PATH,
6462
"Path to the initramfs");
6563
DEFINE_string(extra_kernel_cmdline, CF_DEFAULTS_EXTRA_KERNEL_CMDLINE,

‎base/cvd/cuttlefish/host/commands/assemble_cvd/assemble_cvd_flags.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ DECLARE_string(y_res);
3939
DECLARE_string(dpi);
4040
DECLARE_string(refresh_rate_hz);
4141
DECLARE_string(overlays);
42-
DECLARE_vec(kernel_path);
4342
DECLARE_vec(initramfs_path);
4443
DECLARE_string(extra_kernel_cmdline);
4544
DECLARE_string(extra_bootconfig_args);

‎base/cvd/cuttlefish/host/commands/assemble_cvd/disk_flags.cc‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
#include "cuttlefish/host/commands/assemble_cvd/disk/sd_card.h"
5353
#include "cuttlefish/host/commands/assemble_cvd/disk/vbmeta_enforce_minimum_size.h"
5454
#include "cuttlefish/host/commands/assemble_cvd/disk_builder.h"
55+
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
5556
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
5657
#include "cuttlefish/host/commands/assemble_cvd/super_image_mixer.h"
5758
#include "cuttlefish/host/libs/avb/avb.h"
@@ -68,11 +69,12 @@ namespace cuttlefish {
6869

6970
using vm_manager::Gem5Manager;
7071

71-
Result<void> ResolveInstanceFiles(const SystemImageDirFlag& system_image_dir) {
72+
Result<void> ResolveInstanceFiles(const KernelPathFlag& kernel_path,
73+
const SystemImageDirFlag& system_image_dir) {
7274
// It is conflict (invalid) to pass both kernel_path/initramfs_path
7375
// and image file paths.
74-
bool flags_kernel_initramfs_has_input = (!FLAGS_kernel_path.empty())
75-
|| (!FLAGS_initramfs_path.empty());
76+
bool flags_kernel_initramfs_has_input =
77+
(!kernel_path.HasValue()) || (!FLAGS_initramfs_path.empty());
7678
bool flags_image_has_input =
7779
(!FLAGS_super_image.empty()) || (!FLAGS_vendor_boot_image.empty()) ||
7880
(!FLAGS_vbmeta_vendor_dlkm_image.empty()) ||
@@ -245,6 +247,7 @@ static fruit::Component<> DiskChangesPerInstanceComponent(
245247

246248
Result<void> DiskImageFlagsVectorization(
247249
CuttlefishConfig& config, const FetcherConfig& fetcher_config,
250+
const KernelPathFlag& kernel_path,
248251
const SystemImageDirFlag& system_image_dir) {
249252
std::vector<std::string> boot_image =
250253
android::base::Split(FLAGS_boot_image, ",");
@@ -300,13 +303,10 @@ Result<void> DiskImageFlagsVectorization(
300303
android::base::Split(FLAGS_bootloader, ",");
301304
std::vector<std::string> initramfs_path =
302305
android::base::Split(FLAGS_initramfs_path, ",");
303-
std::vector<std::string> kernel_path =
304-
android::base::Split(FLAGS_kernel_path, ",");
305306

306307
std::vector<std::string> blank_sdcard_image_mb =
307308
android::base::Split(FLAGS_blank_sdcard_image_mb, ",");
308309

309-
std::string cur_kernel_path;
310310
std::string cur_initramfs_path;
311311
std::string cur_boot_image;
312312
std::string cur_vendor_boot_image;
@@ -433,12 +433,7 @@ Result<void> DiskImageFlagsVectorization(
433433
} else {
434434
instance.set_bootloader(bootloader[instance_index]);
435435
}
436-
if (instance_index >= kernel_path.size()) {
437-
cur_kernel_path = kernel_path[0];
438-
} else {
439-
cur_kernel_path = kernel_path[instance_index];
440-
}
441-
instance.set_kernel_path(cur_kernel_path);
436+
instance.set_kernel_path(kernel_path.KernelPathForIndex(instance_index));
442437
if (instance_index >= initramfs_path.size()) {
443438
cur_initramfs_path = initramfs_path[0];
444439
} else {
@@ -460,7 +455,8 @@ Result<void> DiskImageFlagsVectorization(
460455
// Repacking a boot.img changes boot_image and vendor_boot_image paths
461456
const CuttlefishConfig& const_config = const_cast<const CuttlefishConfig&>(config);
462457
const CuttlefishConfig::InstanceSpecific const_instance = const_config.ForInstance(num);
463-
if (!cur_kernel_path.empty() && config.vm_manager() != VmmMode::kGem5) {
458+
if (!kernel_path.KernelPathForIndex(instance_index).empty() &&
459+
config.vm_manager() != VmmMode::kGem5) {
464460
const std::string new_boot_image_path =
465461
const_instance.PerInstancePath("boot_repacked.img");
466462
// change the new flag value to corresponding instance
@@ -471,7 +467,8 @@ Result<void> DiskImageFlagsVectorization(
471467
"/userdata.img");
472468
instance.set_new_data_image(const_instance.PerInstancePath("userdata.img"));
473469

474-
if (!cur_kernel_path.empty() || !cur_initramfs_path.empty()) {
470+
if (!kernel_path.KernelPathForIndex(instance_index).empty() ||
471+
!cur_initramfs_path.empty()) {
475472
const std::string new_vendor_boot_image_path =
476473
const_instance.PerInstancePath("vendor_boot_repacked.img");
477474
// Repack the vendor boot images if kernels and/or ramdisks are passed in.

‎base/cvd/cuttlefish/host/commands/assemble_cvd/disk_flags.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,22 @@
2020
#include "cuttlefish/host/commands/assemble_cvd/disk/metadata_image.h"
2121
#include "cuttlefish/host/commands/assemble_cvd/disk/misc_image.h"
2222
#include "cuttlefish/host/commands/assemble_cvd/disk_builder.h"
23+
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
2324
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
2425
#include "cuttlefish/host/libs/config/cuttlefish_config.h"
2526
#include "cuttlefish/host/libs/config/fetcher_config.h"
2627

2728
namespace cuttlefish {
2829

29-
Result<void> ResolveInstanceFiles(const SystemImageDirFlag& system_image_dir);
30+
Result<void> ResolveInstanceFiles(const KernelPathFlag& kernel_path,
31+
const SystemImageDirFlag& system_image_dir);
3032

3133
Result<void> CreateDynamicDiskFiles(const FetcherConfig& fetcher_config,
3234
const CuttlefishConfig& config,
3335
const SystemImageDirFlag&);
3436
Result<void> DiskImageFlagsVectorization(CuttlefishConfig& config,
3537
const FetcherConfig& fetcher_config,
38+
const KernelPathFlag&,
3639
const SystemImageDirFlag&);
3740
DiskBuilder OsCompositeDiskBuilder(
3841
const CuttlefishConfig& config,

‎base/cvd/cuttlefish/host/commands/assemble_cvd/flags.cc‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
#include "cuttlefish/host/commands/assemble_cvd/disk_flags.h"
5454
#include "cuttlefish/host/commands/assemble_cvd/display.h"
5555
#include "cuttlefish/host/commands/assemble_cvd/flags/display_proto.h"
56+
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
5657
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
5758
#include "cuttlefish/host/commands/assemble_cvd/graphics_flags.h"
5859
#include "cuttlefish/host/commands/assemble_cvd/guest_config.h"
@@ -115,7 +116,8 @@ Result<std::string> GetAndroidInfoConfig(
115116
}
116117

117118
#ifdef __ANDROID__
118-
Result<std::vector<GuestConfig>> ReadGuestConfig(const SystemImageDir&) {
119+
Result<std::vector<GuestConfig>> ReadGuestConfig(const KernelPathFlag&,
120+
const SystemImageDir&) {
119121
std::vector<GuestConfig> rets;
120122
auto instance_nums =
121123
CF_EXPECT(InstanceNumsCalculator().FromGlobalGflags().Calculate());
@@ -131,15 +133,13 @@ Result<std::vector<GuestConfig>> ReadGuestConfig(const SystemImageDir&) {
131133
}
132134
#else
133135
Result<std::vector<GuestConfig>> ReadGuestConfig(
136+
const KernelPathFlag& kernel_path,
134137
const SystemImageDirFlag& system_image_dir) {
135138
std::vector<GuestConfig> guest_configs;
136139
std::vector<std::string> boot_image =
137140
android::base::Split(FLAGS_boot_image, ",");
138-
std::vector<std::string> kernel_path =
139-
android::base::Split(FLAGS_kernel_path, ",");
140141
std::string kernel_image_path = "";
141142
std::string cur_boot_image;
142-
std::string cur_kernel_path;
143143

144144
std::string current_path = StringFromEnv("PATH", "");
145145
std::string bin_folder = DefaultHostArtifactsPath("bin");
@@ -154,18 +154,14 @@ Result<std::vector<GuestConfig>> ReadGuestConfig(
154154
// for the ikconfig header in the image before extracting the config list.
155155
// This code is liable to break if the boot image ever includes the
156156
// ikconfig header outside the kernel.
157-
cur_kernel_path = "";
158-
if (instance_index < kernel_path.size()) {
159-
cur_kernel_path = kernel_path[instance_index];
160-
}
161157

162158
cur_boot_image = "";
163159
if (instance_index < boot_image.size()) {
164160
cur_boot_image = boot_image[instance_index];
165161
}
166162

167-
if (!cur_kernel_path.empty()) {
168-
kernel_image_path = cur_kernel_path;
163+
if (!kernel_path.KernelPathForIndex(instance_index).empty()) {
164+
kernel_image_path = kernel_path.KernelPathForIndex(instance_index);
169165
} else if (!cur_boot_image.empty()) {
170166
kernel_image_path = cur_boot_image;
171167
}
@@ -597,6 +593,7 @@ std::string DefaultBootloaderArchDir(Arch arch) {
597593
Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
598594
const std::string& root_dir, const std::vector<GuestConfig>& guest_configs,
599595
fruit::Injector<>& injector, const FetcherConfig& fetcher_config,
596+
const KernelPathFlag& kernel_path,
600597
const SystemImageDirFlag& system_image_dir) {
601598
CuttlefishConfig tmp_config_obj;
602599
// If a snapshot path is provided, do not read all flags to set up the config.
@@ -1605,7 +1602,7 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
16051602
"The set of flags is incompatible with snapshot");
16061603

16071604
CF_EXPECT(DiskImageFlagsVectorization(tmp_config_obj, fetcher_config,
1608-
system_image_dir));
1605+
kernel_path, system_image_dir));
16091606

16101607
return tmp_config_obj;
16111608
}
@@ -1804,16 +1801,17 @@ void SetDefaultFlagsForOpenwrt(Arch target_arch) {
18041801
}
18051802

18061803
Result<std::vector<GuestConfig>> GetGuestConfigAndSetDefaults(
1804+
const KernelPathFlag& kernel_path,
18071805
const SystemImageDirFlag& system_image_dir) {
18081806
auto instance_nums =
18091807
CF_EXPECT(InstanceNumsCalculator().FromGlobalGflags().Calculate());
18101808
int32_t instances_size = instance_nums.size();
18111809

1812-
CF_EXPECT(ResolveInstanceFiles(system_image_dir),
1810+
CF_EXPECT(ResolveInstanceFiles(kernel_path, system_image_dir),
18131811
"Failed to resolve instance files");
18141812

18151813
std::vector<GuestConfig> guest_configs =
1816-
CF_EXPECT(ReadGuestConfig(system_image_dir));
1814+
CF_EXPECT(ReadGuestConfig(kernel_path, system_image_dir));
18171815

18181816
// TODO(weihsu), b/250988697:
18191817
// assume all instances are using same VM manager/app/arch,

‎base/cvd/cuttlefish/host/commands/assemble_cvd/flags.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <fruit/fruit.h>
2121

2222
#include "cuttlefish/common/libs/utils/result.h"
23+
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
2324
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
2425
#include "cuttlefish/host/commands/assemble_cvd/guest_config.h"
2526
#include "cuttlefish/host/libs/config/cuttlefish_config.h"
@@ -28,12 +29,12 @@
2829
namespace cuttlefish {
2930

3031
Result<std::vector<GuestConfig>> GetGuestConfigAndSetDefaults(
31-
const SystemImageDirFlag&);
32+
const KernelPathFlag&, const SystemImageDirFlag&);
3233
// Must be called after ParseCommandLineFlags.
3334
Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
3435
const std::string& root_dir, const std::vector<GuestConfig>& guest_configs,
3536
fruit::Injector<>& injector, const FetcherConfig& fetcher_config,
36-
const SystemImageDirFlag&);
37+
const KernelPathFlag& kernel_path, const SystemImageDirFlag&);
3738

3839
std::string GetConfigFilePath(const CuttlefishConfig& config);
3940

‎base/cvd/cuttlefish/host/commands/assemble_cvd/flags/BUILD.bazel‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,18 @@ cf_cc_library(
3434
],
3535
)
3636

37+
cf_cc_library(
38+
name = "kernel_path",
39+
srcs = ["kernel_path.cc"],
40+
hdrs = ["kernel_path.h"],
41+
deps = [
42+
"//cuttlefish/host/commands/assemble_cvd:flags_defaults",
43+
"//cuttlefish/host/libs/config:fetcher_config",
44+
"//libbase",
45+
"@gflags",
46+
],
47+
)
48+
3749
cf_cc_library(
3850
name = "system_image_dir",
3951
srcs = ["system_image_dir.cc"],

0 commit comments

Comments
 (0)