[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