[PATCH] Fix check for device state when reading its IPXConfig (#1001776)

[Radek Vykydal]

Device has to be in ACTIVATED state for IPXConfig to be valid.

1) use a recently added nm module function to get IP configuration, replacing
python-dbus with GDBus here. Python-dbus would give us stale value.

2) Don't use get_cached_property of GDBus as it would give us stale value too.
Instead use DBus.Properties interface call to get current values.
---
 pyanaconda/nm.py                    | 77 ++++++++++++++++++++-----------------
 pyanaconda/ui/gui/spokes/network.py | 64 +++++++++++-------------------
 2 files changed, 63 insertions(+), 78 deletions(-)

diff --git a/pyanaconda/nm.py b/pyanaconda/nm.py
index 1109d6e..4f0923d 100644
--- a/pyanaconda/nm.py
+++ b/pyanaconda/nm.py
@@ -73,10 +73,27 @@ def _get_proxy(bus_type=Gio.BusType.SYSTEM,
                                            cancellable)
     return proxy

+def _get_property(object_path, prop, interface_name_suffix=""):
+    interface_name = "org.freedesktop.NetworkManager" + interface_name_suffix
+    proxy = _get_proxy(object_path=object_path, interface_name="org.freedesktop.DBus.Properties")
+    args = GLib.Variant('(ss)', (interface_name, prop))
+    try:
+        prop = proxy.call_sync("Get",
+                                args,
+                                Gio.DBusCallFlags.NONE,
+                                DEFAULT_DBUS_TIMEOUT,
+                                None)
+    except GLib.GError as e:
+        if "org.freedesktop.DBus.Error.AccessDenied" in e.message:
+            return None
+        else:
+            raise
+
+    return prop.unpack()[0]
+
 def nm_state():
     """Return state of NetworkManager"""
-    proxy = _get_proxy()
-    return proxy.get_cached_property("State").unpack()
+    return _get_property("/org/freedesktop/NetworkManager", "State")

# FIXME - use just GLOBAL? There is some connectivity checking
 # for GLOBAL in NM (nm_connectivity_get_connected), not sure if
@@ -107,11 +124,11 @@ def nm_devices():

devices = devices.unpack()[0]
     for device in devices:
-        proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
-        device_type = proxy.get_cached_property("DeviceType").unpack()
+        device_type = _get_property(device, "DeviceType", ".Device")
         if device_type not in supported_device_types:
             continue
-        interfaces.append(proxy.get_cached_property("Interface").unpack())
+        iface = _get_property(device, "Interface", ".Device")
+        interfaces.append(iface)

return interfaces

@@ -120,19 +137,15 @@ def nm_activated_devices():

interfaces = []

-    proxy = _get_proxy()
-    active_connections = proxy.get_cached_property("ActiveConnections").unpack()
+    active_connections = _get_property("/org/freedesktop/NetworkManager", "ActiveConnections")
     for ac in active_connections:
-        proxy = _get_proxy(object_path=ac, interface_name="org.freedesktop.NetworkManager.Connection.Active")
-        state = proxy.get_cached_property("State")
-        if not state or state.unpack() != NetworkManager.ActiveConnectionState.ACTIVATED:
-            continue
-        devices = proxy.get_cached_property("Devices")
-        if not devices:
+        state = _get_property(ac, "State", ".Connection.Active")
+        if state != NetworkManager.ActiveConnectionState.ACTIVATED:
             continue
-        for device in devices.unpack():
-            proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
-            interfaces.append(proxy.get_cached_property("Interface").unpack())
+        devices = _get_property(ac, "Devices", ".Connection.Active")
+        for device in devices:
+            iface = _get_property(device, "Interface", ".Device")
+            interfaces.append(iface)

return interfaces

@@ -181,18 +194,14 @@ def nm_device_property(name, prop):
         raise

device = device.unpack()[0]
-    proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")

-    if prop in proxy.get_cached_property_names():
-        retval = proxy.get_cached_property(prop).unpack()
-    else:
+    retval = _get_property(device, prop, ".Device")
+    if not retval:
         # Look in device type based interface
         interface = _device_type_specific_interface(device)
         if interface:
-            proxy = _get_proxy(object_path=device, interface_name=interface)
-            if prop in proxy.get_cached_property_names():
-                retval = proxy.get_cached_property(prop).unpack()
-            else:
+            retval = _get_property(device, prop, interface[30:])
+            if not retval:
                 raise PropertyNotFoundError(prop)
         else:
             raise PropertyNotFoundError(prop)
@@ -284,10 +293,10 @@ def nm_device_ip_config(name, version=4):
         return []

if version == 4:
-        dbus_iface = "org.freedesktop.NetworkManager.IP4Config"
+        dbus_iface = ".IP4Config"
         prop= "Ip4Config"
     elif version == 6:
-        dbus_iface = "org.freedesktop.NetworkManager.IP6Config"
+        dbus_iface = ".IP6Config"
         prop= "Ip6Config"
     else:
         return []
@@ -296,9 +305,7 @@ def nm_device_ip_config(name, version=4):
     if config == "/":
         return []

-    proxy = _get_proxy(object_path=config, interface_name=dbus_iface)
-
-    addresses = proxy.get_cached_property("Addresses").unpack()
+    addresses = _get_property(config, "Addresses", dbus_iface)
     addr_list = []
     for addr, prefix, gateway in addresses:
         # TODO - look for a library function (could have used IPy but byte order!)
@@ -310,7 +317,7 @@ def nm_device_ip_config(name, version=4):
             gateway_str = nm_dbus_ay_to_ipv6(gateway)
         addr_list.append([addr_str, prefix, gateway_str])

-    nameservers = proxy.get_cached_property("Nameservers").unpack()
+    nameservers = _get_property(config, "Nameservers", dbus_iface)
     ns_list = []
     for ns in nameservers:
         # TODO - look for a library function
@@ -322,7 +329,6 @@ def nm_device_ip_config(name, version=4):

return [addr_list, ns_list]

