Skip to content
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.

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

Minifiy nvidia jetson orin image #64

Closed
wants to merge 0 commits into from

Conversation

remimimimimi
Copy link
Contributor

@remimimimimi remimimimimi commented Jan 31, 2023

Related to #45.
I reduced the image size from 1.9GB to 1.2GB by configuring qemu, systemd, and git. Next step will be apply @vadika kernel config.

tpmSupport = false; # XXX: Probably shouldn't be disabled
hostCpuOnly = true;
};
git = super.git.override {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see git on image via modules/development/essentials.nix but this is good for now.

We can mark it as # XXX (or # TODO:) for now.

On most minimal host profile git should not be needed - even for updates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve done overlay for git because of microvm. It uses it internally in the host nixos module, and with this overlay, image size was reduced by 200mb. So it is actually unrelated to the development environment, but if you want to add git to devenv you should be aware of this change.

It also can be done with a little hack for microvm module, by overlaying packages especially for it. But for now, let it be this way.

targets/default.nix Outdated Show resolved Hide resolved
@vilvo
Copy link
Contributor

vilvo commented Feb 1, 2023

Related to #45. I reduced the image size from 1.9GB to 1.2GB by configuring qemu, systemd, and git. Next step will be apply @vadika kernel config.

Super nice work. I think we can remove "Draft" from this and go and review and merge this. More minimal kernel config warrants a PR of its' own. Same for the # XXX-ideas.

@remimimimimi remimimimimi marked this pull request as ready for review February 6, 2023 15:45
@vilvo
Copy link
Contributor

vilvo commented Feb 8, 2023

It definitely reduces the image size:

ghaf (54521a6) [?] # this PR
❯ ls -lah result/nixos.img
Permissions Size User Date Modified Name
.r--r--r--  5.1G root  1 Jan  1970  result/nixos.img

ghaf (197f9dc) [?] # main
❯ ls -lah result/nixos.img
Permissions Size User Date Modified Name
.r--r--r--  5.7G root  1 Jan  1970  result/nixos.img

As of now, "This branch cannot be rebased due to conflicts"
so please rebase your fork branch with the ghaf-repo main and resolve conflicts.

While at it, I'd prefer you squash the commits in your PR and also please add DCO (Developer Certificate of Origin) with signed-off-by: to the PR commit(s).

@remimimimimi remimimimimi force-pushed the minification branch 5 times, most recently from f89d170 to 6ca73a7 Compare February 9, 2023 04:39
Copy link
Contributor

@vilvo vilvo left a comment

Choose a reason for hiding this comment

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

Looks good to me. Basics work - boots, ssh, netvm starts. Reduces the image and host size.
None disabled features are critical for now but we may want to come back to TPM support and such later.

@vilvo vilvo requested a review from mikatammi February 10, 2023 09:21
@unbel13ver unbel13ver self-requested a review February 20, 2023 15:29
Copy link
Contributor

@unbel13ver unbel13ver left a comment

Choose a reason for hiding this comment

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

Great work, approved.
The only note: probably, your work should be rebased on top of @mikatammi 's "The great refactor" first.

@mikatammi
Copy link
Contributor

In general I'd like to say good work and nice investigation

Now that the_great_refactor has been merged, I'd put the import of ../configurations/host/minimal.nix into the targets/common-release.nix

@mikatammi
Copy link
Contributor

One thing I've noticed during my builds, is that both qemu and qemu-host-cpu-only -packages get built, even though the ghaf currently doesn't even use QEMU. So it shouldn't be building QEMU at all but for some reason it does. I haven't checked the dependency tree to see what pulls it in

@vilvo
Copy link
Contributor

vilvo commented Feb 21, 2023

One thing I've noticed during my builds, is that both qemu and qemu-host-cpu-only -packages get built, even though the ghaf currently doesn't even use QEMU. So it shouldn't be building QEMU at all but for some reason it does. I haven't checked the dependency tree to see what pulls it in

I would not optimize QEMU out from development supporting packages. It's still the swiss-knife of VMMs to test KVM supported functionality when somethings do not work out-of-the box with other VMMs.

@vilvo
Copy link
Contributor

vilvo commented Feb 22, 2023

In general I'd like to say good work and nice investigation

Now that the_great_refactor has been merged, I'd put the import of ../configurations/host/minimal.nix into the targets/common-release.nix

Please go ahead. @remimimimimi 's work is still early and I have not heard updates from it for a while. ../configurations/host/minimal.nix can be a placeholder for that work and support the general ghaf macro structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants