Fix application of network --mtu kickstart option in Anaconda #3678
Conversation
| @@ -498,6 +498,8 @@ def create_connections_from_ksdata(nm_client, network_data, device_name, ifname_ | |||
| s390cfg = get_s390_settings(device_name) | |||
| _update_wired_connection_with_s390_settings(con, s390cfg) | |||
|
|
|||
| update_connection_ethernet_settings_from_ksdata(con, network_data) | |||
There was a problem hiding this comment.
I don't understand why this is at top level of the function here, while above you decide what kind of connection this is, and one of the options seems to be explicitly this same thing - ethernet:
# type "802-3-ethernet"
if is_ethernet_device(nm_client, device_name):
(...ethernet stuff...)
update_connection_ethernet_settings_from_ksdata(con, network_data)
Maybe I'm mistaking them for each another because there isn't any indication how much ethernet-ish the new function is, except for its name?
There was a problem hiding this comment.
It applies also to virtual devices, eg bond, but as NM "802-3-ethernet" setting (it would be applied by NM to ports/slaves for virtual device connections). But your point is very good as for infiniband devices it is "infiniband" setting.
I think I should update the PR.
There was a problem hiding this comment.
It might be just a question of naming things :)
rvykydal
force-pushed
the
apply-network-mtu-in-stage2
branch
from
3a9182b
to
60e31af
Compare
| if not s_wired: | ||
| s_wired = NM.SettingWired.new() | ||
| connection.add_setting(s_wired) | ||
| s_wired.props.mtu = int(network_data.mtu) |
There was a problem hiding this comment.
Can the conversion fail? How likely is it?
There was a problem hiding this comment.
Thank you,
I hoped that such value would not make it so far and neglected this possibility, but actually it would.
Unit tests would make me think about it more:)
I'll update the PR and add unit test for the new function.
There was a problem hiding this comment.
Ok. The value should be ideally validated and converted by pykickstart if that's possible.
There was a problem hiding this comment.
Unit test added, int conversion checked.
rvykydal
force-pushed
the
apply-network-mtu-in-stage2
branch
3 times, most recently
from
e1e24f5
to
46b74cb
Compare
This was broken when application of kickstart network command was moved to Anaconda.
rvykydal
force-pushed
the
apply-network-mtu-in-stage2
branch
from
46b74cb
to
e562ec1
Compare
|
/kickstart-test --testtype smoke |
1 similar comment
|
/kickstart-test --testtype smoke |
This was broken when application of kickstart network command was moved
to Anaconda.