From 4456abb3d3e9f35e5760b4d0883a44d371c5eb84 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
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 <thaller@redhat.com>
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