Easy patch (help out!): preserve wireless-enabled and networking-enabled

[Dan Williams]

Anyone want to do a really simple patch?  It would close some bugs and
help out a lot:

NM 0.8 merged nm-system-settings and NetworkManager, and so now we have
a config file.  We should be using that config file to store the state
of nm-manager.c's priv->sleeping and priv->wireless_enabled.  Enable
Networking is really just a huge kludge that sets priv->sleeping=TRUE
internally, since the networking-disabled and suspend aren't really any
different (at some point we fix that, but whatever).

So any patch would update NetworkManager.c::parse_config_file() to grab
the initial values
of keys named "NetworkingEnabled" and "WirelessEnabled" out of the
config file, and pass those to nm_manager_get() when it's called in
NetworkManager.c::main().  nm_manager_get() would then use those
arguments to set the initial values of priv->sleeping and
priv->wireless_enabled.

Then in src/nm-manager.c, look for manager_set_wireless_enabled() and
impl_manager_sleep() we should write out the respective values to the
config file.  For a quick and dirty way of doing that, look in
src/system-settings/nm-sysconfig-settings.c::default_wired_deleted().

Dan


_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Tony Espy-2]

On 10/23/2009 01:19 PM, Dan Williams wrote:

> Anyone want to do a really simple patch?  It would close some bugs and
> help out a lot:
>
> NM 0.8 merged nm-system-settings and NetworkManager, and so now we have
> a config file.  We should be using that config file to store the state
> of nm-manager.c's priv->sleeping and priv->wireless_enabled.  Enable
> Networking is really just a huge kludge that sets priv->sleeping=TRUE
> internally, since the networking-disabled and suspend aren't really any
> different (at some point we fix that, but whatever).
>
> So any patch would update NetworkManager.c::parse_config_file() to grab
> the initial values
> of keys named "NetworkingEnabled" and "WirelessEnabled" out of the
> config file, and pass those to nm_manager_get() when it's called in
> NetworkManager.c::main().  nm_manager_get() would then use those
> arguments to set the initial values of priv->sleeping and
> priv->wireless_enabled.
>
> Then in src/nm-manager.c, look for manager_set_wireless_enabled() and
> impl_manager_sleep() we should write out the respective values to the
> config file.  For a quick and dirty way of doing that, look in
> src/system-settings/nm-sysconfig-settings.c::default_wired_deleted().
>    

sure, i'll take a stab at it.

ciao,
/tony

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

On Friday 23 October 2009 19:19:14 Dan Williams wrote:
> Anyone want to do a really simple patch?  It would close some bugs and
> help out a lot:
>

Hello Dan,

I've done the patch. Please, review it.

NetworkingEnabled and WirelessEnabled are read from config file. If not present,
the values are regarded as true; if case of mangled values (or something),
false is used.
On changing values in the applet, the keys are changed (created)
in the cofiguration file.

Jirka

[net_wifi_enable_config_file.patch]

---

diff --git a/src/NetworkManager.c b/src/NetworkManager.c
index f14c6cb..f91308d 100644
--- a/src/NetworkManager.c
+++ b/src/NetworkManager.c
@@ -246,7 +246,7 @@ write_pidfile (const char *pidfile)
 }
 
 static gboolean
-parse_config_file (const char *filename, char **plugins, GError **error)
+parse_config_file (const char *filename, NMConfigSettings *config_values, GError **error)
 {
  GKeyFile *config;
 
@@ -261,10 +261,20 @@ parse_config_file (const char *filename, char **plugins, GError **error)
  if (!g_key_file_load_from_file (config, filename, G_KEY_FILE_NONE, error))
  return FALSE;
 
- *plugins = g_key_file_get_value (config, "main", "plugins", error);
+ config_values->plugins = g_key_file_get_value (config, "main", "plugins", error);
  if (*error)
  return FALSE;
 
+ config_values->networking_enabled = g_key_file_get_boolean (config, "main", "NetworkingEnabled", error);
+ if (*error && (*error)->code == G_KEY_FILE_ERROR_KEY_NOT_FOUND)
+ config_values->networking_enabled = TRUE;
+ g_clear_error (error);
+
+ config_values->wireless_enabled = g_key_file_get_boolean (config, "main", "WirelessEnabled", error);
+ if (*error && (*error)->code == G_KEY_FILE_ERROR_KEY_NOT_FOUND)
+ config_values->wireless_enabled = TRUE;
+ g_clear_error (error);
+
  g_key_file_free (config);
  return TRUE;
 }
@@ -280,7 +290,8 @@ main (int argc, char *argv[])
  gboolean become_daemon = FALSE;
  gboolean g_fatal_warnings = FALSE;
  char *pidfile = NULL, *user_pidfile = NULL;
- char *config = NULL, *plugins = NULL;
+ char *config = NULL;
+ NMConfigSettings cfg_values = {NULL, TRUE, TRUE};
  gboolean success;
  NMPolicy *policy = NULL;
  NMVPNManager *vpn_manager = NULL;
@@ -295,7 +306,7 @@ main (int argc, char *argv[])
  { "g-fatal-warnings", 0, 0, G_OPTION_ARG_NONE, &g_fatal_warnings, "Make all warnings fatal", NULL },
  { "pid-file", 0, 0, G_OPTION_ARG_FILENAME, &user_pidfile, "Specify the location of a PID file", "filename" },
  { "config", 0, 0, G_OPTION_ARG_FILENAME, &config, "Config file location", "/path/to/config.file" },
- { "plugins", 0, 0, G_OPTION_ARG_STRING, &plugins, "List of plugins separated by ,", "plugin1,plugin2" },
+ { "plugins", 0, 0, G_OPTION_ARG_STRING, &cfg_values.plugins, "List of plugins separated by ,", "plugin1,plugin2" },
  {NULL}
  };
 
@@ -333,7 +344,7 @@ main (int argc, char *argv[])
 
  /* Parse the config file */
  if (config) {
- if (!parse_config_file (config, &plugins, &error)) {
+ if (!parse_config_file (config, &cfg_values, &error)) {
  g_warning ("Config file %s invalid: (%d) %s.",
            config,
            error ? error->code : -1,
@@ -342,7 +353,7 @@ main (int argc, char *argv[])
  }
  } else {
  config = NM_DEFAULT_SYSTEM_CONF_FILE;
- if (!parse_config_file (config, &plugins, &error)) {
+ if (!parse_config_file (config, &cfg_values, &error)) {
  g_warning ("Default config file %s invalid: (%d) %s.",
            config,
            error ? error->code : -1,
@@ -419,7 +430,7 @@ main (int argc, char *argv[])
  goto done;
  }
 
- manager = nm_manager_get (config, plugins, &error);
+ manager = nm_manager_get (config, &cfg_values, &error);
  if (manager == NULL) {
  nm_error ("Failed to initialize the network manager: %s",
           error && error->message ? error->message : "(unknown)");
diff --git a/src/nm-device-olpc-mesh.c b/src/nm-device-olpc-mesh.c
index 743f47c..5aa6966 100644
--- a/src/nm-device-olpc-mesh.c
+++ b/src/nm-device-olpc-mesh.c
@@ -898,6 +898,7 @@ check_companion_cb (gpointer user_data)
  NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self);
  NMManager *manager;
  GSList *list;
+ NMConfigSettings cfg_values = {NULL, TRUE, TRUE};
 
  if (priv->companion != NULL) {
  nm_device_state_changed (NM_DEVICE (user_data),
@@ -909,7 +910,7 @@ check_companion_cb (gpointer user_data)
  if (priv->device_added_cb != 0)
  return FALSE;
 
- manager = nm_manager_get (NULL, NULL, NULL);
+ manager = nm_manager_get (NULL, &cfg_values, NULL);
 
  priv->device_added_cb = g_signal_connect (manager, "device-added",
                                           G_CALLBACK (device_added_cb), self);
diff --git a/src/nm-manager.c b/src/nm-manager.c
index e082ab0..b488d88 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1105,6 +1105,9 @@ manager_set_wireless_enabled (NMManager *manager, gboolean enabled)
 {
  NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager);
  GSList *iter;
+ GKeyFile *config;
+ char *data;
+ gsize len = 0;
 
  if (priv->wireless_enabled == enabled)
  return;
@@ -1117,6 +1120,21 @@ manager_set_wireless_enabled (NMManager *manager, gboolean enabled)
 
  g_object_notify (G_OBJECT (manager), NM_MANAGER_WIRELESS_ENABLED);
 
+ /* If there was config file specified, update "WirelessEnabled" key */
+ if (priv->config_file) {
+ if ((config = g_key_file_new ()) != NULL) {
+ g_key_file_set_list_separator (config, ',');
+ g_key_file_load_from_file (config, priv->config_file, G_KEY_FILE_KEEP_COMMENTS, NULL);
+ g_key_file_set_boolean (config, "main", "WirelessEnabled", enabled);
+ data = g_key_file_to_data (config, &len, NULL);
+ if (data) {
+ g_file_set_contents (priv->config_file, data, len, NULL);
+ g_free (data);
+ }
+ g_key_file_free (config);
+ }
+ }
+
  /* Don't touch devices if asleep/networking disabled */
  if (priv->sleeping)
  return;
@@ -2385,6 +2403,9 @@ impl_manager_sleep (NMManager *self, gboolean sleep, GError **error)
 {
  NMManagerPrivate *priv;
  GSList *iter;
+ GKeyFile *config;
+ char *data;
+ gsize len = 0;
 
  g_return_val_if_fail (NM_IS_MANAGER (self), FALSE);
 
@@ -2399,6 +2420,21 @@ impl_manager_sleep (NMManager *self, gboolean sleep, GError **error)
 
  priv->sleeping = sleep;
 
+ /* If there was config file specified, update "NetworkingEnabled" key */
+ if (priv->config_file) {
+ if ((config = g_key_file_new ()) != NULL) {
+ g_key_file_set_list_separator (config, ',');
+ g_key_file_load_from_file (config, priv->config_file, G_KEY_FILE_KEEP_COMMENTS, NULL);
+ g_key_file_set_boolean (config, "main", "NetworkingEnabled", !sleep);
+ data = g_key_file_to_data (config, &len, NULL);
+ if (data) {
+ g_file_set_contents (priv->config_file, data, len, NULL);
+ g_free (data);
+ }
+ g_key_file_free (config);
+ }
+ }
+
  if (sleep) {
  nm_info ("Sleeping...");
 
@@ -2569,9 +2605,10 @@ nm_manager_start (NMManager *self)
  break;
  }
 
- nm_info ("Wireless now %s by radio killswitch",
-         (priv->wireless_hw_enabled && we) ? "enabled" : "disabled");
- manager_set_wireless_enabled (self, we);
+ nm_info ("Wireless %s by radio killswitch; %s by config file",
+         (priv->wireless_hw_enabled && we) ? "enabled" : "disabled",
+         (priv->wireless_enabled) ? "enabled" : "disabled");
+ manager_set_wireless_enabled (self, priv->wireless_enabled && we);
 
  system_unmanaged_devices_changed_cb (priv->sys_settings, NULL, self);
  system_hostname_changed_cb (priv->sys_settings, NULL, self);
@@ -2589,7 +2626,7 @@ nm_manager_start (NMManager *self)
 }
 
 NMManager *
-nm_manager_get (const char *config_file, const char *plugins, GError **error)
+nm_manager_get (const char *config_file, NMConfigSettings *config_values, GError **error)
 {
  static NMManager *singleton = NULL;
  NMManagerPrivate *priv;
@@ -2606,7 +2643,7 @@ nm_manager_get (const char *config_file, const char *plugins, GError **error)
  bus = nm_dbus_manager_get_connection (priv->dbus_mgr);
  g_assert (bus);
 
- priv->sys_settings = nm_sysconfig_settings_new (config_file, plugins, bus, error);
+ priv->sys_settings = nm_sysconfig_settings_new (config_file, config_values->plugins, bus, error);
  if (!priv->sys_settings) {
  g_object_unref (singleton);
  return NULL;
@@ -2615,6 +2652,10 @@ nm_manager_get (const char *config_file, const char *plugins, GError **error)
 
  priv->config_file = g_strdup (config_file);
 
+ priv->sleeping = !config_values->networking_enabled;
+
+ priv->wireless_enabled = config_values->wireless_enabled;
+
  g_signal_connect (priv->sys_settings, "notify::" NM_SYSCONFIG_SETTINGS_UNMANAGED_SPECS,
                   G_CALLBACK (system_unmanaged_devices_changed_cb), singleton);
  g_signal_connect (priv->sys_settings, "notify::" NM_SETTINGS_SYSTEM_INTERFACE_HOSTNAME,
diff --git a/src/nm-manager.h b/src/nm-manager.h
index 8e48574..f691533 100644
--- a/src/nm-manager.h
+++ b/src/nm-manager.h
@@ -74,7 +74,15 @@ typedef struct {
 
 GType nm_manager_get_type (void);
 
-NMManager *nm_manager_get (const char *config_file, const char *plugins, GError **error);
+typedef struct {
+ const char *plugins;
+ gboolean networking_enabled;
+ gboolean wireless_enabled;
+} NMConfigSettings;
+
+NMManager *nm_manager_get (const char *config_file,
+                           NMConfigSettings *config_values,
+                           GError **error);
 
 void nm_manager_start (NMManager *manager);
 

---

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Tony Espy-2]

On 10/27/2009 07:58 AM, Jirka Klimes wrote:

On Friday 23 October 2009 19:19:14 Dan Williams wrote:
  

Anyone want to do a really simple patch?  It would close some bugs and
help out a lot:

    

Hello Dan,

I've done the patch. Please, review it.
  

Jirka --

You beat me to the punch...  I finished up my version last night, but hadn't tested it yet.

You version is actually a little bit cleaner, as I hadn't created a new struct for the config parameters.  I've attached my version ( which is actually a quilt patch for Ubuntu ) for reference.

The one thing I think you may have missed, is checking for G_KEY_FILE_ERROR_INVALID_VALUE when reading the values from the file.  Other than that, looks good to me!

Regards,
/tony

NetworkingEnabled and WirelessEnabled are read from config file. If not present, 
the values are regarded as true; if case of mangled values (or something), 
false is used.
On changing values in the applet, the keys are changed (created)
in the cofiguration file.

Jirka
  

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list
  

[add-enabled-persistance-to-config-file.patch]

---

Index: network-manager-0.8~a~git.20091013t193206.679d548/src/NetworkManager.c
===================================================================
--- network-manager-0.8~a~git.20091013t193206.679d548.orig/src/NetworkManager.c 2009-10-26 21:29:13.647651662 -0400
+++ network-manager-0.8~a~git.20091013t193206.679d548/src/NetworkManager.c 2009-10-26 21:29:13.751628571 -0400
@@ -242,7 +242,11 @@
 }
 
 static gboolean
-parse_config_file (const char *filename, char **plugins, GError **error)
+parse_config_file (const char *filename,
+   char **plugins,
+   gboolean *net_enabled,
+   gboolean *wireless_enabled,
+   GError **error)
 {
  GKeyFile *config;
 
@@ -261,7 +265,35 @@
  if (*error)
  return FALSE;
 
- g_key_file_free (config);
+ *net_enabled = g_key_file_get_boolean (config, "main", "NetEnabled", error);
+ if (*error) {
+ if (g_error_matches(*error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND))
+ *net_enabled = TRUE;
+ else {
+ if (g_error_matches(*error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_INVALID_VALUE))
+ nm_error ("Invalid value found for NetEnabled in config file: %s", filename);
+ else
+ nm_error ("Unknown error reading NetEnabled from config file: %s", filename);
+
+ return FALSE;
+ }
+ }
+
+ *wireless_enabled = g_key_file_get_boolean (config, "main", "WirelessEnabled", error);
+ if (*error) {
+ if (g_error_matches(*error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND))
+ *wireless_enabled = TRUE;
+ else {
+ if (g_error_matches(*error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_INVALID_VALUE))
+ nm_error ("Invalid value found for WirelessEnabled in config file: %s", filename);
+ else
+ nm_error ("Unknown error reading WirelessEnabled from config file: %s", filename);
+
+ return FALSE;
+ }
+ }
+
+ g_key_file_free (config);
  return TRUE;
 }
 
@@ -275,6 +307,7 @@
  GOptionContext *opt_ctx = NULL;
  gboolean become_daemon = FALSE;
  gboolean g_fatal_warnings = FALSE;
+ gboolean net_enabled, wireless_enabled = TRUE;
  char *pidfile = NULL, *user_pidfile = NULL;
  char *config = NULL, *plugins = NULL;
  gboolean success;
@@ -329,7 +362,7 @@
 
  /* Parse the config file */
  if (config) {
- if (!parse_config_file (config, &plugins, &error)) {
+ if (!parse_config_file (config, &plugins, &net_enabled, &wireless_enabled, &error)) {
  g_warning ("Config file %s invalid: (%d) %s.",
            config,
            error ? error->code : -1,
@@ -338,7 +371,7 @@
  }
  } else {
  config = NM_DEFAULT_SYSTEM_CONF_FILE;
- if (!parse_config_file (config, &plugins, &error)) {
+ if (!parse_config_file (config, &plugins, &net_enabled, &wireless_enabled, &error)) {
  g_warning ("Default config file %s invalid: (%d) %s.",
            config,
            error ? error->code : -1,
@@ -415,7 +448,11 @@
  goto done;
  }
 
- manager = nm_manager_get (config, plugins, &error);
+ manager = nm_manager_get (config,
+  plugins,
+  net_enabled,
+  wireless_enabled,
+  &error);
  if (manager == NULL) {
  nm_error ("Failed to initialize the network manager: %s",
           error && error->message ? error->message : "(unknown)");
Index: network-manager-0.8~a~git.20091013t193206.679d548/src/nm-device-olpc-mesh.c
===================================================================
--- network-manager-0.8~a~git.20091013t193206.679d548.orig/src/nm-device-olpc-mesh.c 2009-10-14 18:03:07.000000000 -0400
+++ network-manager-0.8~a~git.20091013t193206.679d548/src/nm-device-olpc-mesh.c 2009-10-26 21:29:13.751628571 -0400
@@ -909,7 +909,8 @@
  if (priv->device_added_cb != 0)
  return FALSE;
 
- manager = nm_manager_get (NULL, NULL, NULL);
+ /* Used to get singleton, all parameters ignored... */
+ manager = nm_manager_get (NULL, NULL, FALSE, FALSE, NULL);
 
  priv->device_added_cb = g_signal_connect (manager, "device-added",
                                           G_CALLBACK (device_added_cb), self);
Index: network-manager-0.8~a~git.20091013t193206.679d548/src/nm-manager.c
===================================================================
--- network-manager-0.8~a~git.20091013t193206.679d548.orig/src/nm-manager.c 2009-10-26 21:29:13.663638141 -0400
+++ network-manager-0.8~a~git.20091013t193206.679d548/src/nm-manager.c 2009-10-26 21:34:59.518629892 -0400
@@ -1109,6 +1109,9 @@
 {
  NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager);
  GSList *iter;
+ GKeyFile *config;
+ gsize len = 0;
+ char *data;
 
  if (priv->wireless_enabled == enabled)
  return;
@@ -1130,6 +1133,20 @@
  if (NM_IS_DEVICE_WIFI (iter->data))
  nm_device_wifi_set_enabled (NM_DEVICE_WIFI (iter->data), enabled);
  }
+
+ config = g_key_file_new ();
+ if (config) {
+ /* Do I need to do this??? probably doesn't hurt... */
+ g_key_file_set_list_separator (config, ',');
+ g_key_file_load_from_file (config, priv->config_file, G_KEY_FILE_KEEP_COMMENTS, NULL);
+ g_key_file_set_boolean(config, "main", "WirelessEnabled", enabled);
+
+ data = g_key_file_to_data (config, &len, NULL);
+ if (data) {
+ g_file_set_contents (priv->config_file, data, len, NULL);
+ g_free (data);
+ }
+ }
 }
 
 static void
@@ -2366,7 +2383,10 @@
 impl_manager_sleep (NMManager *self, gboolean sleep, GError **error)
 {
  NMManagerPrivate *priv;
+ GKeyFile *config;
  GSList *iter;
+ gsize len = 0;
+ char *data;
 
  g_return_val_if_fail (NM_IS_MANAGER (self), FALSE);
 
@@ -2412,6 +2432,23 @@
  }
 
  nm_manager_update_state (self);
+
+ config = g_key_file_new ();
+ if (!config)
+ goto cleanup;
+
+ /* Do I need to do this??? probably doesn't hurt... */
+ g_key_file_set_list_separator (config, ',');
+ g_key_file_load_from_file (config, priv->config_file, G_KEY_FILE_KEEP_COMMENTS, NULL);
+ g_key_file_set_boolean(config, "main", "NetEnabled", !sleep);
+
+ data = g_key_file_to_data (config, &len, NULL);
+ if (data) {
+ g_file_set_contents (priv->config_file, data, len, NULL);
+ g_free (data);
+ }
+
+ cleanup:
  return TRUE;
 }
 
@@ -2569,7 +2606,11 @@
 }
 
 NMManager *
-nm_manager_get (const char *config_file, const char *plugins, GError **error)
+nm_manager_get (const char *config_file,
+ const char *plugins,
+ gboolean net_enabled,
+ gboolean wireless_enabled,
+ GError **error)
 {
  static NMManager *singleton = NULL;
  NMManagerPrivate *priv;
@@ -2586,6 +2627,9 @@
  bus = nm_dbus_manager_get_connection (priv->dbus_mgr);
  g_assert (bus);
 
+ priv->sleeping = !net_enabled;
+ priv->wireless_enabled = wireless_enabled;
+
  priv->sys_settings = nm_sysconfig_settings_new (config_file, plugins, bus, error);
  if (!priv->sys_settings) {
  g_object_unref (singleton);
Index: network-manager-0.8~a~git.20091013t193206.679d548/src/nm-manager.h
===================================================================
--- network-manager-0.8~a~git.20091013t193206.679d548.orig/src/nm-manager.h 2009-10-14 18:03:07.000000000 -0400
+++ network-manager-0.8~a~git.20091013t193206.679d548/src/nm-manager.h 2009-10-26 21:29:13.755651570 -0400
@@ -73,7 +73,11 @@
 
 GType nm_manager_get_type (void);
 
-NMManager *nm_manager_get (const char *config_file, const char *plugins, GError **error);
+NMManager *nm_manager_get (const char *config_file,
+   const char *plugins,
+   gboolean net_enabled,
+   gboolean wireless_enabled,
+   GError **error);
 
 void nm_manager_start (NMManager *manager);
 

---

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

On Tuesday 27 October 2009 15:04:51 Tony Espy wrote:

> On 10/27/2009 07:58 AM, Jirka Klimes wrote:
> > On Friday 23 October 2009 19:19:14 Dan Williams wrote:
> >> Anyone want to do a really simple patch?  It would close some bugs and
> >> help out a lot:
> >
> > Hello Dan,
> >
> > I've done the patch. Please, review it.
>
> Jirka --
>
> You beat me to the punch...  I finished up my version last night, but
> hadn't tested it yet.
>

Sorry for that. I hope you didn't spend much time on it.
Nevertheless it's good when more people check the stuff.

> You version is actually a little bit cleaner, as I hadn't created a new
> struct for the config parameters.  I've attached my version ( which is
> actually a quilt patch for Ubuntu ) for reference.
>
> The one thing I think you may have missed, is checking for
> G_KEY_FILE_ERROR_INVALID_VALUE when reading the values from the file.
> Other than that, looks good to me!
>

I'm aware of G_KEY_FILE_ERROR_INVALID_VALUE. However I decided not to produce
errors for reading the values.
So:
missing keys -         regarded as TRUE
erroneous values - regarded as FALSE

That's why no handling of G_KEY_FILE_ERROR_INVALID_VALUE.
But the behaviour can of course be changed if this is not what we want.

> Regards,
> /tony
>
> > NetworkingEnabled and WirelessEnabled are read from config file. If not
> > present, the values are regarded as true; if case of mangled values (or
> > something), false is used.
> > On changing values in the applet, the keys are changed (created)
> > in the cofiguration file.
> >
> > Jirka
> >

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Tony Espy-2]

On 10/27/2009 10:29 AM, Jirka Klimes wrote:

> On Tuesday 27 October 2009 15:04:51 Tony Espy wrote:
>    
>> On 10/27/2009 07:58 AM, Jirka Klimes wrote:
>>      
>>> On Friday 23 October 2009 19:19:14 Dan Williams wrote:
>>>        
>>>> Anyone want to do a really simple patch?  It would close some bugs and
>>>> help out a lot:
>>>>          
>>> Hello Dan,
>>>
>>> I've done the patch. Please, review it.
>>>        
>> Jirka --
>>
>> You beat me to the punch...  I finished up my version last night, but
>> hadn't tested it yet.
>>
>>      
> Sorry for that. I hope you didn't spend much time on it.
> Nevertheless it's good when more people check the stuff.
>    

No worries...

>> You version is actually a little bit cleaner, as I hadn't created a new
>> struct for the config parameters.  I've attached my version ( which is
>> actually a quilt patch for Ubuntu ) for reference.
>>
>> The one thing I think you may have missed, is checking for
>> G_KEY_FILE_ERROR_INVALID_VALUE when reading the values from the file.
>> Other than that, looks good to me!
>>
>>      
> I'm aware of G_KEY_FILE_ERROR_INVALID_VALUE. However I decided not to produce
> errors for reading the values.
> So:
> missing keys -         regarded as TRUE
> erroneous values - regarded as FALSE
>    

Well, the only problem I see is that a bad value means you could end up
with networking and/or wireless disabled.

That said, at least with your approach, a user can restore one or both
via the applet, whereas if we return an error, the user has to figure
out how to restart NetworkManger.

If we decide to stick with your approach, perhaps a log message is
warranted?

One last thing that's been eating at me a little, is that we're saving
program state to a configuration file.   As such, we could potentially
lose state if we decide to drop a new configuration file into place in a
future update.

[ Dan - any thoughts on creating a separate .state file instead? ]

Regards,
/tony

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Dan Williams]

On Tue, 2009-10-27 at 10:50 -0400, Tony Espy wrote:

> On 10/27/2009 10:29 AM, Jirka Klimes wrote:
> > On Tuesday 27 October 2009 15:04:51 Tony Espy wrote:
> >    
> >> On 10/27/2009 07:58 AM, Jirka Klimes wrote:
> >>      
> >>> On Friday 23 October 2009 19:19:14 Dan Williams wrote:
> >>>        
> >>>> Anyone want to do a really simple patch?  It would close some bugs and
> >>>> help out a lot:
> >>>>          
> >>> Hello Dan,
> >>>
> >>> I've done the patch. Please, review it.
> >>>        
> >> Jirka --
> >>
> >> You beat me to the punch...  I finished up my version last night, but
> >> hadn't tested it yet.
> >>
> >>      
> > Sorry for that. I hope you didn't spend much time on it.
> > Nevertheless it's good when more people check the stuff.
> >    
>
> No worries...
>
> >> You version is actually a little bit cleaner, as I hadn't created a new
> >> struct for the config parameters.  I've attached my version ( which is
> >> actually a quilt patch for Ubuntu ) for reference.
> >>
> >> The one thing I think you may have missed, is checking for
> >> G_KEY_FILE_ERROR_INVALID_VALUE when reading the values from the file.
> >> Other than that, looks good to me!
> >>
> >>      
> > I'm aware of G_KEY_FILE_ERROR_INVALID_VALUE. However I decided not to produce
> > errors for reading the values.
> > So:
> > missing keys -         regarded as TRUE
> > erroneous values - regarded as FALSE
> >    
>
> Well, the only problem I see is that a bad value means you could end up
> with networking and/or wireless disabled.
>
> That said, at least with your approach, a user can restore one or both
> via the applet, whereas if we return an error, the user has to figure
> out how to restart NetworkManger.
>
> If we decide to stick with your approach, perhaps a log message is
> warranted?
>
> One last thing that's been eating at me a little, is that we're saving
> program state to a configuration file.   As such, we could potentially
> lose state if we decide to drop a new configuration file into place in a
> future update.
>
> [ Dan - any thoughts on creating a separate .state file instead? ]

Yeah, that's probably valid.  I wonder where to put it though, since we
want this to be preserved across reboot, and /var/run or /var/lib isn't
guaranteed to be so, right?  That's usually what /etc is for.  Or are
you suggesting /etc/NetworkManager.state ?

Dan


_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Dan Williams]

On Tue, 2009-10-27 at 12:58 +0100, Jirka Klimes wrote:

> On Friday 23 October 2009 19:19:14 Dan Williams wrote:
> > Anyone want to do a really simple patch?  It would close some bugs and
> > help out a lot:
> >
>
> Hello Dan,
>
> I've done the patch. Please, review it.
>
> NetworkingEnabled and WirelessEnabled are read from config file. If not present,
> the values are regarded as true; if case of mangled values (or something),
> false is used.
> On changing values in the applet, the keys are changed (created)
> in the cofiguration file.

Thanks!  Looks good so far, but I can see the logic in Tony's comments
below where we shouldn't really mix state and configuration.  Not your
fault but mine when I initially proposed the solution.  So three things:

1) I think we should default to "on" for both network and wifi when we
have an error reading the state file

2) I guess we should leave the config file bits alone and do
an /etc/NetworkManager.state file instead?

3) I'd also consolidate the impl_manager_sleep() and
manager_set_wireless_enabled() bits into one helper function (since they
pretty much do the same thing)

Care to respin the patch?

Thanks!
Dan

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Witold Sowa]

Dan Williams pisze:
> 2) I guess we should leave the config file bits alone and do
> an /etc/NetworkManager.state file instead?

It's probably quite a distribution specific issue, but shouldn't we
store the state in /var instead of /etc ?

Regards,
Witek.
_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Dan Williams]

On Wed, 2009-10-28 at 20:02 +0100, Witold Sowa wrote:
> Dan Williams pisze:
> > 2) I guess we should leave the config file bits alone and do
> > an /etc/NetworkManager.state file instead?
>
> It's probably quite a distribution specific issue, but shouldn't we
> store the state in /var instead of /etc ?

