GNOME.org
 

Re: [PATCH] Easy patch (help out!): preserve wireless-enabled and networking-enabled

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

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);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]