lib: add krun_set_init_binary, backcompat via new krun_init crate by ggoodman · Pull Request #593 · containers/libkrun

ggoodman · 2026-03-17T16:30:41Z

Intent

Our goal with this PR is to prepare for a future Rust API where the main crate does not always embed or compile an init binary as part of being usable.

Today, init.krun is effectively a hard-wired implementation detail of the lower layers (especially devices), which makes sense for the current C-oriented flow, but creates friction for a slim Rust-first API design. We want to preserve the existing behavior for C consumers while moving toward a model where init payload is an explicit input, not an unconditional side effect of depending on core crates.

In short: keep the current default UX where needed, but stop forcing the init payload into every build shape.

Approach

This PR splits policy from mechanism and introduces an explicit init contract.

First, it moves default init embedding/build responsibilities into a dedicated sibling crate (krun_init) instead of keeping that logic in devices. That lets us preserve current behavior (default payload is still available) without making devices itself always own the init artifact.

Second, it changes virtio-fs passthrough to accept init payload through configuration (Option<Arc<[u8]>>) rather than via a crate-level static include. The synthetic /init.krun inode is now exposed only when payload is present, which makes the behavior explicit and composable.

Third, it replaces the stringly cmdline check with a typed boot contract. We now express the requirement as InitPolicy::InitKrunFromVirtioFs, and enforce it centrally in vmm::builder. This keeps validation robust and avoids heuristic coupling to command-line text parsing.

Fourth, on the C side, it keeps backward-compatible defaults by auto-wiring the default init payload, and additionally introduces a runtime override API (krun_set_init_binary) so callers can inject their own init bytes for a context.

Trade-offs

The main benefit is architectural clarity: core crates no longer implicitly force init embedding semantics, and the contract around /init.krun is explicit and validated in one place. This also gives us a clean migration path toward a Rust API where init choice is deliberate.

The cost is extra plumbing and a slightly larger configuration surface (init_policy, payload propagation through fs config). We also introduce another crate (krun_init) in the dependency graph, which is intentional but still additional structure to maintain.

Overall, this is a deliberate trade: modest complexity increase now in exchange for a cleaner long-term API boundary and less hidden policy in shared internals.

tylerfanelli · 2026-03-17T23:39:31Z

include/libkrun.h

+ * Returns:
+ *  Zero on success or a negative error number on failure.
+ */
+int32_t krun_set_init_binary(uint32_t ctx_id, const uint8_t *init_binary, size_t init_binary_len);


krun_set_init would be a bit more succinct.

Suggested change

int32_t krun_set_init_binary(uint32_t ctx_id, const uint8_t *init_binary, size_t init_binary_len);

int32_t krun_set_init(uint32_t ctx_id, const uint8_t *bin, size_t size);

tylerfanelli · 2026-03-18T00:09:52Z

src/devices/src/virtio/fs/macos/passthrough.rs

    /// Table of exported FDs to share with other subsystems. Not supported for macos.
    pub export_table: Option<ExportTable>,
    pub allow_root_dir_delete: bool,
+    pub init_payload: Option<Arc<[u8]>>,


Should this perhaps be an Arc<[u8]> rather than Option<Arc<[u8]>>, in which if there is no init binary set, we fall back to the default?

Wouldn't that result in any binary built off this now forced to include the bytes of the default init? My intention here is to avoid that by putting it in another compilation unit and forcing the user to provide it.

But to maintain parity with the c API, I did the default fallback.

Yea, I guess there is no way to "fall back" without including the bytes within the compilation.

But are we requiring that an init binary always be set now?

If we are that is not the intention. My intention was as follows:

For the current c api, always fall back to the current init.

For the speculative rust api, require that an init binary be configured. Document that the built-in init binary is in the libkrun_init crate.

The new fallback mechanism in the c api works based on this: https://github.com/ggoodman/libkrun/blob/b9881fd56291a9c67ebd556ec3cd4177536166ee/src/libkrun/src/lib.rs#L92

tylerfanelli · 2026-03-18T00:15:32Z

src/devices/src/virtio/fs/linux/passthrough.rs

+            if off >= init_payload.len() {
+                return Ok(0);
+            }


Can you explain why zero is returned?

Added a comment. Read past EOF should return 0 bytes read.

tylerfanelli · 2026-03-18T00:16:07Z

src/devices/src/virtio/fs/macos/passthrough.rs

+            if off >= init_payload.len() {
+                return Ok(0);
+            }


Ditto here, why return zero?

Added a comment. Read past EOF should return 0 bytes read.

d-e-s-o · 2026-03-18T03:33:18Z

src/libkrun/src/lib.rs

+        return -libc::EINVAL;
+    }
+
+    let payload = Arc::<[u8]>::from(slice::from_raw_parts(init_binary, init_binary_len));


I feel a bit uneasy about this potentially sizable copy (and heap allocation; for something that with great likelihood is already statically included in the binary anyway). It strikes me as something we should try and avoid, if at all possible. Have you considered replacing the Arc<[u8]> construct with a &'static [u8]? Would it impede usability significantly?

