Skip to content

Commit c63ac16

Browse files
committed
Create a InitramfsPathFlag 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_initramfs_path` can be accessed. This is unintuitive. With a custom type, access to the value can be gated behind correct initialization. Bug: b/430378744 Test: Fetch with `--kernel_build` Test: Launch with `--kernel_path` and `--initramfs_path`
1 parent 06f9d77 commit c63ac16

11 files changed

Lines changed: 177 additions & 60 deletions

File tree

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ cf_cc_binary(
2525
"//cuttlefish/common/libs/utils:flag_parser",
2626
"//cuttlefish/common/libs/utils:in_sandbox",
2727
"//cuttlefish/common/libs/utils:tee_logging",
28+
"//cuttlefish/host/commands/assemble_cvd/disk:factory_reset_protected",
29+
"//cuttlefish/host/commands/assemble_cvd/disk:metadata_image",
30+
"//cuttlefish/host/commands/assemble_cvd/disk:misc_image",
31+
"//cuttlefish/host/commands/assemble_cvd/flags:initramfs_path",
32+
"//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
33+
"//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
2834
"//cuttlefish/host/commands/assemble_cvd:assemble_cvd_flags",
2935
"//cuttlefish/host/commands/assemble_cvd:clean",
3036
"//cuttlefish/host/commands/assemble_cvd:disk_flags",
@@ -33,18 +39,13 @@ cf_cc_binary(
3339
"//cuttlefish/host/commands/assemble_cvd:flags",
3440
"//cuttlefish/host/commands/assemble_cvd:flags_defaults",
3541
"//cuttlefish/host/commands/assemble_cvd:touchpad",
36-
"//cuttlefish/host/commands/assemble_cvd/disk:factory_reset_protected",
37-
"//cuttlefish/host/commands/assemble_cvd/disk:metadata_image",
38-
"//cuttlefish/host/commands/assemble_cvd/disk:misc_image",
39-
"//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
40-
"//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
4142
"//cuttlefish/host/libs/command_util",
43+
"//cuttlefish/host/libs/config/adb",
44+
"//cuttlefish/host/libs/config/fastboot",
4245
"//cuttlefish/host/libs/config:ap_boot_flow",
4346
"//cuttlefish/host/libs/config:config_flag",
4447
"//cuttlefish/host/libs/config:custom_actions",
4548
"//cuttlefish/host/libs/config:fetcher_config",
46-
"//cuttlefish/host/libs/config/adb",
47-
"//cuttlefish/host/libs/config/fastboot",
4849
"//cuttlefish/host/libs/feature:inject",
4950
"//libbase",
5051
"@gflags",
@@ -161,6 +162,7 @@ cf_cc_library(
161162
"//cuttlefish/host/commands/assemble_cvd/disk:pstore",
162163
"//cuttlefish/host/commands/assemble_cvd/disk:sd_card",
163164
"//cuttlefish/host/commands/assemble_cvd/disk:vbmeta_enforce_minimum_size",
165+
"//cuttlefish/host/commands/assemble_cvd/flags:initramfs_path",
164166
"//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
165167
"//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
166168
"//cuttlefish/host/libs/avb",
@@ -234,6 +236,7 @@ cf_cc_library(
234236
"//cuttlefish/host/commands/assemble_cvd:network_flags",
235237
"//cuttlefish/host/commands/assemble_cvd:touchpad",
236238
"//cuttlefish/host/commands/assemble_cvd/flags:display_proto",
239+
"//cuttlefish/host/commands/assemble_cvd/flags:initramfs_path",
237240
"//cuttlefish/host/commands/assemble_cvd/flags:kernel_path",
238241
"//cuttlefish/host/commands/assemble_cvd/flags:system_image_dir",
239242
"//cuttlefish/host/libs/config:ap_boot_flow",

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

Lines changed: 16 additions & 24 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/initramfs_path.h"
4344
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
4445
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
4546
#include "cuttlefish/host/commands/assemble_cvd/flags_defaults.h"
@@ -310,17 +311,18 @@ Result<SharedFD> SetLogger(std::string runtime_dir_parent) {
310311
Result<const CuttlefishConfig*> InitFilesystemAndCreateConfig(
311312
FetcherConfig fetcher_config, const std::vector<GuestConfig>& guest_configs,
312313
fruit::Injector<>& injector, SharedFD log,
313-
const KernelPathFlag& kernel_path,
314+
const InitramfsPathFlag& initramfs_path, const KernelPathFlag& kernel_path,
314315
const SystemImageDirFlag& system_image_dir) {
315316
{
316317
// The config object is created here, but only exists in memory until the
317318
// SaveConfig line below. Don't launch cuttlefish subprocesses between these
318319
// two operations, as those will assume they can read the config object from
319320
// disk.
320-
auto config = CF_EXPECT(InitializeCuttlefishConfiguration(
321-
FLAGS_instance_dir, guest_configs, injector,
322-
fetcher_config, kernel_path, system_image_dir),
323-
"cuttlefish configuration initialization failed");
321+
auto config = CF_EXPECT(
322+
InitializeCuttlefishConfiguration(
323+
FLAGS_instance_dir, guest_configs, injector, fetcher_config,
324+
initramfs_path, kernel_path, system_image_dir),
325+
"cuttlefish configuration initialization failed");
324326

325327
const std::string snapshot_path = FLAGS_snapshot_path;
326328
if (!snapshot_path.empty()) {
@@ -506,17 +508,6 @@ Result<const CuttlefishConfig*> InitFilesystemAndCreateConfig(
506508
return config;
507509
}
508510

509-
const std::string kKernelDefaultPath = "kernel";
510-
const std::string kInitramfsImg = "initramfs.img";
511-
static void ExtractKernelParamsFromFetcherConfig(
512-
const FetcherConfig& fetcher_config) {
513-
std::string discovered_ramdisk =
514-
fetcher_config.FindCvdFileWithSuffix(kInitramfsImg);
515-
516-
SetCommandLineOptionWithMode("initramfs_path", discovered_ramdisk.c_str(),
517-
google::FlagSettingMode::SET_FLAGS_DEFAULT);
518-
}
519-
520511
Result<void> VerifyConditionsOnSnapshotRestore(
521512
const std::string& snapshot_path) {
522513
if (snapshot_path.empty()) {
@@ -580,9 +571,6 @@ Result<int> AssembleCvdMain(int argc, char** argv) {
580571

581572
FetcherConfig fetcher_config = FindFetcherConfig(input_files);
582573

583-
// set gflags defaults to point to kernel/RD from fetcher config
584-
ExtractKernelParamsFromFetcherConfig(fetcher_config);
585-
586574
auto args = ArgsToVec(argc - 1, argv + 1);
587575

588576
bool help = false;
@@ -615,6 +603,8 @@ Result<int> AssembleCvdMain(int argc, char** argv) {
615603
/* remove_flags */ false);
616604
}
617605

606+
InitramfsPathFlag initramfs_path =
607+
InitramfsPathFlag::FromGlobalGflags(fetcher_config);
618608
KernelPathFlag kernel_path = KernelPathFlag::FromGlobalGflags(fetcher_config);
619609

620610
SystemImageDirFlag system_image_dir =
@@ -649,13 +639,15 @@ Result<int> AssembleCvdMain(int argc, char** argv) {
649639
// them in place, and either errors out on unknown flags or accepts any flags.
650640

651641
auto guest_configs =
652-
CF_EXPECT(GetGuestConfigAndSetDefaults(kernel_path, system_image_dir),
642+
CF_EXPECT(GetGuestConfigAndSetDefaults(initramfs_path, kernel_path,
643+
system_image_dir),
653644
"Failed to parse arguments");
654645

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");
646+
auto config =
647+
CF_EXPECT(InitFilesystemAndCreateConfig(
648+
std::move(fetcher_config), guest_configs, injector, log,
649+
initramfs_path, kernel_path, system_image_dir),
650+
"Failed to create config");
659651

660652
std::cout << GetConfigFilePath(*config) << "\n";
661653
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(initramfs_path, CF_DEFAULTS_INITRAMFS_PATH,
62-
"Path to the initramfs");
6361
DEFINE_string(extra_kernel_cmdline, CF_DEFAULTS_EXTRA_KERNEL_CMDLINE,
6462
"Additional flags to put on the kernel command line");
6563
DEFINE_string(extra_bootconfig_args, CF_DEFAULTS_EXTRA_BOOTCONFIG_ARGS,

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(initramfs_path);
4342
DECLARE_string(extra_kernel_cmdline);
4443
DECLARE_string(extra_bootconfig_args);
4544
DECLARE_vec(guest_enforce_security);

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

Lines changed: 16 additions & 19 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/initramfs_path.h"
5556
#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/super_image_mixer.h"
@@ -69,12 +70,13 @@ namespace cuttlefish {
6970

7071
using vm_manager::Gem5Manager;
7172

72-
Result<void> ResolveInstanceFiles(const KernelPathFlag& kernel_path,
73+
Result<void> ResolveInstanceFiles(const InitramfsPathFlag& initramfs_path,
74+
const KernelPathFlag& kernel_path,
7375
const SystemImageDirFlag& system_image_dir) {
7476
// It is conflict (invalid) to pass both kernel_path/initramfs_path
7577
// and image file paths.
7678
bool flags_kernel_initramfs_has_input =
77-
(!kernel_path.HasValue()) || (!FLAGS_initramfs_path.empty());
79+
(!kernel_path.HasValue()) || (!initramfs_path.HasValue());
7880
bool flags_image_has_input =
7981
(!FLAGS_super_image.empty()) || (!FLAGS_vendor_boot_image.empty()) ||
8082
(!FLAGS_vbmeta_vendor_dlkm_image.empty()) ||
@@ -247,7 +249,7 @@ static fruit::Component<> DiskChangesPerInstanceComponent(
247249

248250
Result<void> DiskImageFlagsVectorization(
249251
CuttlefishConfig& config, const FetcherConfig& fetcher_config,
250-
const KernelPathFlag& kernel_path,
252+
const InitramfsPathFlag& initramfs_path, const KernelPathFlag& kernel_path,
251253
const SystemImageDirFlag& system_image_dir) {
252254
std::vector<std::string> boot_image =
253255
android::base::Split(FLAGS_boot_image, ",");
@@ -301,13 +303,10 @@ Result<void> DiskImageFlagsVectorization(
301303

302304
std::vector<std::string> bootloader =
303305
android::base::Split(FLAGS_bootloader, ",");
304-
std::vector<std::string> initramfs_path =
305-
android::base::Split(FLAGS_initramfs_path, ",");
306306

307307
std::vector<std::string> blank_sdcard_image_mb =
308308
android::base::Split(FLAGS_blank_sdcard_image_mb, ",");
309309

310-
std::string cur_initramfs_path;
311310
std::string cur_boot_image;
312311
std::string cur_vendor_boot_image;
313312
std::string cur_super_image;
@@ -434,12 +433,8 @@ Result<void> DiskImageFlagsVectorization(
434433
instance.set_bootloader(bootloader[instance_index]);
435434
}
436435
instance.set_kernel_path(kernel_path.KernelPathForIndex(instance_index));
437-
if (instance_index >= initramfs_path.size()) {
438-
cur_initramfs_path = initramfs_path[0];
439-
} else {
440-
cur_initramfs_path = initramfs_path[instance_index];
441-
}
442-
instance.set_initramfs_path(cur_initramfs_path);
436+
instance.set_initramfs_path(
437+
initramfs_path.InitramfsPathForIndex(instance_index));
443438

444439
using android::base::ParseInt;
445440

@@ -467,12 +462,14 @@ Result<void> DiskImageFlagsVectorization(
467462
"/userdata.img");
468463
instance.set_new_data_image(const_instance.PerInstancePath("userdata.img"));
469464

470-
if (!kernel_path.KernelPathForIndex(instance_index).empty() ||
471-
!cur_initramfs_path.empty()) {
465+
bool has_kernel = !kernel_path.KernelPathForIndex(instance_index).empty();
466+
bool has_initramfs =
467+
!initramfs_path.InitramfsPathForIndex(instance_index).empty();
468+
if (has_kernel || has_initramfs) {
472469
const std::string new_vendor_boot_image_path =
473470
const_instance.PerInstancePath("vendor_boot_repacked.img");
474471
// Repack the vendor boot images if kernels and/or ramdisks are passed in.
475-
if (!cur_initramfs_path.empty()) {
472+
if (has_initramfs) {
476473
// change the new flag value to corresponding instance
477474
instance.set_new_vendor_boot_image(new_vendor_boot_image_path.c_str());
478475
}
@@ -491,10 +488,10 @@ Result<void> DiskImageFlagsVectorization(
491488

492489
// We will need to rebuild vendor_dlkm if custom ramdisk is specified, as a
493490
// result super image would need to be rebuilt as well.
494-
if (CF_EXPECT(SuperImageNeedsRebuilding(fetcher_config,
495-
const_instance.default_target_zip(),
496-
const_instance.system_target_zip())) ||
497-
!cur_initramfs_path.empty()) {
491+
if (CF_EXPECT(SuperImageNeedsRebuilding(
492+
fetcher_config, const_instance.default_target_zip(),
493+
const_instance.system_target_zip())) ||
494+
has_initramfs) {
498495
const std::string new_super_image_path =
499496
const_instance.PerInstancePath("super.img");
500497
instance.set_new_super_image(new_super_image_path);

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,21 +20,24 @@
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/initramfs_path.h"
2324
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
2425
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
2526
#include "cuttlefish/host/libs/config/cuttlefish_config.h"
2627
#include "cuttlefish/host/libs/config/fetcher_config.h"
2728

2829
namespace cuttlefish {
2930

30-
Result<void> ResolveInstanceFiles(const KernelPathFlag& kernel_path,
31+
Result<void> ResolveInstanceFiles(const InitramfsPathFlag&,
32+
const KernelPathFlag& kernel_path,
3133
const SystemImageDirFlag& system_image_dir);
3234

3335
Result<void> CreateDynamicDiskFiles(const FetcherConfig& fetcher_config,
3436
const CuttlefishConfig& config,
3537
const SystemImageDirFlag&);
3638
Result<void> DiskImageFlagsVectorization(CuttlefishConfig& config,
3739
const FetcherConfig& fetcher_config,
40+
const InitramfsPathFlag&,
3841
const KernelPathFlag&,
3942
const SystemImageDirFlag&);
4043
DiskBuilder OsCompositeDiskBuilder(

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

Lines changed: 6 additions & 4 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/initramfs_path.h"
5657
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
5758
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
5859
#include "cuttlefish/host/commands/assemble_cvd/graphics_flags.h"
@@ -593,7 +594,7 @@ std::string DefaultBootloaderArchDir(Arch arch) {
593594
Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
594595
const std::string& root_dir, const std::vector<GuestConfig>& guest_configs,
595596
fruit::Injector<>& injector, const FetcherConfig& fetcher_config,
596-
const KernelPathFlag& kernel_path,
597+
const InitramfsPathFlag& initramfs_path, const KernelPathFlag& kernel_path,
597598
const SystemImageDirFlag& system_image_dir) {
598599
CuttlefishConfig tmp_config_obj;
599600
// If a snapshot path is provided, do not read all flags to set up the config.
@@ -1602,7 +1603,8 @@ Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
16021603
"The set of flags is incompatible with snapshot");
16031604

16041605
CF_EXPECT(DiskImageFlagsVectorization(tmp_config_obj, fetcher_config,
1605-
kernel_path, system_image_dir));
1606+
initramfs_path, kernel_path,
1607+
system_image_dir));
16061608

16071609
return tmp_config_obj;
16081610
}
@@ -1801,13 +1803,13 @@ void SetDefaultFlagsForOpenwrt(Arch target_arch) {
18011803
}
18021804

18031805
Result<std::vector<GuestConfig>> GetGuestConfigAndSetDefaults(
1804-
const KernelPathFlag& kernel_path,
1806+
const InitramfsPathFlag& initramfs_path, const KernelPathFlag& kernel_path,
18051807
const SystemImageDirFlag& system_image_dir) {
18061808
auto instance_nums =
18071809
CF_EXPECT(InstanceNumsCalculator().FromGlobalGflags().Calculate());
18081810
int32_t instances_size = instance_nums.size();
18091811

1810-
CF_EXPECT(ResolveInstanceFiles(kernel_path, system_image_dir),
1812+
CF_EXPECT(ResolveInstanceFiles(initramfs_path, kernel_path, system_image_dir),
18111813
"Failed to resolve instance files");
18121814

18131815
std::vector<GuestConfig> guest_configs =

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

Lines changed: 4 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/initramfs_path.h"
2324
#include "cuttlefish/host/commands/assemble_cvd/flags/kernel_path.h"
2425
#include "cuttlefish/host/commands/assemble_cvd/flags/system_image_dir.h"
2526
#include "cuttlefish/host/commands/assemble_cvd/guest_config.h"
@@ -29,12 +30,13 @@
2930
namespace cuttlefish {
3031

3132
Result<std::vector<GuestConfig>> GetGuestConfigAndSetDefaults(
32-
const KernelPathFlag&, const SystemImageDirFlag&);
33+
const InitramfsPathFlag&, const KernelPathFlag&, const SystemImageDirFlag&);
3334
// Must be called after ParseCommandLineFlags.
3435
Result<CuttlefishConfig> InitializeCuttlefishConfiguration(
3536
const std::string& root_dir, const std::vector<GuestConfig>& guest_configs,
3637
fruit::Injector<>& injector, const FetcherConfig& fetcher_config,
37-
const KernelPathFlag& kernel_path, const SystemImageDirFlag&);
38+
const InitramfsPathFlag&, const KernelPathFlag& kernel_path,
39+
const SystemImageDirFlag&);
3840

3941
std::string GetConfigFilePath(const CuttlefishConfig& config);
4042

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 = "initramfs_path",
39+
srcs = ["initramfs_path.cc"],
40+
hdrs = ["initramfs_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 = "kernel_path",
3951
srcs = ["kernel_path.cc"],

0 commit comments

Comments
 (0)