Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not crash when checking if an interface is a hotplug one (bsc#1184623) #1203

Merged
merged 3 commits into from
Apr 13, 2021

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Apr 13, 2021

Problem

When we try to configure a virtual interface or an interface which is not present it does not have an interface associated and the hotplug check produces a crash.

Solution

  • Just do not crash when the interface is not present and return early in case of a hotplug interface.

Tests

  • Tested manually

@coveralls
Copy link

coveralls commented Apr 13, 2021

Coverage Status

Coverage remained the same at 79.678% when pulling 4b9c385 on fix_hotplug_check into 30fd929 on master.

log.info "AddressDialog res: #{ret.inspect}"

return ret if [:back, :abort].include?(ret)
return ret if builder.interface&.hotplug?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could even return if !builder.boot_protocol.dhcp?

Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

The change looks good. I was just considering whether we could simplify it a little bit more...

src/include/network/lan/address.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

It looks good to me. But perhaps @mchf has additional comments so, please, let's wait for him.

Copy link
Member

@mchf mchf left a comment

Choose a reason for hiding this comment

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

lgtm

@teclator teclator merged commit c0ea7e2 into master Apr 13, 2021
@teclator teclator deleted the fix_hotplug_check branch April 13, 2021 13:07
@yast-bot
Copy link
Contributor

✔️ Public Jenkins job #243 successfully finished
✔️ Created OBS submit request #884961

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.

None yet

5 participants