firecracker icon indicating copy to clipboard operation
firecracker copied to clipboard

Published 20 hours ago • verified publisher icon firecracker-microvm

Cmdline

Open upxe opened this issue 3 years ago • 3 comments
trafficstars

Reason for This PR

Fixes #3023: FC v0.25.2 and later fails to start VMs with valid kernel command lines containing multiple -- substrings, and fails to handle correctly kernel command lines containing boot parameters starting with --.

Description of Changes

  • Add a cmdline module under vmm containing a split function that splits kernel command lines into boot parameters and init arguments, more faithfuly following the kernel's handling of -- substrings.

  • Change build_microvm_for_boot to use that function rather than str::split("--").

  • Extended integration_tests/functional/test_kernel_cmdline.py accordingly.

  • [ ] This functionality can be added in rust-vmm.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

PR Checklist

[Reviewer TODO: Verify that these criteria are met. Request changes if not]

  • [ ] All commits in this PR are signed (git commit -s).
  • [ ] The issue which led to this PR has a clear conclusion.
  • [ ] This PR follows the solution outlined in the related issue.
  • [ ] The description of changes is clear and encompassing.
  • [ ] Any required documentation changes (code and docs) are included in this PR.
  • [ ] Any newly added unsafe code is properly documented.
  • [ ] Any API changes follow the Runbook for Firecracker API changes.
  • [ ] Any user-facing changes are mentioned in CHANGELOG.md.
  • [ ] All added/changed functionality is tested.

upxe avatar Jun 01 '22 15:06 upxe

Hey, this test is failing because our integration test does not recognise your Copyright header.

Could I ask you to amend the test as well?

bchalios avatar Jun 09 '22 12:06 bchalios

done; also happy to assign copyright if that's easier for you

upxe avatar Jun 09 '22 15:06 upxe

Just to leave a note also in this PR. People interested in this issue should have a look at the related issue #3023 where we are providing regular updates on the solution taken.

xmarcalx avatar Sep 19 '22 10:09 xmarcalx

@upxe thanks a lot for your contribution! Wanted to let you know that we fixed this in rust-vmm loader (which is what Firecracker uses for cmdline interaction). We are now consuming the fix (through https://github.com/firecracker-microvm/firecracker/pull/3122) and issue is now fixed.

dianpopa avatar Oct 27 '22 13:10 dianpopa