Yes, was checking into that.  /var/lib apparently *must* persist across
reboot, so I believe we should do:

/var/lib/NetworkManager.state

Dan


_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Dan Williams]

On Wed, 2009-10-28 at 12:33 -0700, Dan Williams wrote:

> On Wed, 2009-10-28 at 20:02 +0100, Witold Sowa wrote:
> > Dan Williams pisze:
> > > 2) I guess we should leave the config file bits alone and do
> > > an /etc/NetworkManager.state file instead?
> >
> > It's probably quite a distribution specific issue, but shouldn't we
> > store the state in /var instead of /etc ?
>
> Yes, was checking into that.  /var/lib apparently *must* persist across
> reboot, so I believe we should do:
>
> /var/lib/NetworkManager.state

Further correction:

/var/lib/NetworkManager/NetworkManager.state

as convention dictates.

Dan


_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Eugene Crosser]

Dan Williams wrote:

> 2) I guess we should leave the config file bits alone and do
> an /etc/NetworkManager.state file instead?

Not sure what the Linux filesystem standard has to say about it, but it seems
that /var/something (like /var/lib) is a more appropriate place. /etc is more
for user configuration, and may be mounted r/o (I actually did that once on a
specialized installation).

Well, the "standard" does in fact say something:
http://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard

    var/lib/ State information. Persistent data modified by programs as they