-
 def nm_ntp_servers_from_dhcp():
     """Return a list of NTP servers that were specified the reply of the
        DHCP server or empty list if no NTP servers were returned.
@@ -335,16 +341,15 @@ def nm_ntp_servers_from_dhcp():
     for device in active_devices:
         # harvest NTP server addresses from DHCPv4
         dhcp4_path = nm_device_property(device, "Dhcp4Config")
-        dhcp4_proxy = _get_proxy(object_path=dhcp4_path,
-                interface_name="org.freedesktop.NetworkManager.DHCP4Config")
-        options = dhcp4_proxy.get_cached_property("Options")
-        if options and 'ntp_servers' in options.unpack():
+        options = _get_property(dhcp4_path, "Options", ".DHCP4Config")
+        if options and 'ntp_servers' in options:
             # NTP server addresses returned by DHCP are whitespace delimited
-            ntp_servers_string = options.unpack()["ntp_servers"] 
+            ntp_servers_string = options["ntp_servers"]
             for ip in ntp_servers_string.split(" "):
                 ntp_servers.append(ip)

# NetworkManager does not request NTP/SNTP options for DHCP6
+    print ntp_servers
     return ntp_servers

def _device_settings(name):
diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
index f7c1275..132c7f2 100644
--- a/pyanaconda/ui/gui/spokes/network.py
+++ b/pyanaconda/ui/gui/spokes/network.py
@@ -43,7 +43,7 @@ from pyanaconda.ui.gui.utils import gtk_call_once, enlightbox
 from pyanaconda.ui.common import FirstbootSpokeMixIn

from pyanaconda import network
-from pyanaconda.nm import nm_activated_devices, nm_device_setting_value, nm_dbus_ay_to_ipv6
+from pyanaconda.nm import nm_device_setting_value, nm_device_ip_config

from gi.repository import GLib, GObject, Pango, Gio, NetworkManager, NMClient
 import dbus
@@ -704,12 +704,9 @@ class NetworkControlBox(object):
         self._refresh_parent_vlanid(device)
         self._refresh_speed_hwaddr(device, state)
         self._refresh_ap(device, state)
-        self._refresh_device_cfg(device, state)
+        self._refresh_device_cfg(device)

-    def _refresh_device_cfg(self, device, state):
-
-        if state is None:
-            state = device.get_state()
+    def _refresh_device_cfg(self, device):

dev_type = device.get_device_type()
         if dev_type == NetworkManager.DeviceType.ETHERNET:
@@ -721,45 +718,28 @@ class NetworkControlBox(object):
         elif dev_type == NetworkManager.DeviceType.VLAN:
             dt = "wired"

-        if state == NetworkManager.DeviceState.ACTIVATED:
-            ipv4cfg = device.get_ip4_config()
-            ipv6cfg = device.get_ip6_config()
-        else:
-            ipv4cfg = None
-            ipv6cfg = None
-
-        if ipv4cfg:
-            addr = socket.inet_ntoa(struct.pack('=L',
-                                                ipv4cfg.get_addresses()[0].get_address()))
-            self._set_device_info_value(dt, "ipv4", addr)
-            dnss = " ".join(socket.inet_ntoa(struct.pack('=L', addr))
-                             for addr in ipv4cfg.get_nameservers())
-            self._set_device_info_value(dt, "dns", dnss)
-            gateway = socket.inet_ntoa(struct.pack('=L',
-                                                   ipv4cfg.get_addresses()[0].get_gateway()))
-            self._set_device_info_value(dt, "route", gateway)
-            if dt == "wired":
-                prefix = ipv4cfg.get_addresses()[0].get_prefix()
-                nm_utils.nm_utils_ip4_prefix_to_netmask.argtypes = [ctypes.c_uint32]
-                nm_utils.nm_utils_ip4_prefix_to_netmask.restype = ctypes.c_uint32
-                netmask = nm_utils.nm_utils_ip4_prefix_to_netmask(prefix)
-                netmask = socket.inet_ntoa(struct.pack('=L', netmask))
-                self._set_device_info_value(dt, "subnet", netmask)
+        ipv4cfg = nm_device_ip_config(device.get_iface(), version=4)
+        ipv6cfg = nm_device_ip_config(device.get_iface(), version=6)
+
+        if ipv4cfg and ipv4cfg[0]:
+            addr_str, prefix, gateway_str = ipv4cfg[0][0]
+            netmask_str = network.prefix2netmask(prefix)
+            dnss_str = " ".join(ipv4cfg[1])
         else:
-            self._set_device_info_value(dt, "ipv4", None)
-            self._set_device_info_value(dt, "dns", None)
-            self._set_device_info_value(dt, "route", None)
-            if dt == "wired":
-                self._set_device_info_value(dt, "subnet", None)
+            addr_str = dnss_str = gateway_str = netmask_str = None
+        self._set_device_info_value(dt, "ipv4", addr_str)
+        self._set_device_info_value(dt, "dns", dnss_str)
+        self._set_device_info_value(dt, "route", gateway_str)
+        if dt == "wired":
+            self._set_device_info_value(dt, "subnet", netmask_str)

-        # TODO NM_GI_BUGS - segfaults on get_addres(), get_prefix()
         addr6_str = ""
-        if ipv6cfg:
-            config = dbus.SystemBus().get_object(NM_SERVICE, ipv6cfg.get_path())
-            for addr, _prefix, _gw in getNMObjProperty(config, ".IP6Config", "Addresses"):
-                ipv6_addr = nm_dbus_ay_to_ipv6(addr)
-                if not ipv6_addr.startswith("fe80:"):
-                    addr6_str += "%s\n" % ipv6_addr
+        if ipv6cfg and ipv6cfg[0]:
+            for ipv6addr in ipv6cfg[0]:
+                addr_str, prefix, gateway_str = ipv6addr
+                # Do not display link-local addresses
+                if not addr_str.startswith("fe80:"):
+                    addr6_str += "%s/%d\n" % (addr_str, prefix)

self._set_device_info_value(dt, "ipv6", addr6_str.strip() or None)

[Vratislav Podzimek]

On Mon, 2013-09-09 at 09:41 +0200, Radek Vykydal wrote:
> Device has to be in ACTIVATED state for IPXConfig to be valid.
> use a recently added nm module function to get IP configuration, replacing
>
> python-dbus with GDBus here. Python-dbus would give us stale value.
> Don't use get_cached_property of GDBus as it would give us stale value too.
> Instead use DBus.Properties interface call to get current values.
>
> pyanaconda/nm.py                    | 77 ++++++++++++++++++++-----------------
>  pyanaconda/ui/gui/spokes/network.py | 64 +++++++++++-------------------
>  2 files changed, 63 insertions(+), 78 deletions(-)
>
> diff --git a/pyanaconda/nm.py b/pyanaconda/nm.py
> index 1109d6e..4f0923d 100644
> --- a/pyanaconda/nm.py
> +++ b/pyanaconda/nm.py
> @@ -73,10 +73,27 @@ def _get_proxy(bus_type=Gio.BusType.SYSTEM,
>                                             cancellable)
>      return proxy
>
> +def _get_property(object_path, prop, interface_name_suffix=""):
> interface_name = "org.freedesktop.NetworkManager" + interface_name_suffix
> proxy = _get_proxy(object_path=object_path, interface_name="org.freedesktop.DBus.Properties")
> args = GLib.Variant('(ss)', (interface_name, prop))
> try:
>    prop = proxy.call_sync("Get",
>                            args,
>                            Gio.DBusCallFlags.NONE,
>                            DEFAULT_DBUS_TIMEOUT,
>                            None)
> except GLib.GError as e:
>    if "org.freedesktop.DBus.Error.AccessDenied" in e.message:
>        return None
>    else:
>        raise
> return prop.unpack()[0]

I'd suggest using dbus_get_property_safe_sync from
pyanaconda/safe_dbus.py. From what I recall using proxy.call_sync may
cause troubles if it was used in a non-main thread. The _get_property
function could stay almost the same, it should just handle exceptions
raised from the safe_dbus module.
> def nm_state():
>      """Return state of NetworkManager"""
> proxy = _get_proxy()
> return proxy.get_cached_property("State").unpack()
> return _get_property("/org/freedesktop/NetworkManager", "State")
>
> # FIXME - use just GLOBAL? There is some connectivity checking
>  # for GLOBAL in NM (nm_connectivity_get_connected), not sure if
> @@ -107,11 +124,11 @@ def nm_devices():
>  devices = devices.unpack()[0]
>  for device in devices:
>    proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
>    device_type = proxy.get_cached_property("DeviceType").unpack()
>    device_type = _get_property(device, "DeviceType", ".Device")
>    if device_type not in supported_device_types:
>        continue
>    interfaces.append(proxy.get_cached_property("Interface").unpack())
>    iface = _get_property(device, "Interface", ".Device")
>    interfaces.append(iface)
>
> return interfaces
>
> @@ -120,19 +137,15 @@ def nm_activated_devices():
>  interfaces = []
>
> proxy = _get_proxy()
> active_connections = proxy.get_cached_property("ActiveConnections").unpack()
> active_connections = _get_property("/org/freedesktop/NetworkManager", "ActiveConnections")
> for ac in active_connections:
>    proxy = _get_proxy(object_path=ac, interface_name="org.freedesktop.NetworkManager.Connection.Active")
>    state = proxy.get_cached_property("State")
>    if not state or state.unpack() != NetworkManager.ActiveConnectionState.ACTIVATED:
>        continue
>    devices = proxy.get_cached_property("Devices")
>    if not devices:
>    state = _get_property(ac, "State", ".Connection.Active")
>    if state != NetworkManager.ActiveConnectionState.ACTIVATED:
>        continue
>    for device in devices.unpack():
>        proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
>        interfaces.append(proxy.get_cached_property("Interface").unpack())
>    devices = _get_property(ac, "Devices", ".Connection.Active")
>    for device in devices:
>        iface = _get_property(device, "Interface", ".Device")
>        interfaces.append(iface)
>
> return interfaces
>
> @@ -181,18 +194,14 @@ def nm_device_property(name, prop):
>          raise
>  device = device.unpack()[0]
>
> proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
>
> if prop in proxy.get_cached_property_names():
>    retval = proxy.get_cached_property(prop).unpack()
>
> else:
> retval = _get_property(device, prop, ".Device")
> if not retval:
>     # Look in device type based interface
>     interface = _device_type_specific_interface(device)
>     if interface:
>        proxy = _get_proxy(object_path=device, interface_name=interface)
>        if prop in proxy.get_cached_property_names():
>            retval = proxy.get_cached_property(prop).unpack()
>        else:
>        retval = _get_property(device, prop, interface[30:])
>        if not retval:
>            raise PropertyNotFoundError(prop)
>    else:
>        raise PropertyNotFoundError(prop)
>
> @@ -284,10 +293,10 @@ def nm_device_ip_config(name, version=4):
>          return []
>  if version == 4:
>    dbus_iface = "org.freedesktop.NetworkManager.IP4Config"
>    dbus_iface = ".IP4Config"
>    prop= "Ip4Config"
> elif version == 6:
>    dbus_iface = "org.freedesktop.NetworkManager.IP6Config"
>    dbus_iface = ".IP6Config"
>    prop= "Ip6Config"
> else:
>        return []
>
> @@ -296,9 +305,7 @@ def nm_device_ip_config(name, version=4):
>      if config == "/":
>          return []
> proxy = _get_proxy(object_path=config, interface_name=dbus_iface)
> addresses = proxy.get_cached_property("Addresses").unpack()
> addresses = _get_property(config, "Addresses", dbus_iface)
> addr_list = []
> for addr, prefix, gateway in addresses:
>     # TODO - look for a library function (could have used IPy but byte order!)
>
> @@ -310,7 +317,7 @@ def nm_device_ip_config(name, version=4):
>              gateway_str = nm_dbus_ay_to_ipv6(gateway)
>          addr_list.append([addr_str, prefix, gateway_str])
> nameservers = proxy.get_cached_property("Nameservers").unpack()
> nameservers = _get_property(config, "Nameservers", dbus_iface)
> ns_list = []
> for ns in nameservers:
>     # TODO - look for a library function
>
> @@ -322,7 +329,6 @@ def nm_device_ip_config(name, version=4):
>  return [addr_list, ns_list]
>
> def nm_ntp_servers_from_dhcp():
>      """Return a list of NTP servers that were specified the reply of the
>         DHCP server or empty list if no NTP servers were returned.
> @@ -335,16 +341,15 @@ def nm_ntp_servers_from_dhcp():
>      for device in active_devices:
>          # harvest NTP server addresses from DHCPv4
>          dhcp4_path = nm_device_property(device, "Dhcp4Config")
>    dhcp4_proxy = _get_proxy(object_path=dhcp4_path,
>            interface_name="org.freedesktop.NetworkManager.DHCP4Config")
>    options = dhcp4_proxy.get_cached_property("Options")
>    if options and 'ntp_servers' in options.unpack():
>    options = _get_property(dhcp4_path, "Options", ".DHCP4Config")
>    if options and 'ntp_servers' in options:
>        # NTP server addresses returned by DHCP are whitespace delimited
>        ntp_servers_string = options.unpack()["ntp_servers"] 
>        ntp_servers_string = options["ntp_servers"]
>        for ip in ntp_servers_string.split(" "):
>            ntp_servers.append(ip)
>
>    # NetworkManager does not request NTP/SNTP options for DHCP6
>
> print ntp_servers

