provision: record expand: true on the disk as GPT attribute bit 56 - #31
provision: record expand: true on the disk as GPT attribute bit 56#31mobileoverlord wants to merge 1 commit into
Conversation
Emit AVOCADO_PARTITION_<NAME>_FLAGS for every partition (0x0100000000000000 when the manifest says expand: "true", 0 otherwise) so the fwup templates can write it as the partition's GPT attribute flags. An image written to a file and flashed later cannot be expanded at flash time, and nothing else on the device knows what the manifest asked for; with the bit on the partition the initramfs can extend it to the disk before /var is opened. Bits 48-63 are the partition type owner's; 56 is Avocado's grow-to-disk mark.
There was a problem hiding this comment.
Pull request overview
This PR extends stone provision to persist the manifest’s expand: "true" intent onto the partition itself by emitting per-partition GPT attribute flags (bit 56 in the type-owner range), enabling boot-time expansion when images are written to files and flashed later.
Changes:
- Add
AVOCADO_PARTITION_<NAME>_FLAGSenv var emission, encodingexpand: "true"as GPT attribute bit 56 (0x0100000000000000) and0otherwise. - Introduce
partition_gpt_flags()and a unit test covering the flag formatting/behavior. - Minor formatting refactors in manifest/bundle code and tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/commands/stone/bundle/mod.rs | Minor test formatting adjustment for bundle error output predicate. |
| src/manifest.rs | Formatting-only refactors plus test formatting; no behavioral change in this diff. |
| src/commands/stone/provision.rs | Adds GPT flag constant + formatter and emits ..._FLAGS env vars; includes unit test. |
| src/commands/stone/bundle.rs | Formatting-only refactors (function signature and a few wrapped lines). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| env_vars.insert( | ||
| format!("AVOCADO_PARTITION_{name_upper}_FLAGS"), | ||
| partition_gpt_flags(partition).to_string(), | ||
| ); |
| } | ||
|
|
||
| /// GPT attribute bit set on partitions the manifest marks `expand: "true"`. | ||
| /// Bits 48-63 are reserved for the partition type's owner; 56 is ours. |
There was a problem hiding this comment.
doc: "Bits 48-63 are reserved for the partition type's owner; 56 is ours" is not accurate for the partitions this lands on. meta-avocado's rootdisk.conf templates default the var partition type to 4d21b016-b534-45c2-a9fb-5c16e091fd2d, which is the Discoverable Partitions Specification's /var type; rootfs and boot are DPS types too. The owner of the 48-63 range for that GUID is the UAPI Group, not Avocado, and the spec already allocates 59 (grow-file-system), 60 (read-only) and 63 (no-auto) in it.
Bit 56 is genuinely unassigned there today, so nothing is broken. What the comment does is tell the next reader the bit is ours to allocate, when a future DPS allocation could take it and stone neither sets nor checks the partition type, so the type is entirely template-controlled. Either move var to an Avocado-owned type GUID, or state plainly that this squats an unassigned bit in a namespace we do not own.
| .name | ||
| .clone() | ||
| .unwrap_or_else(|| format!("#{idx}")); | ||
| let label = p.name.clone().unwrap_or_else(|| format!("#{idx}")); |
There was a problem hiding this comment.
problem: validate_partitions only constrains the size-omitted case. Now that expand: "true" escapes to the device and drives an on-device repartition, two manifests it accepts do the wrong thing.
A partition with an explicit size and expand: "true" that is not last hits the (true, true) arm and passes. A manifest with var (256 MiB, expand: "true") followed by data validates clean, and stone stamps bit 56 on var. fwup catches this on the expand = channel ("a partition can't be specified after the one with expand = true"), but only when the template wires expand = for that partition; the flags channel has no equivalent guard.
The mirror case is silent rather than dangerous. Every partition variable is gated behind if let Some(partition_name), so an unnamed partition asking for expansion gets no _FLAGS at all. tests/fixtures/coverage/stone.json:42-47 is exactly that: a final partition with expand: "true" and no name. It passes stone validate, and stone provision --verbose against it emits no AVOCADO_PARTITION_* lines. That also makes "emits AVOCADO_PARTITION_<NAME>_FLAGS for every partition" untrue against the repo's own fixture.
Both close with one clause in validate_partitions: reject expand: "true" on any partition that is not last, and require a name on it.
What
stone provisionnow emitsAVOCADO_PARTITION_<NAME>_FLAGSfor every partition:0x0100000000000000when the manifest saysexpand: "true",0x0otherwise. The fwup templates write it as the partition's GPT attribute flags (flags = ${VAR_PART_FLAGS}, meta-avocado).Why
An image written to a file (
uuupath) cannot be expanded at flash time, and nothing else on the device knows what the manifest asked for. With the mark on the partition itself, the initramfs can extend it to the disk before/varis opened — and the build stays medium-agnostic. Bits 48–63 of a GPT entry are the partition type owner's; 56 is Avocado's "grow to disk".No layout change for manifests without
expand. Test added.