run, e.g., databases, packaging system metadata, etc.

Eugene



_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[John Mahoney-4]

On Wed, Oct 28, 2009 at 3:33 PM, Dan Williams <dcbw@...> wrote:

On Wed, 2009-10-28 at 20:02 +0100, Witold Sowa wrote:
> Dan Williams pisze:
> > 2) I guess we should leave the config file bits alone and do
> > an /etc/NetworkManager.state file instead?
>
> It's probably quite a distribution specific issue, but shouldn't we
> store the state in /var instead of /etc ?

Yes, was checking into that.  /var/lib apparently *must* persist across
reboot, so I believe we should do:

What standard is that based on?  I am not disargeeing it is just that I have never heard this stated before.  I happen to work in an environment that tries to minimize writes and we map all of /var to a tmpfs.  I am not saying this is the norm but was courious what the source was you were basing this on.
 

/var/lib/NetworkManager.state

Dan

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Alexander Sack-6]

On Wed, Oct 28, 2009 at 11:50:26AM -0700, Dan Williams wrote:
>
> Yeah, that's probably valid.  I wonder where to put it though, since we
> want this to be preserved across reboot, and /var/run or /var/lib isn't
> guaranteed to be so, right?  That's usually what /etc is for.  Or are
> you suggesting /etc/NetworkManager.state ?

