Skip to content

Conversation

lzrd
Copy link
Contributor

@lzrd lzrd commented Jun 9, 2025

Error code for use in oxidecomputer/hubris#2050

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

Some very minor nits. Otherwise, looks good!

write!(f, "invalid component for operation")
}
Self::InvalidPreferredSlotId => {
write!(f, "update to preferred boot SlotId is not permitted")
Copy link
Member

Choose a reason for hiding this comment

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

nitpicky: i find this wording a bit confusing, is it saying "the image in the preferred boot slot cannot be updated" or "the preferred slot ID cannot be updated to the value provided"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this?

            Self::InvalidPreferredSlotId => {
                write!(f, "updating a bootloader preferred slot is not permitted")
            }

Copy link
Member

Choose a reason for hiding this comment

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

much nicer!

InvalidComponent,
#[error("an image was not found")]
ImageNotFound,
#[error("cannot update preferred boot image")]
Copy link
Member

Choose a reason for hiding this comment

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

nitpicky, feel free to ignore: should this use the same string as the UpdateError variant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same string will be use in both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, but removal of the gateway-sp-comms entry makes this moot.

InvalidComponent,
#[error("an image was not found")]
ImageNotFound,
#[error("updating a bootloader preferred slot is not permitted")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should add this variant right now.

This UpdateError (unlike the one in gateway-messages) is used internally in gateway-sp-comms, so (1) we don't need an InvalidPreferredSlotId variant here in order to make progress on SP work, and (2) it's not obvious when gateway-sp-comms will produce this error.

Copy link
Contributor Author

@lzrd lzrd Jun 18, 2025

Choose a reason for hiding this comment

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

This supports a new semantic where attempts to update a RoT Hubris image that is a preferred boot image will fail with this error. See Hubris PR #2050

Copy link
Contributor

Choose a reason for hiding this comment

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

I know that, and the variant in gateway_messages::sp_to_mgs::UpdateError is necessary for hubris#2050. I'm saying that adding a variant to gateway_sp_comms::Error doesn't need to happen in this PR, because we don't have clear semantics for how it would be used.

@jgallagher jgallagher self-requested a review June 16, 2025 15:52
@lzrd lzrd force-pushed the stoltz/invalid-prep-slot branch 2 times, most recently from c5344fd to ea32256 Compare June 16, 2025 22:54
@lzrd lzrd force-pushed the stoltz/invalid-prep-slot branch from ea32256 to 76a2d83 Compare June 18, 2025 17:23
@lzrd lzrd merged commit 8cb4c31 into main Jun 18, 2025
12 checks passed
jgallagher added a commit that referenced this pull request Jun 18, 2025
jgallagher added a commit that referenced this pull request Jun 18, 2025
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.

4 participants