Skip to content

libkrun-efi: init at 1.15.1, krunkit: init at 0.2.2 (Supersedes #416870) #417062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

libkrun-efi: init at 1.15.1, krunkit: init at 0.2.2 (Supersedes #416870) #417062

wants to merge 3 commits into from
+175 −0

Conversation

quinneden
Copy link
Contributor

@quinneden quinneden commented Jun 15, 2025 •
edited
Loading

Things done

  • Added quinneden to maintainers/maintainers-list.nix
  • Added libkrun-efi, the EFI variant of libkrun for aarch64-darwin.
  • Added krunkit, a CLI tool for running linux VMs on aarch64-darwin using libkrun-efi.
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@quinneden quinneden mentioned this pull request Jun 15, 2025
Closed
13 tasks
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Jun 15, 2025
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5606

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5633

@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Jul 4, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Jul 14, 2025
quinneden
quinneden commented Jul 14, 2025
View reviewed changes
@quinneden quinneden force-pushed the init-libkrun-efi-and-krunkit branch from 6a82cfd to 0097281 Compare July 14, 2025 20:01
booxter
booxter reviewed Aug 13, 2025
View reviewed changes
Copy link
Contributor

@booxter booxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work. I can confirm it works with podman libkrun backend to serve models through ramalama. See some comments about library linkage and GPU support flag.

booxter
booxter reviewed Aug 13, 2025
View reviewed changes
@booxter
Copy link
Contributor

booxter commented Aug 13, 2025

Oh and one more thing: please avoid adding commits like "Apply suggestions from code review" or "Delete pkgs/by-name/li/libkrun-efi/virglrenderer-override.nix" Instead, please squash these where they belong logically.

@booxter booxter mentioned this pull request Aug 15, 2025
Closed
@quinneden
Copy link
Contributor Author

Oh and one more thing: please avoid adding commits like "Apply suggestions from code review" or "Delete pkgs/by-name/li/libkrun-efi/virglrenderer-override.nix" Instead, please squash these where they belong logically.

Will do, thank you for the feedback.

@quinneden quinneden force-pushed the init-libkrun-efi-and-krunkit branch 3 times, most recently from 776652b to b8ad818 Compare August 16, 2025 09:09
booxter
booxter reviewed Aug 19, 2025
View reviewed changes
Copy link
Contributor

@booxter booxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good to me. The number of patches is a bit high for my taste, but it's subjective and whoever will actually merge the PR could clarify if it's ok as-is.

One question I have is why we do withGpu = false by default, and whether perhaps it should be the other way around - default to support GPU emulation. Otherwise all consumers (e.g. podman) will have to explicitly request it, and I'm not sure it's worth a burden. What's the drawback of enabling GPU=1 by default?

@booxter booxter mentioned this pull request Aug 19, 2025
Closed
13 tasks
@quinneden
Copy link
Contributor Author

quinneden commented Aug 19, 2025 •
edited
Loading

What's the drawback of enabling GPU=1 by default?

None that I'm aware of. I honestly don't know why I decided to default to false, but I agree that it's probably better to have it enabled.

@quinneden quinneden force-pushed the init-libkrun-efi-and-krunkit branch from c1e9694 to 97d6736 Compare August 19, 2025 04:14
@booxter
Copy link
Contributor

booxter commented Aug 20, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 417062
Commit: 97d6736f2d2a7eee5c159568cc64fe0c0f2cd99b

aarch64-darwin

✅ 3 packages built:
  • krunkit
  • libkrun-efi
  • libkrun-efi.dev

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 20, 2025
alyssais
alyssais reviewed Aug 27, 2025
View reviewed changes
@booxter booxter mentioned this pull request Aug 29, 2025
Open
booxter
booxter reviewed Sep 3, 2025
View reviewed changes
Copy link
Contributor

@booxter booxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with podman+ramalama, and it still works as expected.

@booxter
Copy link
Contributor

booxter commented Sep 3, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 417062 --package libkrun-efi.passthru.tests --package krunkit.passthru.tests
Commit: b52ff139be531c19df1ce78227c4afd5d2902a81

x86_64-linux

⏩ 6 packages marked as broken and skipped:
  • krunkit.passthru.tests.withoutGpu
  • krunkit.passthru.tests.withoutGpu.dev
  • libkrun-efi.passthru.tests.virglrenderer
  • libkrun-efi.passthru.tests.virglrenderer.debug
  • libkrun-efi.passthru.tests.withoutGpu
  • libkrun-efi.passthru.tests.withoutGpu.dev

aarch64-linux

⏩ 6 packages marked as broken and skipped:
  • krunkit.passthru.tests.withoutGpu
  • krunkit.passthru.tests.withoutGpu.dev
  • libkrun-efi.passthru.tests.virglrenderer
  • libkrun-efi.passthru.tests.virglrenderer.debug
  • libkrun-efi.passthru.tests.withoutGpu
  • libkrun-efi.passthru.tests.withoutGpu.dev

x86_64-darwin

✅ 1 package built:
  • libkrun-efi.passthru.tests.virglrenderer

aarch64-darwin

✅ 3 packages built:
  • krunkit.passthru.tests.withoutGpu
  • krunkit.passthru.tests.withoutGpu.dev (krunkit.passthru.tests.withoutGpu.dev.dev)
  • libkrun-efi.passthru.tests.virglrenderer

@booxter
Copy link
Contributor

booxter commented Sep 3, 2025

Apart from the spurious test case and commit messages not complying with guidelines, this PR looks ok to me.

(For commits, I'd prefer if this PR has just three: 1) add to maintainers; 2) add libkrun-efi; 3) add krunkit.)

@booxter
Copy link
Contributor

booxter commented Sep 3, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 417062
Commit: b52ff139be531c19df1ce78227c4afd5d2902a81

aarch64-darwin

✅ 3 packages built:
  • krunkit
  • libkrun-efi
  • libkrun-efi.dev

@quinneden quinneden force-pushed the init-libkrun-efi-and-krunkit branch 2 times, most recently from 3b0757e to 02d2f1a Compare September 3, 2025 09:53
booxter
booxter approved these changes Sep 3, 2025
View reviewed changes
Copy link
Contributor

@booxter booxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also update the title of the PR to reflect the versions.

SuperSandro2000
SuperSandro2000 reviewed Sep 3, 2025
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see the contributing guide for the ordering of meta and passthru

@quinneden quinneden changed the title libkrun-efi: init at 1.13.0, krunkit: init at 0.2.1 (Supersedes #416870) libkrun-efi: init at 1.15.0, krunkit: init at 0.2.2 (Supersedes #416870) Sep 3, 2025
@quinneden quinneden changed the title libkrun-efi: init at 1.15.0, krunkit: init at 0.2.2 (Supersedes #416870) libkrun-efi: init at 1.15.1, krunkit: init at 0.2.2 (Supersedes #416870) Sep 3, 2025
@quinneden quinneden force-pushed the init-libkrun-efi-and-krunkit branch from 02d2f1a to 55a2a63 Compare September 3, 2025 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alyssais alyssais alyssais left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

+1 more reviewer

@booxter booxter booxter approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@quinneden @nixos-discourse @booxter @alyssais @SuperSandro2000