Skip to content

Conversation

ilmanzo
Copy link
Contributor

@ilmanzo ilmanzo commented Feb 15, 2023

Changes

Replace HashMap usage in seccomp with BTreeMap to preserve order of items.
This enable reproducible builds (see issue #3439)

Reason

fixes #3439

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following
Developer Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • If a specific issue led to this PR, this PR closes the issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this PR.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet contribution quality standards.

  • This functionality can be added in rust-vmm.

@roypat
Copy link
Contributor

roypat commented Feb 16, 2023

Good morning! Sorry for not getting back to you yesterday.

I did trigger the CI for you, and it seems there's some build/clippy failures in the unit tests. Could you have a look at those?

Signed-off-by: Andrea Manzini <ilmanzo@gmail.com>
@ilmanzo ilmanzo force-pushed the reproducible_builds branch from ff2afa8 to c99d5bd Compare February 16, 2023 09:46
Copy link
Contributor

@roypat roypat left a comment

Choose a reason for hiding this comment

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

LGTM! The binary files produced are indeed identical across runs now:

$ for i in $(seq 100) ; do cargo run -p seccompiler -- --input-file resource/seccomp/unimplemented.json --target-arch x86_64 --output-file x > /dev/null ; md5sum x ; done | sort | uniq -c
100 ccf7c06d325fee897449f7a3a698a243  x

@roypat roypat requested a review from a team February 16, 2023 10:54
@luminitavoicu
Copy link
Contributor

I think it would be worth adding a CHANGELOG.md entry for this, @pb8o @roypat WDYT? It should be under:
## Unreleased -> ###Changed

@roypat
Copy link
Contributor

roypat commented Feb 17, 2023

Synced with @luminitavoicu off-line and decided to merge this as is. We'll have to do some changelog work before the release anyway, so we'll just add an entry when that happens

@roypat roypat merged commit 2ad11bf into firecracker-microvm:main Feb 17, 2023
roypat added a commit to roypat/firecracker that referenced this pull request Feb 17, 2023
Added missing entries for 1.3 release
- bchalios@'s net scatter-gather improvements (firecracker-microvm#2958)
- seccompiler change to make builds reproducible (firecracker-microvm#3445)

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
roypat added a commit to roypat/firecracker that referenced this pull request Feb 17, 2023
Added missing entries for 1.3 release
- bchalios@'s net scatter-gather improvements (firecracker-microvm#2958)
- seccompiler change to make builds reproducible (firecracker-microvm#3445)

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
roypat added a commit to roypat/firecracker that referenced this pull request Feb 17, 2023
Added missing entries for 1.3 release
- bchalios@'s net scatter-gather improvements (firecracker-microvm#2958)
- seccompiler change to make builds reproducible (firecracker-microvm#3445)

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
luminitavoicu pushed a commit that referenced this pull request Feb 17, 2023
Added missing entries for 1.3 release
- bchalios@'s net scatter-gather improvements (#2958)
- seccompiler change to make builds reproducible (#3445)

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
jkrshnmenon pushed a commit to jkrshnmenon/firecracker that referenced this pull request Mar 7, 2023
Added missing entries for 1.3 release
- bchalios@'s net scatter-gather improvements (firecracker-microvm#2958)
- seccompiler change to make builds reproducible (firecracker-microvm#3445)

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
JonathanWoollett-Light pushed a commit to JonathanWoollett-Light/firecracker that referenced this pull request Mar 17, 2023
Added missing entries for 1.3 release
- bchalios@'s net scatter-gather improvements (firecracker-microvm#2958)
- seccompiler change to make builds reproducible (firecracker-microvm#3445)

Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
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.

[Improvement] Reproducible builds
4 participants