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 modify interface name when enslaving it (bsc#1165463) #1045

Merged
merged 2 commits into from Mar 2, 2020

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Mar 2, 2020

Problem

When enslaving an interface with a physical port id it is renamed adding the description as the name.

Solution

Use a dup of the interface name.

Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

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

LGTM

I'm just thinking if we could somehow avoid a similar bug in the future. Would something like interface.name.freeze work?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 70.009% when pulling dfaf520 on do_not_modify_slave_name into e65a94c on master.

@teclator
Copy link
Contributor Author

teclator commented Mar 2, 2020

LGTM

I'm just thinking if we could somehow avoid a similar bug in the future. Would something like interface.name.freeze work?

Well, the interface name can be modified when renaming. Thus, not sure about freezing it.

@lslezak
Copy link
Member

lslezak commented Mar 2, 2020

I see, then OK.

@teclator
Copy link
Contributor Author

teclator commented Mar 2, 2020

LGTM

I'm just thinking if we could somehow avoid a similar bug in the future. Would something like interface.name.freeze work?

Thinking on it, I think that would be a good idea and at least we should do it in the constructor:

https://github.com/yast/yast-network/blob/master/src/lib/y2network/interface.rb#L77

And when renaming:

https://github.com/yast/yast-network/blob/master/src/lib/y2network/interface.rb#L112

But, will merge as it is by now if you are ok.

@teclator teclator merged commit da22984 into master Mar 2, 2020
@teclator teclator deleted the do_not_modify_slave_name branch March 2, 2020 21:07
@yast-bot
Copy link
Contributor

yast-bot commented Mar 2, 2020

✔️ Internal Jenkins job #72 successfully finished
✔️ Created IBS submit request #212985

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

4 participants