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

nixos/lib/make-ext4-fs: fall back to resize2fs -M if exact resize fails #79368

Merged
merged 1 commit into from Feb 9, 2020

Conversation

@sorki
Copy link
Member

@sorki sorki commented Feb 6, 2020

Workaround to fix resize2fs failures on armv7l and aarch64. I would drop the exact resizing but it seems to work fine on x86 and might affect image reproducibility. There's not much to be gained by aggressive resizing (other than reproducibility) due to zstd compression anyway. Side-note: when using sdImage the image is by default compressed again with bz2 which only causes it grow a little.

See also

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@sorki sorki requested review from samueldr and lovesegfault and removed request for samueldr Feb 7, 2020
@samueldr
Copy link
Member

@samueldr samueldr commented Feb 7, 2020

I'm now questioning the old code path. Shouldn't we prefer always using -M? Did you look it into it enough to know the drawbacks?

If it's because it would end up bigger, it looks like we could run it twice or thrice to get it a bit more snug, but at the expense of performance. Since we're now bzipping the resulting image, it won't affect the hydra build. The main issue we may have is small SD devices and small eMMCs, but that should still be under 4GiB, and certainly far under 8GiB.

@sorki
Copy link
Member Author

@sorki sorki commented Feb 7, 2020

I've tried running it 10 times in a loop (according to launchpad comment) but only the first iteration had any effect.

@samueldr
Copy link
Member

@samueldr samueldr commented Feb 7, 2020

Right, but comparing our current resize thing, with -M do you know what drawbacks there may be?

@sorki
Copy link
Member Author

@sorki sorki commented Feb 8, 2020

Not aware of any drawbacks but I'm no expert when it comes to e2fsprogs. It could affect reproducibility but it's improbable as the result mkfs + file copies should be deterministic.

echo "Failed, resizing to minimum allowed by resize2fs"
fsck.ext4 -y -f $img
resize2fs $img -M
)

This comment has been minimized.

@samueldr

samueldr Feb 8, 2020
Member

Prefer only resize once, with -M, rather than fail once, then doing it with -M.

This also means we might be able to remove some dead code around here, which is good, no?

This comment has been minimized.

@sorki

sorki Feb 9, 2020
Author Member

I'm all for it when it comes to deleting redundant code, updated with snuggly resize dropped.

@sorki sorki force-pushed the sorki:ext4resize branch from 79a38f7 to db894cb Feb 9, 2020
@samueldr
Copy link
Member

@samueldr samueldr commented Feb 9, 2020

This increases the partition size from 2.0 GiB to 2.4GiB, but considering it only adds 1MiB to the final build artifact, it's not an issue. We were not targeting 2.0GiB to fit on a small SD card, but only for hydra.

@samueldr samueldr merged commit cebb0b4 into NixOS:master Feb 9, 2020
12 checks passed
12 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.