Re: possibly ifcfg-rh: fix missing connections if unmanaged interface was present

2010-11-26 Thread Robert Vogelgesang
Hello,

On Sat, Nov 20, 2010 at 04:20:18PM +0300, Andrey Borzenkov wrote:
> Below is patch for ifcfg-mdv (Mandriva plugin that is currently
> in development). Plugin was forked off ifcfg-rh and this part
> is mostly unchanged. From code review the same problem should
> be present in ifcfg-rh as well.

thank you for your work on this bug, and the patch.

> 
> To reproduce - take the first ifcfg file in directory scan order
> and set it as unmanaged. In my case it results in no system
> connections being displayed by NM.

Please see my email to this list from 4-Oct-2010 with subject
"Strange NM 0.8.1 behaviour with static configuration", where I reported
my version of this problem.  You are describing exactly the same issue
in a very similar setup.

I've ported your patch over to the ifcfg-rh plugin; it fixes indeed the
problem I did describe on 4-Oct-2010.  A version of the patch for ifcfg-rh,
against the sources from NetworkManager-0.8.1-9.git20100831.fc12.src.rpm
for Fedora 12, is attached.

Robert

diff -u 
NetworkManager-0.8.1/system-settings/plugins/ifcfg-rh/plugin.c.unmanaged 
NetworkManager-0.8.1/system-settings/plugins/ifcfg-rh/plugin.c
--- NetworkManager-0.8.1/system-settings/plugins/ifcfg-rh/plugin.c.unmanaged
2010-08-31 17:23:22.0 +0200
+++ NetworkManager-0.8.1/system-settings/plugins/ifcfg-rh/plugin.c  
2010-11-26 23:23:41.223169721 +0100
@@ -147,7 +147,6 @@
if (nm_ifcfg_connection_get_unmanaged_spec (connection)) {
PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "Ignoring connection 
'%s' and its "
  "device due to 
NM_CONTROLLED/BRIDGE/VLAN.", cid);
-   g_signal_emit_by_name (plugin, 
NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED);
} else {
/* Wait for the connection to become unmanaged once it 
knows the
 * UDI of it's device, if/when the device gets plugged 
in.
@@ -305,7 +304,9 @@
if (do_new) {
connection = read_one_connection (plugin, path);
if (connection) {
-   if (!nm_ifcfg_connection_get_unmanaged_spec 
(connection))
+   if (nm_ifcfg_connection_get_unmanaged_spec (connection))
+   g_signal_emit_by_name (plugin, 
NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED);
+   else
g_signal_emit_by_name (plugin, 
NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection);
}
}
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


possibly ifcfg-rh: fix missing connections if unmanaged interface was present

2010-11-20 Thread Andrey Borzenkov
Below is patch for ifcfg-mdv (Mandriva plugin that is currently
in development). Plugin was forked off ifcfg-rh and this part
is mostly unchanged. From code review the same problem should
be present in ifcfg-rh as well.

To reproduce - take the first ifcfg file in directory scan order
and set it as unmanaged. In my case it results in no system
connections being displayed by NM.

---

NM starts plugin initialization by requesting list of
unmanaged interfaces. ifcfg-mdv reads in interface list on
demand either in get_connections() or in get_unmanaged_specs().
What now happens is:

get_unmanaged_specs() -> read_connections() -> read_one_connection()
-> signal("unmanaged-specs-changed") -> unmanaged_specs_changed()
-> nm_sysconfig_settings_get_unmanaged_specs() -> load_connections()
-> get_connections()

Now ifcfg-mdv get_connections() will see non-empty connection
hash table and return whatever is available. load_connections()
will mark connections as loaded and never re-request it. And
read_one_connection() never signals that connection was added so
NM will never notice existence of connections added *after* the
first unmanaged interface was found.

There are two places where read_one_connection() is called:
read_connections() and handle_connection_remove_or_new(). The
former is called exactly once on initialization and does not
need any signals - we want to read full list before interacting
with NM. Move signal("unmanaged-specs-changed") to the latter
from read_one_connection().

Signed-off-by: Andrey Borzenkov 

---

 system-settings/plugins/ifcfg-mdv/plugin.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/system-settings/plugins/ifcfg-mdv/plugin.c 
b/system-settings/plugins/ifcfg-mdv/plugin.c
index 4b4667b..68a2c72 100644
--- a/system-settings/plugins/ifcfg-mdv/plugin.c
+++ b/system-settings/plugins/ifcfg-mdv/plugin.c
@@ -135,7 +135,6 @@ read_one_connection (SCPluginIfcfg *plugin, const char 
*filename)
if (nm_ifcfg_connection_get_unmanaged_spec (connection)) {
PLUGIN_PRINT (IFCFG_PLUGIN_NAME, "Ignoring connection 
'%s' and its "
  "device due to 
NM_CONTROLLED/ONBOOT/BRIDGE/VLAN.", cid);
-   g_signal_emit_by_name (plugin, 
NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED);
} else {
/* Wait for the connection to become unmanaged once it 
knows the
 * UDI of it's device, if/when the device gets plugged 
in.
@@ -298,7 +297,9 @@ handle_connection_remove_or_new (SCPluginIfcfg *plugin,
if (do_new) {
connection = read_one_connection (plugin, path);
if (connection) {
-   if (!nm_ifcfg_connection_get_unmanaged_spec 
(connection))
+   if (nm_ifcfg_connection_get_unmanaged_spec (connection))
+   g_signal_emit_by_name (plugin, 
NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED);
+   else
g_signal_emit_by_name (plugin, 
NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection);
}
}
___
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list