I think, I would prefer the &'static [u8] too.

'static is currently the correct lifetime, because we exit the process once VM quits, but this will probably change in the future and this may become more unsafe. For a C API like we have now, having a huge static buffer be caller owned is reasonable behavior IMO. (I wonder though if it will be possible to design the Rust API so it's possible to name a 'vmm lifetime...?)

OK, so I have tried addressing this in fcd6927 without trying to pretend to own the memory range supplied to the c api by using an internal enum of owned vs static byte slices. The init binary that is now embedded in krun_init is of the static variant allowing a zero-copy use-case. However, if folks supply an init binary via krun_set_init(), it will be treated as owned and will result in a copy.

Later, if we have a native rust api, this will allow embedders of libkrun to use the include_bytes macro and the Static() variant if they want.

'static is currently the correct lifetime, because we exit the process once VM quits, but this will probably change in the future and this may become more unsafe.

I'd think 'static would be correct no matter what, right? It would just over-constrain. However, I don't think you can have non-static lifetimes on global data in Rust (and at least currently there is still a bunch of global state, it seems).

That being said, the most flexibility for users (and, hence, future-proof API design) would perhaps be achieved be some custom enum ala:

enum InitBinary { Static(&'static [u8]), Dynamic(Arc<[u8]>), }

Personally I'd probably still err on the side of just using &'static [u8] to force callers to think about what they are doing, but I can't anticipate how this API will be used most frequently.

OK, so I have tried addressing this in fcd6927 without trying to pretend to own the memory range supplied to the c api by using an internal enum of owned vs static byte slices.

Thanks! Do we still need the LazyLock thingy? Constructing an enum variant with static data in a const context should be fine, but haven't tried removing.

Ah, we misunderstood each other.
By "caller owned" I meant that caller owns the memory (possibly dynamically allocated or their static memory) and guarantees it will exist during the VMM lifetime. In that case krun_set_init doesn't need to copy it (so it actually corresponds to your static case).

Both have pros and cons, I am not really in strongly in favor of one or the other.

By "caller owned" I meant that caller owns the memory (possibly dynamically allocated or their static memory) and guarantees it will exist during the VMM lifetime.

Personally, I'd also prefer these semantics.

Oh I see. I did not anticipate that as the default.

In that case, perhaps the union type is not necessary and the ownership requirements can simply be documented on the c api.

It would state that the caller guarantees that the memory will outlive the vm lifetime.

So that leaves some options:

Status quo.

Keep the union type with the expectation that ::Owned() may be helpful for some use-cases of a future rust api _but switch the new c api method to assume ::Static().

Rip out the whole union type and switch everything conceptually to the ::Static().

Rip out the whole union type and switch everything conceptually to the ::Static().

I'd go for this

...then so shall it be (as of b89d788)!

ggoodman · 2026-04-02T13:12:34Z

@mtjhrc / @slp is this something you're willing to entertain in 1.x?

mtjhrc · 2026-04-07T11:43:46Z

Personally, I would be ok with this for 1.18 (last feature release of 1.x branch), because the feature is small and isolated. Please squash the commits and directly introduce just the final simple &'static version only. (instead of the Arc/lazylock/enum sidequests).

ggoodman · 2026-04-07T23:58:09Z

Please squash the commits and directly introduce just the final simple &'static version only. (instead of the Arc/lazylock/enum sidequests).

Rebased and pushed.

I saw that you merged the crate PR (#588) and I strongly believe that this needs to land in the same version as that or it will result in a breaking change.

mtjhrc · 2026-04-08T10:32:56Z

src/libkrun/src/lib.rs

+    match CTX_MAP.lock().unwrap().entry(ctx_id) {
+        Entry::Occupied(mut ctx_cfg_entry) => {
+            let ctx_cfg = ctx_cfg_entry.get_mut();
+            ctx_cfg.set_init_binary(payload);
+
+            for fs_cfg in &mut ctx_cfg.vmr.fs {
+                fs_cfg.init_payload = Some(payload);
+            }
+        }
+        Entry::Vacant(_) => return -libc::ENOENT,
+    }


Instead of having to save the init_binary in both the ctx_cfg and fs_cfg, I would suggest this just sets the init payload for fs device with tag "/dev/root".
If such fs device doesn't exist this returns an error.

@mtjhrc, I took a stab at making the suggested change and came to the conclusion that the juice is not worth the squeeze.

The main downside is that it would result in krun_set_init becoming order-dependent; embedders would need to have set up their root filesystem before calling krun_set_init. I think these two operations are probably conceptually orthogonal from a user's perspective so the order-dependence creates an unnecessary failure mode.

That being said, I'm not at all an expert on this codebase so maybe I'm not seeing the better approach or I'm projecting constraints that the maintainers don't impose upon themselves.

LMK.

CI failure a flake? I can't re-run the tests myself.

I think these two operations are probably conceptually orthogonal from a user's perspective so the order-dependence creates an unnecessary failure mode

Yeah that is a valid point too. Yeah I guess it's fine currently, we'll change it in libkrun 2.x anyway.

The current API is a mess, some things are order dependent some are not. Ideally this would take a handle to some sort of filesystem device object (some function already work on handles, well IDs e.g. krun_add_console_port_* ), but filesystems creation functions don't return the ID (index).

CI failure a flake? I can't re-run the tests myself.

Yes some sort of race condition causes an ENOMEM in the guest kernel (we had a similar issue before related to host kernel, but this seems different). Unrelated to this PR.

d-e-s-o · 2026-04-08T14:05:07Z

src/devices/src/virtio/fs/linux/passthrough.rs

    }

+    fn init_payload(&self) -> io::Result<&[u8]> {
+        self.cfg.init_payload.ok_or_else(ebadf)


Is EBADF really an appropriate error code to return here? I feel like ENOENT is more common for this kind of condition, perhaps ENOMEDIUM if we'd want something more unique.

Done in e129fd6. Will rebase if you confirm that it looks good to you, @d-e-s-o.

Looks good as well, thanks!

Rebased. Thanks.

d-e-s-o · 2026-04-08T14:08:22Z

src/krun_init/build.rs


    if !status.success() {
-        panic!("failed to compile init/init.c: {status}");
+        return None;


I'd probably keep the existing behavior: if the build fails that should be signaled with a dedicated error message, in my opinion. That decouples failure from optionality more cleanly, I'd say.

Agree. Working on it.

I'll rebase this if it looks good. I kept it separate to make it easy for you to review in isolation: 5821070

Seems fine to me, thanks!

Rebased. Thanks.

d-e-s-o · 2026-04-08T14:11:26Z

src/libkrun/src/lib.rs

+    #[cfg(not(feature = "tee"))]
+    fn set_init_binary(&mut self, init_binary: &'static [u8]) {
+        self.init_payload = Some(init_binary);
+    }
+
+    #[cfg(not(feature = "tee"))]
+    fn get_init_binary(&self) -> &'static [u8] {
+        self.init_payload.unwrap_or(DEFAULT_INIT_PAYLOAD)
+    }
+


Not familiar with the tee feature per-se, but just pointing out that Cargo features should be additive, or problems await. It seems that is violated here, because enabling of a feature removes functionality. Perhaps it's fine if it's a niche thing for a very specific use case (or perhaps there is precedent in the repository already), but just pointing that out.

Yes it violates it here, but unfortunately so do many other places in libkrun code base. Yes this means this will cause problems if multiple crates in the compilation use libkrun and features get unified 🫤 Also I think some function might do something different based on features being enabled. This is something we need to fix, but I don't us being able to do this in libkrun 1.x.

ggoodman · 2026-04-08T15:12:24Z

Need me to rebase off main?

Signed-off-by: Geoffrey Goodman <geoff@goodman.dev>

ggoodman requested review from MatiasVara, dorindabassey, jakecorrenti, mtjhrc, slp and tylerfanelli as code owners March 17, 2026 16:30

This was referenced Mar 17, 2026

Rust API #494

Draft

libkrun: Produce a proper Rust library #588

Merged

ggoodman marked this pull request as draft March 17, 2026 19:44

tylerfanelli reviewed Mar 18, 2026

View reviewed changes

ggoodman force-pushed the feat/init-api branch 4 times, most recently from 1bdb760 to f5b0594 Compare March 18, 2026 02:28

ggoodman marked this pull request as ready for review March 18, 2026 02:33

d-e-s-o reviewed Mar 18, 2026

View reviewed changes

ggoodman force-pushed the feat/init-api branch from b89d788 to d2660d7 Compare April 7, 2026 23:56

mtjhrc reviewed Apr 8, 2026

View reviewed changes

d-e-s-o reviewed Apr 8, 2026

View reviewed changes

ggoodman force-pushed the feat/init-api branch 2 times, most recently from e129fd6 to ab92f2b Compare April 8, 2026 14:47

lib: add krun_set_init_binary, backcompat via new krun_init crate

84dd383

Signed-off-by: Geoffrey Goodman <geoff@goodman.dev>

mtjhrc force-pushed the feat/init-api branch from ab92f2b to 84dd383 Compare April 10, 2026 12:14

	int32_t krun_set_init_binary(uint32_t ctx_id, const uint8_t *init_binary, size_t init_binary_len);
	int32_t krun_set_init(uint32_t ctx_id, const uint8_t *bin, size_t size);

Conversation

ggoodman commented Mar 17, 2026

Intent

Approach

Trade-offs

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!

d-e-s-o Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggoodman Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mtjhrc Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ggoodman commented Apr 2, 2026

Uh oh!

mtjhrc commented Apr 7, 2026

Uh oh!

ggoodman commented Apr 7, 2026

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!

d-e-s-o Mar 18, 2026 •

edited

Loading

ggoodman Mar 18, 2026 •

edited

Loading

mtjhrc Mar 18, 2026 •

edited

Loading