Debugging print?

Otherwise this looks good to me.

[Radek Vykydal]

On 09/09/2013 10:23 AM, Vratislav Podzimek wrote:
> On Mon, 2013-09-09 at 09:41 +0200, Radek Vykydal wrote:
> > Device has to be in ACTIVATED state for IPXConfig to be valid.
> > use a recently added nm module function to get IP configuration, replacing
> >
> > python-dbus with GDBus here. Python-dbus would give us stale value.
> > Don't use get_cached_property of GDBus as it would give us stale value too.
> > Instead use DBus.Properties interface call to get current values.
> >
> > pyanaconda/nm.py                    | 77 ++++++++++++++++++++-----------------
> >   pyanaconda/ui/gui/spokes/network.py | 64 +++++++++++-------------------
> >   2 files changed, 63 insertions(+), 78 deletions(-)
> >
> > diff --git a/pyanaconda/nm.py b/pyanaconda/nm.py
> > index 1109d6e..4f0923d 100644
> > --- a/pyanaconda/nm.py
> > +++ b/pyanaconda/nm.py
> > @@ -73,10 +73,27 @@ def _get_proxy(bus_type=Gio.BusType.SYSTEM,
> >                                              cancellable)
> >       return proxy
> >
> > +def _get_property(object_path, prop, interface_name_suffix=""):
> > interface_name = "org.freedesktop.NetworkManager" + interface_name_suffix
> > proxy = _get_proxy(object_path=object_path, interface_name="org.freedesktop.DBus.Properties")
> > args = GLib.Variant('(ss)', (interface_name, prop))
> > try:
> >    prop = proxy.call_sync("Get",
> >                            args,
> >                            Gio.DBusCallFlags.NONE,
> >                            DEFAULT_DBUS_TIMEOUT,
> >                            None)
> > except GLib.GError as e:
> >    if "org.freedesktop.DBus.Error.AccessDenied" in e.message:
> >        return None
> >    else:
> >        raise
> > return prop.unpack()[0]
>
> I'd suggest using dbus_get_property_safe_sync from
> pyanaconda/safe_dbus.py. From what I recall using proxy.call_sync may
> cause troubles if it was used in a non-main thread. The _get_property
> function could stay almost the same, it should just handle exceptions
> raised from the safe_dbus module.