application state that isn't user configuration should go into /var ... so
please do not use /etc to write stuff out that is not supposed to be manually
configured by system admins.

That said, /var/lib sounds good to me. Why do you think its not persistent across
reboots?

 - Alexander

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Alexander Sack-5]

On Thu, Oct 29, 2009 at 12:47:11AM -0400, John Mahoney wrote:

>    On Wed, Oct 28, 2009 at 3:33 PM, Dan Williams <[1]dcbw@...> wrote:
>
>      On Wed, 2009-10-28 at 20:02 +0100, Witold Sowa wrote:
>      > Dan Williams pisze:
>      > > 2) I guess we should leave the config file bits alone and do
>      > > an /etc/NetworkManager.state file instead?
>      >
>      > It's probably quite a distribution specific issue, but shouldn't we
>      > store the state in /var instead of /etc ?
>
>      Yes, was checking into that. �/var/lib apparently *must* persist across
>      reboot, so I believe we should do:
>
>    What standard is that based on?� I am not disargeeing it is just that I
>    have never heard this stated before.� I happen to work in an environment
>    that tries to minimize writes and we map all of /var to a tmpfs.� I am not
>    saying this is the norm but was courious what the source was you were
>    basing this on.

Thats a bogus setup ... /var on tmpfs is wrong ... /var/cache/ and /var/log on tmpfs
is ok ... /var/tmp/ i was told is also technically wrong to be a tmpfs, but probably
won't break systems.

 - Alexander

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

