From 5743a963d81298f0ef3972e91e7f5bba836cd7ca Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Jun 18 2018 09:00:20 +0000 Subject: fix build failure due to unstable tests --- diff --git a/0001-fix-clients-tests.patch b/0001-fix-clients-tests.patch new file mode 100644 index 0000000..afbdb52 --- /dev/null +++ b/0001-fix-clients-tests.patch @@ -0,0 +1,234 @@ +From 4456abb3d3e9f35e5760b4d0883a44d371c5eb84 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Mon, 18 Jun 2018 10:49:46 +0200 +Subject: [PATCH 1/2] tests: fix race in setting signal strength for Wi-Fi AP + in NM stub + +This opens the tests up to races. If we want to change the strength, we +need to do it in a controlled, race-free manner. This is especially the +case, because clients/tests run a large number of nmcli instances in +parallel, and it's thus racy which signal the nmcli processes will +see. + +This also fixes a bug at + + self._dbus_property_set(IFACE_WIFI_AP, PRP_WIFI_AP_STRENGTH, strength) + +@strength must be a D-Bus type, so that python-dbus knows the correct +type for serialization. + +(cherry picked from commit 7e118c00916aa7dd3f04752debbcc09569d6effe) +(cherry picked from commit e05ce581b619a2fa3584b374aa7c3dad909c3a8a) +--- + tools/test-networkmanager-service.py | 26 ++------------------------ + 1 file changed, 2 insertions(+), 24 deletions(-) + +diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py +index ab63b60a8..c55955d99 100755 +--- a/tools/test-networkmanager-service.py ++++ b/tools/test-networkmanager-service.py +@@ -864,8 +864,6 @@ class WifiAp(ExportedObj): + strength = Util.random_int(self.path, 100) + + self.ssid = ssid +- self.strength_counter = 0 +- self.strength_id = GLib.timeout_add_seconds(10, self.strength_cb, None) + + props = { + PRP_WIFI_AP_FLAGS: dbus.UInt32(flags), +@@ -881,17 +879,6 @@ class WifiAp(ExportedObj): + + self.dbus_interface_add(IFACE_WIFI_AP, props, WifiAp.PropertiesChanged) + +- def __del__(self): +- if self.strength_id > 0: +- GLib.source_remove(self.strength_id) +- self.strength_id = 0 +- +- def strength_cb(self, ignored): +- self.strength_counter += 1 +- strength = Util.random_int(self.path + str(self.strength_counter), 100) +- self._dbus_property_set(IFACE_WIFI_AP, PRP_WIFI_AP_STRENGTH, strength) +- return True +- + @dbus.service.signal(IFACE_WIFI_AP, signature='a{sv}') + def PropertiesChanged(self, changed): + pass +@@ -991,25 +978,16 @@ class WimaxNsp(ExportedObj): + + ExportedObj.__init__(self, ExportedObj.create_path(WimaxNsp)) + +- self.strength_id = GLib.timeout_add_seconds(10, self.strength_cb, None) ++ strength = Util.random_int(self.path, 100) + + props = { + PRP_WIMAX_NSP_NAME: name, +- PRP_WIMAX_NSP_SIGNAL_QUALITY: dbus.UInt32(random.randint(0, 100)), ++ PRP_WIMAX_NSP_SIGNAL_QUALITY: dbus.UInt32(strength), + PRP_WIMAX_NSP_NETWORK_TYPE: dbus.UInt32(NM.WimaxNspNetworkType.HOME), + } + + self.dbus_interface_add(IFACE_WIMAX_NSP, props, WimaxNsp.PropertiesChanged) + +- def __del__(self): +- if self.strength_id > 0: +- GLib.source_remove(self.strength_id) +- self.strength_id = 0 +- +- def strength_cb(self, ignored): +- self._dbus_property_set(IFACE_WIMAX_NSP, PRP_WIMAX_NSP_SIGNAL_QUALITY, dbus.UInt32(random.randint(0, 100))) +- return True +- + @dbus.service.signal(IFACE_WIMAX_NSP, signature='a{sv}') + def PropertiesChanged(self, changed): + pass +-- +2.17.1 + + +From 73ee7942519eadc15af4c76c501b9c6f41782032 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Sat, 16 Jun 2018 13:38:45 +0200 +Subject: [PATCH 2/2] tests: improve NetworkManager stub service for Wi-Fi + scanning + +Now that nmcli initiates a scan before displaying Wi-Fi networks, +the stub service must properly support that as well. + +For the moment, the stub service chooses "now" as LastScan timestamp. +This causes nmcli not to trigger a new scan, because nmcli gives +unstable output if multiple nmcli processes in parallel race to +trigger a Wi-Fi scan. That should be fixed. + +(cherry picked from commit 56a0488bbae38f8d802b1cc32ba94c1640f70bc6) +(cherry picked from commit efddb0cef5187f4f77d0c6b94f75063ad8f9d6a2) +--- + clients/cli/devices.c | 11 ++++++++ + tools/test-networkmanager-service.py | 40 +++++++++++++++++++++++----- + 2 files changed, 45 insertions(+), 6 deletions(-) + +diff --git a/clients/cli/devices.c b/clients/cli/devices.c +index 58102ed37..aa28678ff 100644 +--- a/clients/cli/devices.c ++++ b/clients/cli/devices.c +@@ -2772,6 +2772,17 @@ wifi_list_aps (NMDeviceWifi *wifi, + + needs_rescan = rescan_cutoff < 0 || (rescan_cutoff > 0 && nm_device_wifi_get_last_scan (wifi) < rescan_cutoff); + ++ /* FIXME: nmcli should either ++ * - don't request any new scan for any device and print the full AP list right ++ * away. ++ * - or, when requesting a scan on one or more devices, don't print the result ++ * before all requests complete. ++ * ++ * Otherwise: ++ * - the printed output is not self consistent. E.g. it will print the result ++ * on one device at a certain time, while printing the result for another ++ * device at a later point in time. ++ * - the order in which we print the AP list per-device, is unstable. */ + if (needs_rescan) { + data = g_slice_new0 (WifiListData); + data->nmc = nmc; +diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py +index c55955d99..f6da6e1fd 100755 +--- a/tools/test-networkmanager-service.py ++++ b/tools/test-networkmanager-service.py +@@ -49,6 +49,11 @@ class Util: + + PY3 = (sys.version_info[0] == 3) + ++ @staticmethod ++ def g_source_remove(source_id): ++ if source_id is not None: ++ GLib.source_remove(source_id) ++ + @staticmethod + def addr_family_check(family, allow_af_unspec = False): + if family == socket.AF_INET: +@@ -840,6 +845,7 @@ PRP_WIFI_AP_HW_ADDRESS = "HwAddress" + PRP_WIFI_AP_MODE = "Mode" + PRP_WIFI_AP_MAX_BITRATE = "MaxBitrate" + PRP_WIFI_AP_STRENGTH = "Strength" ++PRP_WIFI_AP_LAST_SEEN = "LastSeen" + + class WifiAp(ExportedObj): + +@@ -875,6 +881,7 @@ class WifiAp(ExportedObj): + PRP_WIFI_AP_MODE: dbus.UInt32(getattr(NM,'80211Mode').INFRA), + PRP_WIFI_AP_MAX_BITRATE: dbus.UInt32(54000), + PRP_WIFI_AP_STRENGTH: dbus.Byte(strength), ++ PRP_WIFI_AP_LAST_SEEN: dbus.Int32(NM.utils_get_timestamp_msec() / 1000), + } + + self.dbus_interface_add(IFACE_WIFI_AP, props, WifiAp.PropertiesChanged) +@@ -902,6 +909,18 @@ class WifiDevice(Device): + mac = Util.random_mac(self.ident) + + self.aps = [] ++ self.scan_cb_id = None ++ ++ # Note: we would like to simulate how nmcli calls RequestScan() and we could ++ # do so by using an older timestamp. However, that makes the client tests ++ # racy, because if a bunch of nmcli instances run in parallel against this ++ # service, earlier instances will issue a RequestScan(), while later instances ++ # won't do that (because the LastScan timestamp is already updated). That means, ++ # the later instances will print the scan result immediately, and in another sort ++ # order. That should be fixed, by nmcli not starting to print anything, before ++ # all RequestScan() requests complete, and thus, always print a consistent list ++ # of results. ++ ts = NM.utils_get_timestamp_msec() + + props = { + PRP_WIFI_HW_ADDRESS: mac, +@@ -911,7 +930,7 @@ class WifiDevice(Device): + PRP_WIFI_WIRELESS_CAPABILITIES: dbus.UInt32(0xFF), + PRP_WIFI_ACCESS_POINTS: ExportedObj.to_path_array(self.aps), + PRP_WIFI_ACTIVE_ACCESS_POINT: ExportedObj.to_path(None), +- PRP_WIFI_LAST_SCAN: NM.utils_get_timestamp_msec(), ++ PRP_WIFI_LAST_SCAN: dbus.Int64(ts), + } + + self.dbus_interface_add(IFACE_WIFI, props, WifiDevice.PropertiesChanged) +@@ -928,6 +947,15 @@ class WifiDevice(Device): + + @dbus.service.method(dbus_interface=IFACE_WIFI, in_signature='a{sv}', out_signature='') + def RequestScan(self, props): ++ self.scan_cb_id = Util.g_source_remove(self.scan_cb_id) ++ def cb(): ++ ts = NM.utils_get_timestamp_msec() ++ for ap in self.aps: ++ ap._dbus_property_set(IFACE_WIFI_AP, PRP_WIFI_AP_LAST_SEEN, dbus.Int32(ts / 1000)) ++ self._dbus_property_set(IFACE_WIFI, PRP_WIFI_LAST_SCAN, dbus.Int64(ts)) ++ self.scan_cb_id = None ++ return False ++ self.scan_cb_id = GLib.idle_add(cb) + pass + + @dbus.service.signal(IFACE_WIFI, signature='o') +@@ -947,6 +975,10 @@ class WifiDevice(Device): + self.AccessPointRemoved(ExportedObj.to_path(ap)) + ap.unexport() + ++ def stop(self): ++ self.scan_cb_id = Util.g_source_remove(self.scan_cb_id) ++ super(WifiDevice, self).stop() ++ + @dbus.service.signal(IFACE_WIFI, signature='o') + def AccessPointRemoved(self, ap_path): + pass +@@ -1138,11 +1170,7 @@ class ActiveConnection(ExportedObj): + self.StateChanged(state, dbus.UInt32(reason)) + + def activation_cancel(self): +- if self._activation_id is None: +- return False +- GLib.source_remove(self._activation_id) +- self._activation_id = None +- return True ++ self._activation_id = Util.g_source_remove(self._activation_id) + + def _activation_step2(self): + assert self._activation_id is not None +-- +2.17.1 + diff --git a/NetworkManager.spec b/NetworkManager.spec index 0441362..9ad81fe 100644 --- a/NetworkManager.spec +++ b/NetworkManager.spec @@ -104,6 +104,7 @@ Source2: 00-server.conf Source3: 20-connectivity-fedora.conf # Patch1: +Patch1: 0001-fix-clients-tests.patch Requires(post): systemd Requires(post): /usr/sbin/update-alternatives @@ -413,6 +414,7 @@ by nm-connection-editor and nm-applet in a non-graphical environment. %setup -q -n NetworkManager-%{real_version} #%patch1 -p1 +%patch1 -p1 %build %if %{with regen_docs}