For now I'd assume that calling a method on a proxy synchronously is
safe, as the doc says:

"A GDBusProxy 
https://developer.gnome.org/gio/2.37/GDBusProxy.htmlinstance can be 
used from multiple threads but note that all signals (e.g. "g-signal" 
https://developer.gnome.org/gio/2.37/GDBusProxy.html#GDBusProxy-g-signal, 
"g-properties-changed" 
https://developer.gnome.org/gio/2.37/GDBusProxy.html#GDBusProxy-g-properties-changedand 
"notify" 
https://developer.gnome.org/gobject/unstable/gobject-The-Base-Object-Type.html#GObject-notify) 
are emitted in the thread-default main loop 
https://developer.gnome.org/glib/unstable/glib-The-Main-Event-Loop.html#g-main-context-push-thread-defaultof 
the thread where the instance was constructed."

and I'll get back to dbus_get_property_safe_sync or to Glib guys if
I hit similar issue. In this bz just calling DBus.Properties method instead
using the properties cached by the proxy seems to fix the problem.

(Checking the device state and then getting the config is racy anyway,
but this is another story with the race condition being harder to hit.)

I've peeked into the gdbus code and gdbus_call_proxy_sync is calling
g_dbus_connection_call_sync 
http://fossies.org/dox/glib-2.36.4/gdbusconnection_8c.html#a0140b7433268c65bd4c812d112316a66(called 
by safe_dbus) so the only
difference seems to be the connection object being used. With proxy
in nm.py the connection object seems to by created each time, while
in keyboard.py you are re-using a connection created in __init__ of
LocaledWrapper. Also as you said in your case the connection is
autostarting its service which is not the case with NM soperhaps
this might really be the problem I don't hit with NM service.
> > def nm_state():
> >       """Return state of NetworkManager"""
> > proxy = _get_proxy()
> > return proxy.get_cached_property("State").unpack()
> >
> > return _get_property("/org/freedesktop/NetworkManager", "State")
> >
> > # FIXME - use just GLOBAL? There is some connectivity checking
> > # for GLOBAL in NM (nm_connectivity_get_connected), not sure if
> >
> > @@ -107,11 +124,11 @@ def nm_devices():
> >   devices = devices.unpack()[0]
> >   for device in devices:
> >    proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
> >    device_type = proxy.get_cached_property("DeviceType").unpack()
> >    device_type = _get_property(device, "DeviceType", ".Device")
> >     if device_type not in supported_device_types:
> >         continue
> >    interfaces.append(proxy.get_cached_property("Interface").unpack())
> >    iface = _get_property(device, "Interface", ".Device")
> >    interfaces.append(iface)
> >
> > return interfaces
> >
> > @@ -120,19 +137,15 @@ def nm_activated_devices():
> >   interfaces = []
> >
> > proxy = _get_proxy()
> > active_connections = proxy.get_cached_property("ActiveConnections").unpack()
> > active_connections = _get_property("/org/freedesktop/NetworkManager", "ActiveConnections")
> >  for ac in active_connections:
> >    proxy = _get_proxy(object_path=ac, interface_name="org.freedesktop.NetworkManager.Connection.Active")
> >    state = proxy.get_cached_property("State")
> >    if not state or state.unpack() != NetworkManager.ActiveConnectionState.ACTIVATED:
> >        continue
> >    devices = proxy.get_cached_property("Devices")
> >    if not devices:
> >    state = _get_property(ac, "State", ".Connection.Active")
> >    if state != NetworkManager.ActiveConnectionState.ACTIVATED:
> >         continue
> >    for device in devices.unpack():
> >        proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
> >        interfaces.append(proxy.get_cached_property("Interface").unpack())
> >    devices = _get_property(ac, "Devices", ".Connection.Active")
> >    for device in devices:
> >        iface = _get_property(device, "Interface", ".Device")
> >        interfaces.append(iface)
> >
> > return interfaces
> >
> > @@ -181,18 +194,14 @@ def nm_device_property(name, prop):
> >           raise
> >   device = device.unpack()[0]
> >
> > proxy = _get_proxy(object_path=device, interface_name="org.freedesktop.NetworkManager.Device")
> >
> > if prop in proxy.get_cached_property_names():
> >    retval = proxy.get_cached_property(prop).unpack()
> >
> > else:
> > retval = _get_property(device, prop, ".Device")
> > if not retval:
> >      # Look in device type based interface
> >      interface = _device_type_specific_interface(device)
> >      if interface:
> >        proxy = _get_proxy(object_path=device, interface_name=interface)
> >        if prop in proxy.get_cached_property_names():
> >            retval = proxy.get_cached_property(prop).unpack()
> >        else:
> >        retval = _get_property(device, prop, interface[30:])
> >        if not retval:
> >             raise PropertyNotFoundError(prop)
> >     else:
> >         raise PropertyNotFoundError(prop)
> >
> > @@ -284,10 +293,10 @@ def nm_device_ip_config(name, version=4):
> >           return []
> >   if version == 4:
> >    dbus_iface = "org.freedesktop.NetworkManager.IP4Config"
> >    dbus_iface = ".IP4Config"
> >     prop= "Ip4Config"
> > elif version == 6:
> >    dbus_iface = "org.freedesktop.NetworkManager.IP6Config"
> >    dbus_iface = ".IP6Config"
> >     prop= "Ip6Config"
> > else:
> >     return []
> >
> > @@ -296,9 +305,7 @@ def nm_device_ip_config(name, version=4):
> >       if config == "/":
> >           return []
> > proxy = _get_proxy(object_path=config, interface_name=dbus_iface)
> > addresses = proxy.get_cached_property("Addresses").unpack()
> > addresses = _get_property(config, "Addresses", dbus_iface)
> >  addr_list = []
> >  for addr, prefix, gateway in addresses:
> >      # TODO - look for a library function (could have used IPy but byte order!)
> >
> > @@ -310,7 +317,7 @@ def nm_device_ip_config(name, version=4):
> >               gateway_str = nm_dbus_ay_to_ipv6(gateway)
> >           addr_list.append([addr_str, prefix, gateway_str])
> > nameservers = proxy.get_cached_property("Nameservers").unpack()
> > nameservers = _get_property(config, "Nameservers", dbus_iface)
> >  ns_list = []
> >  for ns in nameservers:
> >      # TODO - look for a library function
> >
> > @@ -322,7 +329,6 @@ def nm_device_ip_config(name, version=4):
> >   return [addr_list, ns_list]
> >
> > def nm_ntp_servers_from_dhcp():
> >     """Return a list of NTP servers that were specified the reply of the
> >        DHCP server or empty list if no NTP servers were returned.
> >
> > @@ -335,16 +341,15 @@ def nm_ntp_servers_from_dhcp():
> >       for device in active_devices:
> >           # harvest NTP server addresses from DHCPv4
> >           dhcp4_path = nm_device_property(device, "Dhcp4Config")
> >    dhcp4_proxy = _get_proxy(object_path=dhcp4_path,
> >            interface_name="org.freedesktop.NetworkManager.DHCP4Config")
> >    options = dhcp4_proxy.get_cached_property("Options")
> >    if options and 'ntp_servers' in options.unpack():
> >    options = _get_property(dhcp4_path, "Options", ".DHCP4Config")
> >    if options and 'ntp_servers' in options:
> >         # NTP server addresses returned by DHCP are whitespace delimited
> >        ntp_servers_string = options.unpack()["ntp_servers"]
> >        ntp_servers_string = options["ntp_servers"]
> >         for ip in ntp_servers_string.split(" "):
> >             ntp_servers.append(ip)
> >
> >     # NetworkManager does not request NTP/SNTP options for DHCP6
> >
> > print ntp_servers
>
> Debugging print?

Yes, thanks

Radek

[Vratislav Podzimek]

On Mon, 2013-09-09 at 12:13 +0200, Radek Vykydal wrote:
> On 09/09/2013 10:23 AM, Vratislav Podzimek wrote:
> > On Mon, 2013-09-09 at 09:41 +0200, Radek Vykydal wrote:
> > > Device has to be in ACTIVATED state for IPXConfig to be valid.
> > > use a recently added nm module function to get IP configuration, replacing
> > >
> > > python-dbus with GDBus here. Python-dbus would give us stale value.
> > > Don't use get_cached_property of GDBus as it would give us stale value too.
> > > Instead use DBus.Properties interface call to get current values.
> > >
> > > pyanaconda/nm.py                    | 77 ++++++++++++++++++++-----------------
> > >  pyanaconda/ui/gui/spokes/network.py | 64 +++++++++++-------------------
> > >  2 files changed, 63 insertions(+), 78 deletions(-)
> > >
> > > diff --git a/pyanaconda/nm.py b/pyanaconda/nm.py
> > > index 1109d6e..4f0923d 100644
> > > --- a/pyanaconda/nm.py
> > > +++ b/pyanaconda/nm.py
> > > @@ -73,10 +73,27 @@ def _get_proxy(bus_type=Gio.BusType.SYSTEM,
> > >                                             cancellable)
> > >      return proxy
> > >
> > > +def _get_property(object_path, prop, interface_name_suffix=""):
> > > interface_name = "org.freedesktop.NetworkManager" + interface_name_suffix
> > > proxy = _get_proxy(object_path=object_path, interface_name="org.freedesktop.DBus.Properties")
> > > args = GLib.Variant('(ss)', (interface_name, prop))
> > > try:
> > >    prop = proxy.call_sync("Get",
> > >                            args,
> > >                            Gio.DBusCallFlags.NONE,
> > >                            DEFAULT_DBUS_TIMEOUT,
> > >                            None)
> > > except GLib.GError as e:
> > >    if "org.freedesktop.DBus.Error.AccessDenied" in e.message:
> > >        return None
> > >    else:
> > >        raise
> > > return prop.unpack()[0]
> >
> > I'd suggest using dbus_get_property_safe_sync from
> > pyanaconda/safe_dbus.py. From what I recall using proxy.call_sync may
> > cause troubles if it was used in a non-main thread. The _get_property
> > function could stay almost the same, it should just handle exceptions
> > raised from the safe_dbus module.
>
> For now I'd assume that calling a method on a proxy synchronously is
> safe, as the doc says:
>
> "A GDBusProxy instance can be used from multiple threads but note that
> all signals (e.g. "g-signal", "g-properties-changed" and "notify") are
> emitted in the thread-default main loop of the thread where the
> instance was constructed."
>
> and I'll get back to dbus_get_property_safe_sync or to Glib guys if
> I hit similar issue. In this bz just calling DBus.Properties method
> instead
> using the properties cached by the proxy seems to fix the problem.
>
> (Checking the device state and then getting the config is racy anyway,
> but this is another story with the race condition being harder to
> hit.)
>
> I've peeked into the gdbus code and gdbus_call_proxy_sync is calling
> g_dbus_connection_call_sync (called by safe_dbus) so the only
> difference seems to be the connection object being used. With proxy
> in nm.py the connection object seems to by created each time, while
> in keyboard.py you are re-using a connection created in __init__ of
> LocaledWrapper. Also as you said in your case the connection is
> autostarting its service which is not the case with NM so perhaps
> this might really be the problem I don't hit with NM service.

Sounds good, then. :)