On Wednesday 28 October 2009 19:53:56 Dan Williams wrote:

> On Tue, 2009-10-27 at 12:58 +0100, Jirka Klimes wrote:
> > On Friday 23 October 2009 19:19:14 Dan Williams wrote:
> > > Anyone want to do a really simple patch?  It would close some bugs and
> > > help out a lot:
> >
> > Hello Dan,
> >
> > I've done the patch. Please, review it.
> >
> > NetworkingEnabled and WirelessEnabled are read from config file. If not
> > present, the values are regarded as true; if case of mangled values (or
> > something), false is used.
> > On changing values in the applet, the keys are changed (created)
> > in the cofiguration file.
>
> Thanks!  Looks good so far, but I can see the logic in Tony's comments
> below where we shouldn't really mix state and configuration.  Not your
> fault but mine when I initially proposed the solution.  So three things:
>
> 1) I think we should default to "on" for both network and wifi when we
> have an error reading the state file
>
> 2) I guess we should leave the config file bits alone and do
> an /etc/NetworkManager.state file instead?
>
> 3) I'd also consolidate the impl_manager_sleep() and
> manager_set_wireless_enabled() bits into one helper function (since they
> pretty much do the same thing)
>
> Care to respin the patch?
>
> Thanks!
> Dan
>

Hello,

so here is the patch with the changes incorporated.

Cheers,
Jirka

[state_file.patch]

---

diff --git a/src/NetworkManager.c b/src/NetworkManager.c
index f14c6cb..9ab7709 100644
--- a/src/NetworkManager.c
+++ b/src/NetworkManager.c
@@ -56,6 +56,7 @@
 
 #define NM_DEFAULT_PID_FILE LOCALSTATEDIR"/run/NetworkManager.pid"
 #define NM_DEFAULT_SYSTEM_CONF_FILE SYSCONFDIR"/NetworkManager/nm-system-settings.conf"
