network-ng add interfaces renaming support #921
Conversation
| @@ -45,6 +45,8 @@ class Interface | |||
| attr_reader :configured | |||
| # @return [HwInfo] | |||
| attr_reader :hardware | |||
| # @return [Symbol] Mechanism to rename the interface (nil -no rename-, :bus_id or :mac) | |||
| attr_accessor :renaming_mechanism | |||
There was a problem hiding this comment.
this looks quite limiting for me as I would hope that in future we can allow to rename interface more flexible, e.g. by user provided bus_id or by different udev value, which is currently not supported ( and means that for e.g. usb attached wifi you cannot set udev name, as udev rules are empty in dialog ).
There was a problem hiding this comment.
You are right, I did not though about hotplug devices. Maybe we could add a renaming_value (provided by the user). But I would keep the renaming_mechanism so we know against which attribute we should match.
There was a problem hiding this comment.
Or we could even take those renaming_mechanism/renaming_value to a separate class.
There was a problem hiding this comment.
maybe it will be more flexible to have it in own class.
| :mac | ||
| elsif rule.parts.any? { |p| p.key == "KERNELS" } | ||
| :bus_id | ||
| end |
There was a problem hiding this comment.
don't we kill some complex udev rules there? E.g. if customer has his nice handcrafted udev rule for network card, if it survive run of yast2-network?
There was a problem hiding this comment.
The current implementations kills them anyway.
There was a problem hiding this comment.
yep, but we should aim to behave better as I see many customers use fine tuned udev rules.
| UdevRulePart.new("SUBSYSTEM", "==", "net"), | ||
| UdevRulePart.new("ACTION", "==", "add"), | ||
| UdevRulePart.new("DRIVERS", "==", "?*"), | ||
| UdevRulePart.new("ATTR{type}", "==", "1") |
There was a problem hiding this comment.
some explanation here would be nice.
src/lib/y2network/udev_rule.rb
Outdated
| # @param udev_rules [Array<UdevRule>] List of udev rules | ||
| def write(udev_rules) | ||
| Yast::SCR.Write(Yast::Path.new(".udev_persistent.rules"), udev_rules.map(&:to_s)) | ||
| Yast::SCR.Write(Yast::Path.new(".udev_persistent.nil"), []) |
There was a problem hiding this comment.
explanation for this tricky call?
There was a problem hiding this comment.
Write changes to the file. We do something similar for sysconfig. I will add a note.
There was a problem hiding this comment.
I thought it looks differently with value nil and ".udev_persistent" as path, but I can be wrong.
There was a problem hiding this comment.
I expected that too, but I learn from the old code that, in this case, it is different. I tried it manually and the "expected" version does not work.
src/lib/y2network/udev_rule_part.rb
Outdated
| end | ||
| # @return [String] Key name | ||
| attr_accessor :key | ||
| # @return [String] Operator ("==", "!=", "=", "+=", "-=", ":=") |
There was a problem hiding this comment.
important part is that first two are matching operators and rest are assign operator.
|
what I am missing here is actual renaming in internal structures. So what I mean? That if interface A is renamed to interface B, that it will correctly use name interface B everywhere and also mark properly all objects that uses modified interface as modified, so it is written down. |
The PR is not finished yet :) I am writing a method in Y2Network::Config to handle such renames and keep consistency between connections and interfaces. |
* Tests were passing but the scenarios were wrong.
This PR implements support to rename an interface using a udev rule.