assemble_cvd: Support named/A-B partitions in custom_partition_path flag#2457
assemble_cvd: Support named/A-B partitions in custom_partition_path flag#2457pjj1005 wants to merge 1 commit intogoogle:mainfrom
Conversation
1. Extended custom_partition_path flag format (disk_flags.cc)
Previously, the --custom_partition_path flag only accepted a list of
image file paths separated by semicolons, and partition names were
hard-coded as "custom", "custom_1", "custom_2", etc. This made it
impossible to assign OEM-specific partition names required by the
bootloader (e.g. "oem_ext", "confptstatic", etc.).
The entry format is now extended to 'name:path[:ab]':
- name : partition label visible to the bootloader/kernel
- path : image file path
- :ab : optional suffix; if present, both _a and _b A/B slot
partitions are created from the same image file
Legacy format (path only) is still accepted for backward compatibility,
in which case partition names fall back to the original auto-generated
"custom" / "custom_N" naming.
A LOG(INFO) message is emitted for each partition that is added to
aid debugging of partition configuration issues.
Example:
--custom_partition_path="oem_ext:./oem_ext.img:ab;confcal:./confcal.img"
2. Include partition label in TextConfig() (disk_builder.cpp)
TextConfig() is used to detect whether the composite disk needs to be
rebuilt on the next run. Previously it stored only the image file path
for each partition. As a result, changing only a partition label (while
keeping the same image file) would not trigger a rebuild, leaving the
stale composite disk in use.
Now the format is 'label:path' per line, so any change to either the
partition name or the image path correctly invalidates the cached
composite disk and forces a rebuild.
Change-Id: Ic953c84ce2c07bea2a1f12111726b366a43e33c1
Signed-off-by: Jongjin Park <jongjin1.park@lge.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| "to rootfs and can be used by /dev/block/by-name/custom. Multiple images " | ||
| "can be passed, separated by semicolons and can be used as " | ||
| "/dev/block/by-name/custom_1, /dev/block/by-name/custom_2, etc. Example: " | ||
| "Location of custom image that will be passed as a custom partition " |
There was a problem hiding this comment.
The idea is both valid and useful. My only concern is that the new syntax relies on segment ordering and specific positioning, which will be difficult to manage as we introduce additional segments in the future.
What do you think about using a key=value syntax separated by commas instead? It would look something like this: name=oem,path=./oem.img,ab=true;name=oem2,path=./oem2.img;.... Utilizing the absl library should make the parsing code simple to implement and easy to maintain.
| image_path = std::string(parts[1]); | ||
| if (parts.size() >= 3 && parts[2] == "ab") { | ||
| ab_enabled = true; | ||
| } |
There was a problem hiding this comment.
Validate the input and return errors if needed. For example, if the third parameter is not "ab" or if there are more than 3 parameters total.
| std::string label; | ||
| std::string image_path; | ||
| bool ab_enabled = false; | ||
| std::vector<std::string_view> parts = | ||
| absl::StrSplit(custom_partition_paths[i], ':'); | ||
| if (parts.size() >= 2) { | ||
| label = std::string(parts[0]); | ||
| image_path = std::string(parts[1]); | ||
| if (parts.size() >= 3 && parts[2] == "ab") { | ||
| ab_enabled = true; | ||
| } | ||
| } else { | ||
| label = i > 0 ? "custom_" + std::to_string(i) : "custom"; | ||
| image_path = std::string(custom_partition_paths[i]); | ||
| } |
There was a problem hiding this comment.
Encapsulate the string parsing into a function like this:
struct CustomPartitionSpec {
std::string label;
std::string path;
bool ab_enabled;
};
Result<CustomPartitionSpec> ParseCustomPartitionSpec(std::string_view spec) {...}
| image_path = std::string(custom_partition_paths[i]); | ||
| } | ||
| if (ab_enabled) { | ||
| LOG(INFO) << "Adding custom partition: " << label + "_a"; |
There was a problem hiding this comment.
These logs are too noisy. Use verbose logging (VLOG(1)) or no logging at all (no other partitions are logged in this function).
3 participants
Previously, the --custom_partition_path flag only accepted a list of image file paths separated by semicolons, and partition names were hard-coded as "custom", "custom_1", "custom_2", etc. This made it impossible to assign OEM-specific partition names required by the bootloader (e.g. "oem_ext", "confptstatic", etc.).
The entry format is now extended to 'name:path[:ab]':
partitions are created from the same image file
Legacy format (path only) is still accepted for backward compatibility, in which case partition names fall back to the original auto-generated "custom" / "custom_N" naming.
A LOG(INFO) message is emitted for each partition that is added to aid debugging of partition configuration issues.
Example:
--custom_partition_path="oem_ext:./oem_ext.img:ab;confcal:./confcal.img"
TextConfig() is used to detect whether the composite disk needs to be rebuilt on the next run. Previously it stored only the image file path for each partition. As a result, changing only a partition label (while keeping the same image file) would not trigger a rebuild, leaving the stale composite disk in use.
Now the format is 'label:path' per line, so any change to either the partition name or the image path correctly invalidates the cached composite disk and forces a rebuild.
Change-Id: Ic953c84ce2c07bea2a1f12111726b366a43e33c1