+#define NM_DEFAULT_SYSTEM_STATE_FILE LOCALSTATEDIR"/lib/NetworkManager/NetworkManager.state"
 
 /*
  * Globals
@@ -246,7 +247,7 @@ write_pidfile (const char *pidfile)
 }
 
 static gboolean
-parse_config_file (const char *filename, char **plugins, GError **error)
+parse_config_file (const char *filename, NMConfigSettings *config_values, GError **error)
 {
  GKeyFile *config;
 
@@ -261,7 +262,7 @@ parse_config_file (const char *filename, char **plugins, GError **error)
  if (!g_key_file_load_from_file (config, filename, G_KEY_FILE_NONE, error))
  return FALSE;
 
- *plugins = g_key_file_get_value (config, "main", "plugins", error);
+ config_values->plugins = g_key_file_get_value (config, "main", "plugins", error);
  if (*error)
  return FALSE;
 
@@ -269,6 +270,37 @@ parse_config_file (const char *filename, char **plugins, GError **error)
  return TRUE;
 }
 
+static gboolean
+parse_state_file (const char *filename, NMStateSettings *state_values, GError **error)
+{
+ GKeyFile *state_file;
+
+ state_file = g_key_file_new ();
+ if (!state_file) {
+ g_set_error (error, 0, 0,
+             "Not enough memory to load state file.");
+ return FALSE;
+ }
+
+ g_key_file_set_list_separator (state_file, ',');
+ if (!g_key_file_load_from_file (state_file, filename, G_KEY_FILE_NONE, error))
+ return FALSE;
+
+ /* Reading state bits of NetworkManager; an error defaults to TRUE */
+ state_values->networking_enabled = g_key_file_get_boolean (state_file, "main", "NetworkingEnabled", error);
+ if (*error)
+ state_values->networking_enabled = TRUE;
+ g_clear_error (error);
+
+ state_values->wireless_enabled = g_key_file_get_boolean (state_file, "main", "WirelessEnabled", error);
+ if (*error)
+ state_values->wireless_enabled = TRUE;
+ g_clear_error (error);
+
+ g_key_file_free (state_file);
+ return TRUE;
+}
+
 /*
  * main
  *
@@ -280,7 +312,10 @@ main (int argc, char *argv[])
  gboolean become_daemon = FALSE;
  gboolean g_fatal_warnings = FALSE;
  char *pidfile = NULL, *user_pidfile = NULL;
- char *config = NULL, *plugins = NULL;
+ char *config = NULL;
+ char *state_file = NM_DEFAULT_SYSTEM_STATE_FILE;
+ NMConfigSettings cfg_values = {NULL};
+ NMStateSettings state_values = {TRUE, TRUE};
  gboolean success;
  NMPolicy *policy = NULL;
  NMVPNManager *vpn_manager = NULL;
@@ -294,8 +329,9 @@ main (int argc, char *argv[])
  { "no-daemon", 0, 0, G_OPTION_ARG_NONE, &become_daemon, "Don't become a daemon", NULL },
  { "g-fatal-warnings", 0, 0, G_OPTION_ARG_NONE, &g_fatal_warnings, "Make all warnings fatal", NULL },
  { "pid-file", 0, 0, G_OPTION_ARG_FILENAME, &user_pidfile, "Specify the location of a PID file", "filename" },
+ { "state-file", 0, 0, G_OPTION_ARG_FILENAME, &state_file, "State file location", "/path/to/state.file" },
  { "config", 0, 0, G_OPTION_ARG_FILENAME, &config, "Config file location", "/path/to/config.file" },
- { "plugins", 0, 0, G_OPTION_ARG_STRING, &plugins, "List of plugins separated by ,", "plugin1,plugin2" },
+ { "plugins", 0, 0, G_OPTION_ARG_STRING, &cfg_values.plugins, "List of plugins separated by ,", "plugin1,plugin2" },
  {NULL}
  };
 
@@ -333,7 +369,7 @@ main (int argc, char *argv[])
 
  /* Parse the config file */
  if (config) {
- if (!parse_config_file (config, &plugins, &error)) {
+ if (!parse_config_file (config, &cfg_values, &error)) {
  g_warning ("Config file %s invalid: (%d) %s.",
            config,
            error ? error->code : -1,
@@ -342,7 +378,7 @@ main (int argc, char *argv[])
  }
  } else {
  config = NM_DEFAULT_SYSTEM_CONF_FILE;
- if (!parse_config_file (config, &plugins, &error)) {
+ if (!parse_config_file (config, &cfg_values, &error)) {
  g_warning ("Default config file %s invalid: (%d) %s.",
            config,
            error ? error->code : -1,
@@ -352,6 +388,17 @@ main (int argc, char *argv[])
  }
  }
 
+ g_clear_error (&error);
+
+ /* Parse the state file */
+ if (!parse_state_file (state_file, &state_values, &error)) {
+ g_warning ("State file %s invalid: (%d) %s.",
+           state_file,
+           error ? error->code : -1,
+           (error && error->message) ? error->message : "unknown");
+ /* Not a hard failure */
+ }
+
  pidfile = g_strdup (user_pidfile ? user_pidfile : NM_DEFAULT_PID_FILE);
 
  /* Tricky: become_daemon is FALSE by default, so unless it's TRUE because
@@ -419,7 +466,7 @@ main (int argc, char *argv[])
  goto done;
  }
 
- manager = nm_manager_get (config, plugins, &error);
+ manager = nm_manager_get (config, &cfg_values, state_file, &state_values, &error);
  if (manager == NULL) {
  nm_error ("Failed to initialize the network manager: %s",
           error && error->message ? error->message : "(unknown)");
diff --git a/src/nm-device-olpc-mesh.c b/src/nm-device-olpc-mesh.c
index 743f47c..44d792f 100644
--- a/src/nm-device-olpc-mesh.c
+++ b/src/nm-device-olpc-mesh.c
@@ -898,6 +898,8 @@ check_companion_cb (gpointer user_data)
  NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self);
  NMManager *manager;
  GSList *list;
+ NMConfigSettings conf_values = {NULL};
+ NMStateSettings state_values = {TRUE, TRUE};
 
  if (priv->companion != NULL) {
  nm_device_state_changed (NM_DEVICE (user_data),
@@ -909,7 +911,7 @@ check_companion_cb (gpointer user_data)
  if (priv->device_added_cb != 0)
  return FALSE;
 
- manager = nm_manager_get (NULL, NULL, NULL);
+ manager = nm_manager_get (NULL, &conf_values, NULL, &state_values, NULL);
 
  priv->device_added_cb = g_signal_connect (manager, "device-added",
                                           G_CALLBACK (device_added_cb), self);
diff --git a/src/nm-manager.c b/src/nm-manager.c
index e082ab0..2e6004c 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -149,6 +149,7 @@ typedef struct {
 
 typedef struct {
  char *config_file;
+ char *state_file;
 
  GSList *devices;
  NMState state;
@@ -1100,11 +1101,55 @@ nm_manager_name_owner_changed (NMDBusManager *mgr,
  }
 }
 
+/* Store value into key-file; supported types: boolean, int, string */
+static gboolean
+write_value_to_key_file (const char *filename, const char *group, const char *key,
+                         GType value_type, gpointer value, GError **error)
+{
+ GKeyFile *key_file;
+ char *data;
+ gsize len = 0;
+ gboolean ret = FALSE;
+
+ g_return_val_if_fail (filename != NULL, FALSE);
+ g_return_val_if_fail (group != NULL, FALSE);
+ g_return_val_if_fail (key != NULL, FALSE);
+ g_return_val_if_fail (value_type == G_TYPE_BOOLEAN ||
+                      value_type == G_TYPE_INT ||
+                      value_type == G_TYPE_STRING,
+                      FALSE);
+
+ if ((key_file = g_key_file_new ()) != NULL) {
+ g_key_file_set_list_separator (key_file, ',');
+ g_key_file_load_from_file (key_file, filename, G_KEY_FILE_KEEP_COMMENTS, NULL);
+ switch (value_type) {
+ case G_TYPE_BOOLEAN:
+ g_key_file_set_boolean (key_file, group, key, *((gboolean *) value));
+ break;
+ case G_TYPE_INT:
+ g_key_file_set_integer (key_file, group, key, *((gint *) value));
+ break;
+ case G_TYPE_STRING:
+ g_key_file_set_string (key_file, group, key, *((const gchar **) value));
+ break;
+ }
+ data = g_key_file_to_data (key_file, &len, NULL);
+ if (data) {
+ ret = g_file_set_contents (filename, data, len, error);
+ g_free (data);
+ }
+ g_key_file_free (key_file);
+ }
+
+ return ret;
+}
+
 static void
 manager_set_wireless_enabled (NMManager *manager, gboolean enabled)
 {
  NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager);
  GSList *iter;
+ GError *error = NULL;
 
  if (priv->wireless_enabled == enabled)
  return;
@@ -1117,6 +1162,17 @@ manager_set_wireless_enabled (NMManager *manager, gboolean enabled)
 
  g_object_notify (G_OBJECT (manager), NM_MANAGER_WIRELESS_ENABLED);
 
+ /* Update "WirelessEnabled" key in state file */
+ if (priv->state_file)
+ if (!write_value_to_key_file (priv->state_file, "main", "WirelessEnabled",
+                              G_TYPE_BOOLEAN, (gpointer) &priv->wireless_enabled,
+                              &error)) {
+ g_warning ("Writting to state file %s failed: (%d) %s.",
+           priv->state_file,
+           error ? error->code : -1,
+           (error && error->message) ? error->message : "unknown");
+ }
+
  /* Don't touch devices if asleep/networking disabled */
  if (priv->sleeping)
  return;
@@ -2399,6 +2455,21 @@ impl_manager_sleep (NMManager *self, gboolean sleep, GError **error)
 
  priv->sleeping = sleep;
 
+ /* Update "NetworkingEnabled" key in state file */
+ if (priv->state_file) {
+ GError *err = NULL;
+ gboolean networking_enabled = !sleep;
+ if (!write_value_to_key_file (priv->state_file, "main", "NetworkingEnabled",
+                              G_TYPE_BOOLEAN, (gpointer) &networking_enabled,
+                              &err)) {
+ g_warning ("Writting to state file %s failed: (%d) %s.",
+           priv->state_file,
+           err ? err->code : -1,
+           (err && err->message) ? err->message : "unknown");
+ }
+
+ }
+
  if (sleep) {
  nm_info ("Sleeping...");
 
@@ -2569,9 +2640,10 @@ nm_manager_start (NMManager *self)
  break;
  }
 
- nm_info ("Wireless now %s by radio killswitch",
-         (priv->wireless_hw_enabled && we) ? "enabled" : "disabled");
- manager_set_wireless_enabled (self, we);
+ nm_info ("Wireless %s by radio killswitch; %s by state file",
+         (priv->wireless_hw_enabled && we) ? "enabled" : "disabled",
+         (priv->wireless_enabled) ? "enabled" : "disabled");
+ manager_set_wireless_enabled (self, priv->wireless_enabled && we);
 
  system_unmanaged_devices_changed_cb (priv->sys_settings, NULL, self);
  system_hostname_changed_cb (priv->sys_settings, NULL, self);
@@ -2589,7 +2661,9 @@ nm_manager_start (NMManager *self)
 }
 
 NMManager *
-nm_manager_get (const char *config_file, const char *plugins, GError **error)
+nm_manager_get (const char *config_file, NMConfigSettings *config_values,
+                const char *state_file, NMStateSettings *state_values,
+                GError **error)
 {
  static NMManager *singleton = NULL;
  NMManagerPrivate *priv;
@@ -2606,7 +2680,7 @@ nm_manager_get (const char *config_file, const char *plugins, GError **error)
  bus = nm_dbus_manager_get_connection (priv->dbus_mgr);
  g_assert (bus);
 
- priv->sys_settings = nm_sysconfig_settings_new (config_file, plugins, bus, error);
+ priv->sys_settings = nm_sysconfig_settings_new (config_file, config_values->plugins, bus, error);
  if (!priv->sys_settings) {
  g_object_unref (singleton);
  return NULL;
@@ -2615,6 +2689,12 @@ nm_manager_get (const char *config_file, const char *plugins, GError **error)
 
  priv->config_file = g_strdup (config_file);
 
+ priv->state_file = g_strdup (state_file);
+
+ priv->sleeping = !state_values->networking_enabled;
+
+ priv->wireless_enabled = state_values->wireless_enabled;
+
  g_signal_connect (priv->sys_settings, "notify::" NM_SYSCONFIG_SETTINGS_UNMANAGED_SPECS,
                   G_CALLBACK (system_unmanaged_devices_changed_cb), singleton);
  g_signal_connect (priv->sys_settings, "notify::" NM_SETTINGS_SYSTEM_INTERFACE_HOSTNAME,
diff --git a/src/nm-manager.h b/src/nm-manager.h
index 8e48574..947af6a 100644
--- a/src/nm-manager.h
+++ b/src/nm-manager.h
@@ -74,7 +74,18 @@ typedef struct {
 
 GType nm_manager_get_type (void);
 
-NMManager *nm_manager_get (const char *config_file, const char *plugins, GError **error);
+typedef struct {
+ const char *plugins;
+} NMConfigSettings;
+
+typedef struct {
+ gboolean networking_enabled;
+ gboolean wireless_enabled;
+} NMStateSettings;
+
+NMManager *nm_manager_get (const char *config_file, NMConfigSettings *config_values,
+                           const char *state_file, NMStateSettings *state_values,
+                           GError **error);
 
 void nm_manager_start (NMManager *manager);
 

---

_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list

[Dan Williams]

On Fri, 2009-10-30 at 11:04 +0100, Jirka Klimes wrote:

> On Wednesday 28 October 2009 19:53:56 Dan Williams wrote:
> > On Tue, 2009-10-27 at 12:58 +0100, Jirka Klimes wrote:
> > > On Friday 23 October 2009 19:19:14 Dan Williams wrote:
> > > > Anyone want to do a really simple patch?  It would close some bugs and
> > > > help out a lot:
> > >
> > > Hello Dan,
> > >
> > > I've done the patch. Please, review it.
> > >
> > > NetworkingEnabled and WirelessEnabled are read from config file. If not
> > > present, the values are regarded as true; if case of mangled values (or
> > > something), false is used.
> > > On changing values in the applet, the keys are changed (created)
> > > in the cofiguration file.
> >
> > Thanks!  Looks good so far, but I can see the logic in Tony's comments
> > below where we shouldn't really mix state and configuration.  Not your
> > fault but mine when I initially proposed the solution.  So three things:
> >
> > 1) I think we should default to "on" for both network and wifi when we
> > have an error reading the state file
> >
> > 2) I guess we should leave the config file bits alone and do
> > an /etc/NetworkManager.state file instead?
> >
> > 3) I'd also consolidate the impl_manager_sleep() and
> > manager_set_wireless_enabled() bits into one helper function (since they
> > pretty much do the same thing)
> >
> > Care to respin the patch?
> >
> > Thanks!
> > Dan
> >
>
> Hello,
>
> so here is the patch with the changes incorporated.

Thanks, committed to master and stable with a few cleanups.

Dan


_______________________________________________
NetworkManager-list mailing list
NetworkManager-list@...
http://mail.gnome.org/mailman/listinfo/networkmanager-list