install: Add --output-{json,pairs}-{path,fd} for the install result by cgwalters-bot · Pull Request #2531 · bootc-dev/bootc

cgwalters-bot · 2026-10-02T19:29:47Z

Adds options to bootc install to-filesystem (and to-disk, to-existing-root, which share the code) to write a machine-readable result once the install succeeded, as sketched in #542 (comment):

--output-json-path PATH    --output-json-fd FD
--output-pairs-path PATH   --output-pairs-fd FD

At most one may be given. Pairs are KEY="value" lines as lsblk --pairs --shell (one per line; ", \, $ and backtick are backslash-escaped as in os-release, so eval round-trips). Keys: backend, stateroot, deployment/etc/var paths relative to the target, bootloader, image, image transport and digest; JSON uses camelCase (etcPath), pairs upper snake case (ETC_PATH). A path is replaced atomically; an fd must be inherited (not close-on-exec) and writable, checked before anything is installed. The path's directory is opened up front and carried across bootc's re-execs, so /tmp/... doesn't silently land in the tmpfs bootc mounts there.

pairs=$(bootc install to-filesystem --output-pairs-fd 3 /mnt 3>&1 >&2)
eval "$pairs"
cp my.service "/mnt/$ETC_PATH/systemd/system/"

Not covered: install reset, which has its own flow.

Testing, on a 16-core RHEL 10 devspace (AMD EPYC 7763; no fork CI):

just unit-tests passed (bootc-lib: 290, including 7 new ones covering pairs quoting with a real sh eval round-trip, key names, rendering, and fd/path validation), and so did just validate.
tmt plan-23-install-outside-container, extended to install with --output-json-path and check the reported deployment, /etc (a file written via bootc install mount is found under etcPath), /var, image and digest, plus that an fd the caller didn't pass is rejected before installing: passed for the ostree variant and for composefs (grub, ext4, BLS).
By hand, in a bcvk ephemeral VM: podman run --privileged --preserve-fds 1 localhost/bootc bootc install to-disk --output-pairs-fd 3 ... 3>&1 >&2 into pairs=$(...), then eval "$pairs"; the reported $ETC_PATH, $VAR_PATH and $DEPLOYMENT_PATH/usr existed on the installed disk.

xref #542

The Signed-off-by: Colin Walters <walters@verbum.org> on these commits was added on cgwalters's approval of the review draft: cgwalters-forge#38 (review)

Generated-by: https://github.com/cgwalters/#llms

cgwalters

I bet there's some hidden "information passing" between current to-disk (baseline.rs) and to-filesystem that could be changed to use this in the same way an external installer would, let's do taht

cgwalters · 2026-10-02T19:52:35Z

    pull_result: &composefs_oci::PullResult<Sha512HashValue>,
    allow_missing_fsverity: bool,
-) -> Result<()> {
+) -> Result<(String, Bootloader)> {


Let's make this a proper struct and we should be returning a proper verity digest type, not a String.

Done: setup_composefs_boot now returns ComposefsBootSetup { deployment_id: Sha512HashValue, bootloader }; the hex is only formed where the result path is built.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T19:54:57Z

+/// The storage backend of an installed system.
+#[derive(Debug, Clone, Copy, Serialize, PartialEq, Eq)]
+#[serde(rename_all = "lowercase")]
+pub(crate) enum Backend {


Hmm I think this overlaps with an existing enum we could at least map impl From or alternatively use this one elsewhere (replace composefs_backend cli arg in install state?)

Moved it to crate::store::Backend in a prep commit ("install: Decide the storage backend once"): install State now carries the decided backend, which baseline's ESP sizing and the install branch use instead of composefs_options.composefs_backend, and bootc install mount uses it in place of its own DeploymentBackend. The --composefs-backend flag itself stays a bool.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T19:55:24Z

+}
+
+impl OutputFormat {
+    fn render(self, result: &InstallResult) -> Result<Vec<u8>> {


This should take an impl Write

Done: render() takes an impl Write; the fd gets written directly, the path via atomic_replace_with.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T19:56:19Z

+
+impl Destination {
+    #[context("Validating output fd {fd}")]
+    fn from_fd(fd: RawFd) -> Result<Self> {


Hum would be cleaner to have happen as part of argument parsing I think

Done: --output-*-fd and --output-*-path have value parsers now, so a bad destination is a clap error (invalid value '9' for '--output-json-fd <FD>': fd 9 was not inherited ...), and the path's directory is opened right there.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T19:56:53Z

+/// while everything bootc opens itself is.  That rejects an fd number the
+/// caller didn't actually pass, which may belong to e.g. the async runtime.
+#[allow(unsafe_code)]
+fn adopt_inherited_fd(fd: RawFd) -> Result<OwnedFd> {


This kind of function shouldn't be in this module it should be in a shared relevant place related to above re parsing fds.

Moved to bootc_utils::InheritedFd (crates/utils/src/fd.rs), a FromStr type for fd arguments that takes ownership once at parse time (clones share it via Arc). --progress-fd uses it too now.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T19:58:03Z

+/// caller didn't actually pass, which may belong to e.g. the async runtime.
+#[allow(unsafe_code)]
+fn adopt_inherited_fd(fd: RawFd) -> Result<OwnedFd> {
+    // SAFETY: The borrow is only used for fcntl(), which is fine for any fd number.


Well not really that's not what makes it unsafe, it's that there's just no way to handle I/O safety when we're passed arbitrary numbers xref rust-lang/rust#116059 just link to that

Fixed: the type docs now say there's no I/O safety for an fd known only by number and link rust-lang/rust#116059; the checks are what we have instead.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T19:58:38Z

+    // SAFETY: The borrow is only used for fcntl(), which is fine for any fd number.
+    let borrowed = unsafe { BorrowedFd::borrow_raw(fd) };
+    let fdflags = rustix::io::fcntl_getfd(borrowed).with_context(|| format!("fd {fd}"))?;
+    if fdflags.contains(FdFlags::CLOEXEC) {


Hum interesting...I guess that's one way to check if it's at least likely it came from an external place?

Yes: an fd inherited across exec can't be CLOEXEC, while everything bootc opens itself is, so it catches a number the caller never passed that happens to be in use internally (e.g. the runtime's epoll fd). Kept, in InheritedFd.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T19:59:46Z

+/// Convert a camelCase key to a shell variable name, e.g. `etcPath` to
+/// `ETC_PATH`.  Like `lsblk --shell`, any other character that isn't valid in
+/// a variable name becomes `_`.
+fn shell_key(key: &str) -> String {


This and below should also be in a common helper.

Would https://docs.rs/shlex/latest/shlex/ help us? Maybe not...

Moved to bootc_utils::write_shell_pairs (crates/utils/src/pairs.rs). shlex doesn't fit: try_quote is for command arguments, leaves plain words unquoted and keeps a newline literally inside single quotes, while this format double-quotes every value like lsblk/os-release and keeps one line per key, so it parses without a shell too. That reasoning is in the module doc and commit message.

Generated-by: https://github.com/cgwalters/#llms

Whether to install the composefs backend is decided in prepare_install, from --composefs-backend or the image, but the rest of the install then re-read the CLI struct's bool for it, including baseline's partitioning for to-disk. Record the decision in State as a typed Backend instead, and use the same enum for `bootc install mount`, which had its own copy. The bool stays in InstallComposefsOpts as the parsed flag, which prepare_install still validates the other composefs options against. Prep for reporting the backend in the install result. Generated-by: AI

to-disk partitions and mounts the target itself, then handed the rest of the install a RootSetup it had filled in privately: kargs, the /boot mount spec, root UUID, device info, plus its own LUKS device for teardown. That is information an external installer calling to-filesystem never gets to pass; it has to go through the root mount spec and what bootc finds from the mounts. Two such paths drifted: to-disk also put the --karg values into the root kargs, so for ostree they ended up ahead of the install config's and kargs.d's, rather than last as for to-filesystem. So split out what to-filesystem derives from the mounted target, and have to-disk call it with the root mount spec and kargs (root=UUID or nothing for discoverable partitions, plus LUKS kargs) like any other installer would; it keeps only its own teardown state. Note the device info is now that of the root filesystem's device rather than the whole disk, as with to-filesystem; its users already resolve the backing disks from it. And a root mount spec that is empty no longer drops the other root kargs, which to-disk needs for LUKS with discoverable partitions. Generated-by: AI

Copilot

Copilot review overview

🟡 Changes recommended

Raw-FD soundness, descriptor leakage, path-base correctness, and s390x bootloader reporting must be fixed.

Review effort: Balanced
Findings: 2 · 3

Open (5)

Destination descriptors leak into installation subprocesses · New
Validate raw file descriptors before constructing BorrowedFd · New
Return zipl for composefs s390x installations · New
Report paths relative to the original target root · New
Report zipl as the installed s390x bootloader · New

What changed in this PR

Adds machine-readable installation results for installer integrations.

Changes:

Adds JSON and shell-pairs output destinations via paths or inherited FDs.
Reports deployment, image, backend, and bootloader metadata.
Adds documentation, unit tests, and installation integration coverage.

File	Description
`crates/lib/src/install/output.rs`	Implements output validation, rendering, and writing.
`crates/lib/src/install.rs`	Integrates results into installation flows.
`crates/lib/src/bootc_composefs/boot.rs`	Returns composefs deployment metadata.
`docs/src/man/bootc-install-to-filesystem.8.md`	Documents the output interface and examples.
`docs/src/man/bootc-install-to-disk.8.md`	Documents output options for disk installs.
`docs/src/man/bootc-install-to-existing-root.8.md`	Documents output options for existing-root installs.
`tmt/tests/booted/test-install-outside-container.nu`	Tests JSON output and early FD validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

cgwalters · 2026-10-02T21:17:42Z

+/// This checks that it is open and not close-on-exec: an inherited fd can't be,
+/// while everything bootc opens itself is.  That rejects an fd number the
+/// caller didn't actually pass, which may belong to e.g. the async runtime.


This is a good observation.

Fixed: once prepare_install is past its last re-exec, both the output fd and the path's directory fd are set close-on-exec, so bootupctl and friends don't inherit them. I left _BOOTC_INSTALL_OUTPUT_DIRFD in the environment: it then only names a number that isn't inherited, and removing it from our own environ means unsafe { std::env::remove_var } in a multithreaded process.

Generated-by: https://github.com/cgwalters/#llms

cgwalters · 2026-10-02T21:18:18Z

+    // SAFETY: The borrow is only used for fcntl(), which is fine for any fd number.
+    let borrowed = unsafe { BorrowedFd::borrow_raw(fd) };
+    let fdflags = rustix::io::fcntl_getfd(borrowed).with_context(|| format!("fd {fd}"))?;


This is wrong, there is no way to validate the descriptor, that's the whole reason I linked to the Rust tracking issue here

Agreed; nothing here tries to validate it. The one borrow_raw (for the fcntl that implements the inherited check) now says there's no way to uphold its contract for a command-line number, pointing at #116059.

Generated-by: https://github.com/cgwalters/#llms

    .await?;

-    Ok(())
+    Ok((deploy_id.to_hex(), postfetch.detected_bootloader))


+        deployment_path.as_str().into(),
+        format!("ostree/deploy/{stateroot}/var").into(),


+        stateroot.to_string(),
+        deployment_path.as_str().into(),
+        format!("ostree/deploy/{stateroot}/var").into(),
+        postfetch.detected_bootloader,


Options like --progress-fd take a file descriptor by number, which cannot be I/O-safe in Rust (rust-lang/rust#116059): nothing proves the number isn't owned by something else in the process. Give such options one type that does the checks that are possible at argument parsing: not stdio, open, and not close-on-exec, which an fd inherited across exec can't be while everything bootc opens itself is. --progress-fd uses it now, so a bad fd is reported up front rather than on the first progress message. Prep for the install --output-*-fd options. Generated-by: AI

Prep for `bootc install --output-pairs-*`, which renders its result like `lsblk --pairs --shell` so that scripts without jq can eval it. shlex is not used for the quoting: it quotes command arguments, leaving plain words bare and keeping a newline inside single quotes, while this format double-quotes every value as lsblk and os-release(5) do and keeps it on one line, so that it is also simple to parse without a shell. Generated-by: AI

Installers wrapping `bootc install to-filesystem` need to know where the new deployment landed, typically to drop configuration into its /etc before the first boot, and today have to dig through the ostree or composefs layout for it. bootc already knows all of that at the end of the install, so let it write a small summary: backend, stateroot, the deployment, /etc and /var paths relative to the target, bootloader, and the target image with its digest. The format is JSON or "pairs" in the style of `lsblk --pairs --shell`, so that scripts without jq can just eval it; the destination is a path (replaced atomically) or an fd inherited from the caller. These are four flags rather than a format plus a destination option because each one reads unambiguously on its own; at most one may be given. Both destinations are validated as the arguments are parsed, before anything is installed. A path's directory is opened right then and its fd carried across bootc's re-execs, since prepare_install may mount a tmpfs over /tmp and a path there would otherwise silently land in that. After the last re-exec both are made close-on-exec, so that the tools bootc runs don't hold a caller's pipe open. The options apply to to-disk and to-existing-root too, since they share the same install code; to-disk gets the result back from it the same way as for to-filesystem. Ref: bootc-dev#542 Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>

cgwalters-bot · 2026-10-02T21:26:05Z

Reworked and force-pushed (b40c5d9). Your sign-off is kept on the reworked "install: Add --output-..." commit. The four new prep commits before it still need your sign-off.

About the hidden passing: nothing flows from the install back to to-disk except the result, which it now gets the same way to-filesystem does. The passing happened on the way in. Baseline filled in the RootSetup itself (kargs, the /boot spec, the root UUID, whole-disk device info, plus its LUKS device for teardown), and an external installer can't do that. "install: Have to-disk set up its root the way to-filesystem does" changes to-disk so it hands over only a root mount spec and root kargs, like --root-mount-spec + --karg. It then derives the rest from the mounts with to-filesystem's code. That turned up one drift: to-disk also put --karg into the root kargs, so for ostree they came before the install config's and kargs.d's instead of last. Things to look at:

device_info is now the root's device, which gets resolved to disks via parents as for to-filesystem.
A udev settle runs before the mounts are inspected.
LUKS, s390x and repart /boot have no tmt coverage and weren't run.

Copilot's zipl point (s390x reports grub/systemd) and its point about the sysroot/ prefix (to-existing-root onto ostree) look valid to me. I didn't address them here and can follow up if you want.

Tested on a 16-core devspace (AMD EPYC 9V45): just validate and just unit-tests passed (bootc-lib 287, utils 25). tmt plan-23-install-outside-container passed for ostree and for composefs (grub, ext4, BLS), on b1776e3b. That differs from the pushed head only by a SAFETY comment.

Generated-by: https://github.com/cgwalters/#llms

cgwalters-bot mentioned this pull request Oct 2, 2026

install: Add --output-{json,pairs}-{path,fd} for the install result cgwalters-forge/bootc#38

Closed

github-actions Bot added area/install Issues related to `bootc install` area/documentation Updates to the documentation labels Oct 2, 2026

bootc-bot Bot requested a review from jeckersb October 2, 2026 19:30

cgwalters requested changes Oct 2, 2026

View reviewed changes

cgwalters-bot added 2 commits October 2, 2026 16:27

cgwalters requested a balanced review from Copilot October 2, 2026 21:09

Copilot started reviewing on behalf of cgwalters October 2, 2026 21:09 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

cgwalters-bot added 3 commits October 2, 2026 17:25

cgwalters-bot force-pushed the bot/install-output branch from 12a26b3 to b40c5d9 Compare October 2, 2026 21:25

		deployment_path.as_str().into(),
		format!("ostree/deploy/{stateroot}/var").into(),

Conversation

cgwalters-bot commented Oct 2, 2026

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters-bot commented Oct 2, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants