Skip to content

s390x: Support multiple hostkeys in rdcore zipl - #1754

Merged
nikita-dubrovskii merged 1 commit into
coreos:mainfrom
nikita-dubrovskii:rdcore_zipl_hostkeys
Jun 26, 2026
Merged

s390x: Support multiple hostkeys in rdcore zipl#1754
nikita-dubrovskii merged 1 commit into
coreos:mainfrom
nikita-dubrovskii:rdcore_zipl_hostkeys

Conversation

@nikita-dubrovskii

Copy link
Copy Markdown
Contributor

Allow specifying --hostkey/-k multiple times for flexible Secure Execution configurations.

Allow specifying --hostkey/-k multiple times for flexible Secure
Execution configurations.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the rdcore zipl command to support multiple hostkeys via the --hostkey or -k option, transitioning the internal representation from a single string to a vector of strings. Feedback was provided to address an edge case in src/s390x/zipl.rs where an empty hostkeys vector wrapped in Some would bypass the default /etc/se-hostkeys fallback and cause an execution failure; using a match guard was suggested to resolve this.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/s390x/zipl.rs

@dustymabe dustymabe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM - one strong-ish recommendation

Comment thread src/bin/rdcore/cmdline.rs
/// Path to hostkey
#[arg(long, value_name = "HOSTKEY")]
pub hostkey: Option<String>,
#[arg(short = 'k', long = "hostkey", value_name = "HOSTKEY")]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like long options. I won't require us to not add -k support here, but I'd recommend against it. Usually for short options (unless you use a CLI tool often by hand) you just end up looking up what it means anyway. I'd rather just keep the long option.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, long options are better. I was testing the case where many keys are required, and after typing --hostkey for the third time I ended up using the short version.

Also, pvimg-create also uses -k for a similar option:

-k, --host-key-document <FILE>
Use FILE as a host-key document. Can be specified multiple times and must be specified at least once.

@nikita-dubrovskii
nikita-dubrovskii merged commit bee4500 into coreos:main Jun 26, 2026
15 checks passed
@nikita-dubrovskii
nikita-dubrovskii deleted the rdcore_zipl_hostkeys branch June 26, 2026 13:49
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.

2 participants