diff --git a/package/yast2-network.changes b/package/yast2-network.changes index 5d2c2613f..c394e1911 100644 --- a/package/yast2-network.changes +++ b/package/yast2-network.changes @@ -1,3 +1,14 @@ +------------------------------------------------------------------- +Mon Jun 10 11:55:48 UTC 2019 - Knut Anderssen + +- bsc#1136929 + - Use the hwinfo permanent mac address instead of the current mac + which could be wrong in case of an enslaved interface. +- bsc#1137324 + - Do not create duplicate udev rule attributes when editing an + interface name. +- 4.0.50 + ------------------------------------------------------------------- Mon May 20 14:04:47 UTC 2019 - Michal Filka diff --git a/package/yast2-network.spec b/package/yast2-network.spec index 5ab6476df..f1eb42d27 100644 --- a/package/yast2-network.spec +++ b/package/yast2-network.spec @@ -17,7 +17,7 @@ Name: yast2-network -Version: 4.0.49 +Version: 4.0.50 Release: 0 BuildArch: noarch diff --git a/src/include/network/lan/complex.rb b/src/include/network/lan/complex.rb index 02084b2df..876813751 100644 --- a/src/include/network/lan/complex.rb +++ b/src/include/network/lan/complex.rb @@ -309,8 +309,9 @@ def UpdateSlaves case LanItems.GetDeviceType(current) when "bond" LanItems.startmode = "hotplug" - - LanItems.update_item_udev_rule!(:bus_id) + if LanItems.current_udev_rule.empty? || LanItems.GetItemUdev("KERNELS").empty? + LanItems.update_item_udev_rule!(:bus_id) + end when "br" LanItems.ipaddr = "" end diff --git a/src/include/network/routines.rb b/src/include/network/routines.rb index 843d8b37a..a8df7cc5f 100644 --- a/src/include/network/routines.rb +++ b/src/include/network/routines.rb @@ -629,6 +629,7 @@ def ReadHardware(hwtype) one["parent_busid"] = one["sysfs_id"].split("/")[-2] end one["mac"] = Ops.get_string(resource, ["hwaddr", 0, "addr"], "") + one["permanent_mac"] = Ops.get_string(resource, ["phwaddr", 0, "addr"], "") # is the cable plugged in? nil = don't know one["link"] = Ops.get(resource, ["link", 0, "state"]) diff --git a/src/lib/network/edit_nic_name.rb b/src/lib/network/edit_nic_name.rb index b92c5fed1..000c58625 100644 --- a/src/lib/network/edit_nic_name.rb +++ b/src/lib/network/edit_nic_name.rb @@ -14,6 +14,11 @@ class EditNicName include UIShortcuts include I18n + # @return [String] current udev name before modifying it + attr_reader :old_name + # @return [String] current udev match criteria + attr_reader :old_key + # udev rule attribute for MAC address MAC_UDEV_ATTR = "ATTR{address}".freeze @@ -26,13 +31,16 @@ def initialize Yast.include self, "network/routines.rb" current_item = LanItems.getCurrentItem + current_rule = LanItems.current_udev_rule @old_name = LanItems.current_udev_name - @old_key = MAC_UDEV_ATTR unless LanItems.GetItemUdev(MAC_UDEV_ATTR).empty? - @old_key = BUSID_UDEV_ATTR unless LanItems.GetItemUdev(BUSID_UDEV_ATTR).empty? + unless current_rule.empty? + @old_key = :mac unless LanItems.GetItemUdev(MAC_UDEV_ATTR).empty? + @old_key = :bus_id unless LanItems.GetItemUdev(BUSID_UDEV_ATTR).empty? + end if current_item["hwinfo"] - @mac = current_item["hwinfo"]["mac"] + @mac = current_item["hwinfo"]["permanent_mac"] @bus_id = current_item["hwinfo"]["busid"] else @mac = "" @@ -67,12 +75,12 @@ def run # FIXME: it changes udev key used for device identification # and / or its value only, name is changed elsewhere - LanItems.update_item_udev_rule!(udev_type) + LanItems.update_item_udev_rule!(udev_type) if udev_type && (old_key != udev_type) end close - new_name || @old_name + new_name || old_name end private @@ -118,11 +126,8 @@ def open ) ) - case @old_key - when MAC_UDEV_ATTR - UI.ChangeWidget(Id(:udev_type), :CurrentButton, :mac) - when BUSID_UDEV_ATTR - UI.ChangeWidget(Id(:udev_type), :CurrentButton, :bus_id) + if old_key + UI.ChangeWidget(Id(:udev_type), :CurrentButton, old_key) else Builtins.y2error("Unknown udev rule.") end diff --git a/src/lib/network/network_autoyast.rb b/src/lib/network/network_autoyast.rb index f84daf205..b6e42025c 100644 --- a/src/lib/network/network_autoyast.rb +++ b/src/lib/network/network_autoyast.rb @@ -339,7 +339,7 @@ def assign_udevs_to_devs(udev_rules) busid = i["hwinfo"]["busid"] # Match also parent busid if exist (bsc#1129012) parent_busid = i["hwinfo"]["parent_busid"] || busid - mac = i["hwinfo"]["mac"] + mac = i["hwinfo"]["permanent_mac"] [busid, parent_busid, mac].any? { |v| v.casecmp(key).zero? } end diff --git a/src/modules/LanItems.rb b/src/modules/LanItems.rb index a93c736cd..7ccd21e0c 100644 --- a/src/modules/LanItems.rb +++ b/src/modules/LanItems.rb @@ -258,6 +258,13 @@ def GetDeviceName(item_id) "" # this should never happen end + # Convenience method to obtain the current Item udev rule + # + # @return [Array] Item udev rule + def current_udev_rule + LanItems.GetItemUdevRule(LanItems.current) + end + # Returns name which is going to be used in the udev rule def current_udev_name if LanItems.current_renamed? @@ -355,7 +362,7 @@ def InitItemUdevRule(item_id) udev = GetItemUdevRule(item_id) return udev if !udev.empty? - default_mac = GetLanItem(item_id).fetch("hwinfo", {})["mac"] + default_mac = GetLanItem(item_id).fetch("hwinfo", {})["permanent_mac"] raise ArgumentError, "Cannot propose udev rule - NIC not present" if !default_mac default_udev = GetDefaultUdevRule( @@ -415,7 +422,7 @@ def getUdevFallback if IsEmpty(udev_rules) udev_rules = GetDefaultUdevRule( GetCurrentName(), - Ops.get_string(getCurrentItem, ["hwinfo", "mac"], "") + Ops.get_string(getCurrentItem, ["hwinfo", "permanent_mac"], "") ) Builtins.y2milestone( "No Udev rules found, creating default: %1", @@ -437,13 +444,11 @@ def GetItemUdev(key) # @return [Object, nil] the current item's udev rule without the given key; nil if # there is not udev rules for the current item def RemoveItemUdev(key) - current_rule = LanItems.GetItemUdevRule(LanItems.current) + return nil if current_udev_rule.empty? - return nil if current_rule.empty? - - log.info("Removing #{key} from #{current_rule}") + log.info("Removing #{key} from #{current_udev_rule}") Items()[@current]["udev"]["net"] = - LanItems.RemoveKeyFromUdevRule(current_rule, key) + LanItems.RemoveKeyFromUdevRule(current_udev_rule, key) end # Updates the udev rule of the current Lan Item based on the key given @@ -456,8 +461,13 @@ def RemoveItemUdev(key) # @param based_on [Symbol] principal key to be matched, `:mac` or `:bus_id` # @return [void] def update_item_udev_rule!(based_on = :mac) + new_rule = current_udev_rule.empty? + LanItems.InitItemUdevRule(LanItems.current) if new_rule + case based_on when :mac + return if new_rule + LanItems.RemoveItemUdev("ATTR{dev_port}") # FIXME: While the user is able to modify the udev rule using the @@ -467,7 +477,7 @@ def update_item_udev_rule!(based_on = :mac) LanItems.ReplaceItemUdev( "KERNELS", "ATTR{address}", - LanItems.getCurrentItem.fetch("hwinfo", {}).fetch("mac", "") + LanItems.getCurrentItem.fetch("hwinfo", {}).fetch("permanent_mac", "") ) when :bus_id # Update or insert the dev_port if the sysfs dev_port attribute is present @@ -506,7 +516,6 @@ def ReplaceItemUdev(replace_key, new_key, new_val) # = for assignment # == for equality checks operator = new_key == "NAME" ? "=" : "==" - current_rule = GetItemUdevRule(@current) rule = RemoveKeyFromUdevRule(getUdevFallback, replace_key) # NAME="devname" has to be last in the rule. @@ -516,7 +525,7 @@ def ReplaceItemUdev(replace_key, new_key, new_val) new_rule = AddToUdevRule(rule, "#{new_key}#{operator}\"#{new_val}\"") new_rule.push(name_tuple) - if current_rule.sort != new_rule.sort + if current_udev_rule.sort != new_rule.sort SetModified() log.info("ReplaceItemUdev: new udev rule = #{new_rule}") @@ -1052,7 +1061,7 @@ def getDeviceName(oldname) hardware.each do |hw| hw_dev_name = hw["dev_name"] || "" - hw_dev_mac = hw["mac"] || "" + hw_dev_mac = hw["permanent_mac"] || "" hw_dev_busid = hw["busid"] || "" case oldname @@ -1522,11 +1531,11 @@ def BuildLanOverview conn = HTML.Bold(format("(%s)", _("Not connected"))) if !item_hwinfo["link"] conn = HTML.Bold(format("(%s)", _("No hwinfo"))) if item_hwinfo.empty? - mac_dev = HTML.Bold("MAC : ") + item_hwinfo["mac"].to_s + "
" + mac_dev = HTML.Bold("MAC : ") + item_hwinfo["permanent_mac"].to_s + "
" bus_id = HTML.Bold("BusID : ") + item_hwinfo["busid"].to_s + "
" physical_port_id = HTML.Bold("PhysicalPortID : ") + physical_port_id(ifcfg_name) + "
" - rich << " " << conn << "
" << mac_dev if IsNotEmpty(item_hwinfo["mac"]) + rich << " " << conn << "
" << mac_dev if IsNotEmpty(item_hwinfo["permanent_mac"]) rich << bus_id if IsNotEmpty(item_hwinfo["busid"]) rich << physical_port_id if physical_port_id?(ifcfg_name) # display it only if we need it, don't duplicate "ifcfg_name" above diff --git a/test/edit_nic_name_test.rb b/test/edit_nic_name_test.rb index 98fa21386..f458d9719 100755 --- a/test/edit_nic_name_test.rb +++ b/test/edit_nic_name_test.rb @@ -19,7 +19,7 @@ module Yast stub_const("NetworkInterfaces", double(adapt_old_config!: nil)) # mock devices configuration - allow(LanItems).to receive(:ReadHardware) { [{ "dev_name" => CURRENT_NAME, "mac" => "00:01:02:03:04:05" }] } + allow(LanItems).to receive(:ReadHardware) { [{ "dev_name" => CURRENT_NAME, "permanent_mac" => "00:01:02:03:04:05" }] } allow(LanItems).to receive(:getNetworkInterfaces) { [CURRENT_NAME] } allow(LanItems).to receive(:GetItemUdev) { "" } allow(LanItems).to receive(:GetItemUdev).with("NAME") { CURRENT_NAME } @@ -57,6 +57,10 @@ module Yast expect(@edit_name_dlg.run).to be_equal CURRENT_NAME end + + it "does not touch the current udev rule" do + expect(Yast::LanItems).to_not receive(:update_item_udev_rule!) + end end context "when closed after name change" do @@ -104,6 +108,32 @@ module Yast expect(@edit_name_dlg.run).to eql NEW_NAME end + + context "but used the same matching udev key" do + it "does not touch the current udev rule" do + expect(Yast::LanItems).to_not receive(:update_item_udev_rule!) + end + end + end + + context "when closed without changing the match selection" do + before(:each) do + # emulate UI work + allow(UI).to receive(:QueryWidget).with(:dev_name, :Value) { CURRENT_NAME } + allow(UI).to receive(:QueryWidget).with(:udev_type, :CurrentButton) { :mac } + end + end + + context "when modified the matching udev key" do + before(:each) do + # emulate UI work + allow(UI).to receive(:QueryWidget).with(:dev_name, :Value) { CURRENT_NAME } + allow(UI).to receive(:QueryWidget).with(:udev_type, :CurrentButton) { :bus_id } + end + + it "updates the current udev rule with the key used" do + expect(Yast::LanItems).to_not receive(:update_item_udev_rule!).with(:bus_id) + end end end end diff --git a/test/lan_items_helpers_test.rb b/test/lan_items_helpers_test.rb index ce570d664..43b58d56a 100755 --- a/test/lan_items_helpers_test.rb +++ b/test/lan_items_helpers_test.rb @@ -149,8 +149,8 @@ def udev_rule(mac, name) }, 1 => { "hwinfo" => { - "mac" => "00:00:00:00:00:01", - "dev_name" => "eth1" + "permanent_mac" => "00:00:00:00:00:01", + "dev_name" => "eth1" }, # always exists "udev" => { @@ -198,7 +198,7 @@ def check_GetItemUdev(key, expected_value) before(:each) do allow(Yast::LanItems) .to receive(:GetLanItem) - .and_return("hwinfo" => { "mac" => MAC }, "ifcfg" => "eth0") + .and_return("hwinfo" => { "permanent_mac" => MAC }, "ifcfg" => "eth0") end it "returns proper value when key exists" do @@ -242,8 +242,24 @@ def check_GetItemUdev(key, expected_value) end end +describe "#current_udev_rule" do + let(:busid) { "0000:08:00.0" } + let(:hwinfo) { { "dev_name" => "test0", "busid" => busid, "permanent_mac" => "24:be:05:ce:1e:91" } } + let(:udev_net) { ["KERNELS==\"#{busid}\"", "NAME=\"test0\""] } + let(:rule) { { 0 => { "hwinfo" => hwinfo, "udev" => { "net" => udev_net } } } } + + before do + Yast::LanItems.Items = rule + Yast::LanItems.current = 0 + end + + it "returns the current item udev rule" do + expect(Yast::LanItems.current_udev_rule).to contain_exactly("KERNELS==\"#{busid}\"", "NAME=\"test0\"") + end +end + describe "#update_item_udev_rule!" do - let(:hwinfo) { { "dev_name" => "test0", "busid" => "0000:08:00.0", "mac" => "24:be:05:ce:1e:91" } } + let(:hwinfo) { { "dev_name" => "test0", "busid" => "0000:08:00.0", "permanent_mac" => "24:be:05:ce:1e:91" } } let(:udev_net) { ["ATTR{address}==\"24:be:05:ce:1e:91\"", "KERNEL==\"eth*\"", "NAME=\"test0\""] } let(:rule) { { 0 => { "hwinfo" => hwinfo, "udev" => { "net" => udev_net } } } } diff --git a/test/lan_udev_auto_test.rb b/test/lan_udev_auto_test.rb index 2aa5cf6cb..c3e7f2ff7 100755 --- a/test/lan_udev_auto_test.rb +++ b/test/lan_udev_auto_test.rb @@ -26,9 +26,10 @@ allow(lan_items).to receive(:ReadHardware) { [ { - "dev_name" => NEW_STYLE_NAME, - "mac" => "00:11:22:33:44:FF", - "busid" => "0000:00:19.0" + "dev_name" => NEW_STYLE_NAME, + "mac" => "00:11:22:33:44:FF", + "permanent_mac" => "00:11:22:33:44:FF", + "busid" => "0000:00:19.0" } ] } diff --git a/test/netcard_test.rb b/test/netcard_test.rb index 90421a331..e573764e2 100755 --- a/test/netcard_test.rb +++ b/test/netcard_test.rb @@ -40,6 +40,7 @@ "bus" => HWINFO_DEVICE_BUS, "busid" => HWINFO_DEVICE_BUSID, "mac" => HWINFO_DEVICE_MAC, + "permanent_mac" => HWINFO_DEVICE_MAC, "link" => nil, "wl_channels" => nil, "wl_bitrates" => nil, diff --git a/test/network_autoyast_test.rb b/test/network_autoyast_test.rb index 0fed54496..90607d478 100755 --- a/test/network_autoyast_test.rb +++ b/test/network_autoyast_test.rb @@ -486,19 +486,22 @@ def mock_lan_item(renamed_to: nil) let(:hw_netcard) do [ { - "dev_name" => "eth0", - "busid" => "0000:01:00.0", - "mac" => "00:00:00:00:00:00" + "dev_name" => "eth0", + "busid" => "0000:01:00.0", + "mac" => "00:00:00:00:00:00", + "permanent_mac" => "00:00:00:00:00:00" }, { - "dev_name" => "eth1", - "busid" => "0000:01:00.1", - "mac" => "00:00:00:00:00:01" + "dev_name" => "eth1", + "busid" => "0000:01:00.1", + "mac" => "00:00:00:00:00:01", + "permanent_mac" => "00:00:00:00:00:01" }, { - "dev_name" => "eth2", - "busid" => "0000:01:00.2", - "mac" => "00:00:00:00:00:02" + "dev_name" => "eth2", + "busid" => "0000:01:00.2", + "mac" => "00:00:00:00:00:02", + "permanent_mac" => "00:00:00:00:00:02" } ] end