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

Vratislav Podzimek vpodzime at redhat.com
Mon Sep 9 12:12:38 UTC 2013


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.
> > > 
> > > 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]
> > 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. :)

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list