prep work / enhancements to kernel_cmdline #1723
Conversation
This is an obvious thing one might want to do. Ensures we can make an owned Cmdline directly from an owned String. Signed-off-by: John Eckersberg <jeckersb@redhat.com>
In some cases it will be important to differentiate whether we added a net-new parameter vs. modifying an existing parameter. Motivated by trying to update bootc_kargs to use kernel_cmdline. We want to parse the booted kargs, and then merge with any args defined in kargs.d. But it should error if a value in kargs.d already exists in the booted system with a different value. The previous API did not provide enough visibility to make this determiniation. Signed-off-by: John Eckersberg <jeckersb@redhat.com>
This makes it ergonomic to take two `Cmdline`s and update one by appending the parameters in the other. Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Add a new `add` method to both `bytes::Cmdline` and `utf8::Cmdline` that appends parameters without modifying existing ones. Unlike `add_or_modify`, this method preserves duplicate keys with different values (e.g., multiple `console=` parameters), which is valid for certain kernel parameters. The method returns `Action::Added` when a new parameter is appended, and `Action::Existed` when the exact parameter already exists. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Implement Deref / AsRef / Clone in a few places where it makes sense. Signed-off-by: John Eckersberg <jeckersb@redhat.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements to the kernel_cmdline crate, including a new add method, Extend and IntoIterator implementations for Cmdline, and changing add_or_modify to return a more descriptive Action enum. The changes are well-tested and improve the usability and functionality of the crate. My main feedback is a suggestion to improve the performance of the Extend implementation, which could be important for the new use cases this PR is preparing for.
| Action::Added | ||
| )); | ||
| let mut iter = kargs.iter(); | ||
| assert_eq!(iter.next(), Some(param("console=tty0"))); |
There was a problem hiding this comment.
Minor but I think we could use https://docs.rs/itertools/latest/itertools/fn.equal.html
There was a problem hiding this comment.
Shockingly it looks like we don't (directly) use itertools anywhere in bootc right now.
There was a problem hiding this comment.
Yep totally fine, we can do that kind of stuff as async followup
Signed-off-by: John Eckersberg <jeckersb@redhat.com>
Signed-off-by: John Eckersberg <jeckersb@redhat.com>
2 participants
See individual commits, this is mostly stuff to prepare for bootc_kargs to switch over to using kernel_cmdline for all of its parsing.