sysroot: Merge bootconfig-extra from previously staged deployment#3587
sysroot: Merge bootconfig-extra from previously staged deployment#3587jmarrero wants to merge 1 commit into
Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Pull request overview
Fixes loss of bootconfig-extra extension BLS keys when a staged deployment is replaced by a subsequent staging on the same boot (e.g., bootc stages source-tracked keys, then rpm-ostree re-stages while appending kargs). The change ensures previously staged extension keys are merged forward so consumers don’t inadvertently drop each other’s x-options-source-* metadata.
Changes:
- Merge
bootconfig-extrafrom three sources during staging: merge deployment bootconfig (lowest), previously stagedbootconfig-extra, and new deployment bootconfig (highest). - Persist the merged result back into the staged state GVariant written under
/run/ostree/staged-deployment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Extension keys can come from three sources, merged in priority order | ||
| * (later sources do not override earlier ones): | ||
| * | ||
| * 1. The new deployment's bootconfig (caller set keys directly) | ||
| * 2. The previously staged deployment's bootconfig-extra (a prior | ||
| * consumer like bootc may have staged keys that would be lost | ||
| * when this new staging replaces the old staged GVariant) | ||
| * 3. The merge deployment's bootconfig (on-disk BLS from the | ||
| * currently booted or pending deployment) |
| GVariantIter iter; | ||
| const char *k, *v; | ||
| g_variant_iter_init (&iter, merge_extra); | ||
| while (g_variant_iter_next (&iter, "{&s&s}", &k, &v)) | ||
| g_hash_table_insert (merged_extra, g_strdup (k), g_strdup (v)); |
| /* Priority 2: previously staged deployment's bootconfig-extra */ | ||
| { | ||
| glnx_autofd int staged_fd = -1; | ||
| if (!ot_openat_ignore_enoent (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED, &staged_fd, error)) | ||
| return FALSE; | ||
| if (staged_fd != -1) | ||
| { | ||
| g_autoptr (GBytes) staged_contents = ot_fd_readall_or_mmap (staged_fd, 0, error); | ||
| if (!staged_contents) | ||
| return FALSE; | ||
| g_autoptr (GVariant) staged_data | ||
| = g_variant_new_from_bytes ((GVariantType *)"a{sv}", staged_contents, TRUE); | ||
| g_autoptr (GVariantDict) staged_dict = g_variant_dict_new (staged_data); | ||
|
|
||
| g_autoptr (GVariant) prev_extra = NULL; | ||
| if (g_variant_dict_lookup (staged_dict, "bootconfig-extra", "@a{ss}", &prev_extra)) | ||
| { |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the staging logic in ostree-sysroot-deploy.c to implement a prioritized merging strategy for bootconfig-extra keys, ensuring that custom keys from the new deployment, previously staged state, and merge deployment are correctly preserved. A critical memory management issue was identified regarding the handling of floating references for the staged_data GVariant, which could lead to a double-unref or use-after-free when combined with g_autoptr and g_variant_dict_new.
jmarrero
force-pushed
the
fix-staged-bootconfig-extra-merge
branch
from
cd006bd to
e1c1122
Compare
When a staged deployment is replaced by a new one (e.g. bootc sets source-tracked kargs, then rpm-ostree appends a karg before reboot), the bootconfig-extra keys from the first staging were lost. The new staging only checked the new deployment's bootconfig (always empty) and fell back to the merge deployment's bootconfig (the booted BLS file), which had no knowledge of what was in the previous staged GVariant. Fix this by merging bootconfig-extra from three sources in priority order: 1. Merge deployment's bootconfig (lowest priority) 2. Previously staged deployment's bootconfig-extra 3. New deployment's bootconfig (highest priority) This ensures that x-options-source-* keys set by one consumer survive re-staging by another consumer on the same boot. Assisted-by: OpenCode (Claude Opus 4.6) Signed-off-by: Joseph Marrero Corchado <jmarrero@redhat.com>
jmarrero
force-pushed
the
fix-staged-bootconfig-extra-merge
branch
from
e1c1122 to
8503373
Compare
| return FALSE; | ||
| if (staged_fd != -1) | ||
| { | ||
| g_autoptr (GBytes) staged_contents = ot_fd_readall_or_mmap (staged_fd, 0, error); |
There was a problem hiding this comment.
Hmm I think we will have already loaded this data at this point and cached it in OstreeSysroot. Not big deal though.
When a staged deployment is replaced by a new one (e.g. bootc sets source-tracked kargs, then rpm-ostree appends a karg before reboot), the bootconfig-extra keys from the first staging were lost. The new staging only checked the new deployment's bootconfig (always empty) and fell back to the merge deployment's bootconfig (the booted BLS file), which had no knowledge of what was in the previous staged GVariant.
Fix this by merging bootconfig-extra from three sources in priority order:
This ensures that x-options-source-* keys set by one consumer survive re-staging by another consumer on the same boot.
Assisted-by: OpenCode (Claude Opus 4.6)