Re: [PATCH 0/3] clean up config file handling
On Tuesday 15 of November 2011 18:23:19 Dan Williams wrote: > On Tue, 2011-11-15 at 13:38 +0100, Jirka Klimes wrote: > > On Friday 30 of September 2011 01:05:20 Dan Williams wrote: > > > On Thu, 2011-09-29 at 20:59 +0400, Yury G. Kudryashov wrote: > > > > Dan Williams wrote: > > > > > On Tue, 2011-09-27 at 10:27 -0500, Dan Williams wrote: > > > > >> Split config file reading and handling out of main.c so that it's > > > > >> easier to extend later on. > > > > > > > > > > And I accidentally pushed them. So if anyone has comments, let me > > > > > know and I'll push fixes. > > > > > > > > I saw that with the previous way of handling config file the plugins > > > > used /etc/NetworkManager/NetworkManager.conf even if NetworkManager > > > > is started with '-c' option. Is it still true? If yes, what about > > > > passing a 'config file handle' to plugins? > > > > > > You're right, that's still true. What should probably happen here is > > > that NMSettings should pass the config file path to the plugins when > > > they get created. Either we modify the factory function > > > (nm_system_config_factory) for each plugin, or we make the config file > > > a GObject property on the plugins that gets set by NMSettings right > > > after creating the plugin. I vote #1. Patches anyone? > > > > Solution #1 implemented, please review the patch. > > Looks good to me. > > Dan Pushed as 7b7e426b653a3a342d7b0522957283d0999a9070 Jirka ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH 0/3] clean up config file handling
On Tue, 2011-11-15 at 13:38 +0100, Jirka Klimes wrote: > On Friday 30 of September 2011 01:05:20 Dan Williams wrote: > > On Thu, 2011-09-29 at 20:59 +0400, Yury G. Kudryashov wrote: > > > Dan Williams wrote: > > > > On Tue, 2011-09-27 at 10:27 -0500, Dan Williams wrote: > > > >> Split config file reading and handling out of main.c so that it's > > > >> easier to extend later on. > > > > > > > > And I accidentally pushed them. So if anyone has comments, let me know > > > > and I'll push fixes. > > > > > > I saw that with the previous way of handling config file the plugins used > > > /etc/NetworkManager/NetworkManager.conf even if NetworkManager is started > > > with '-c' option. Is it still true? If yes, what about passing a 'config > > > file handle' to plugins? > > > > You're right, that's still true. What should probably happen here is > > that NMSettings should pass the config file path to the plugins when > > they get created. Either we modify the factory function > > (nm_system_config_factory) for each plugin, or we make the config file a > > GObject property on the plugins that gets set by NMSettings right after > > creating the plugin. I vote #1. Patches anyone? > > > > Solution #1 implemented, please review the patch. Looks good to me. Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH 0/3] clean up config file handling
On Friday 30 of September 2011 01:05:20 Dan Williams wrote:
> On Thu, 2011-09-29 at 20:59 +0400, Yury G. Kudryashov wrote:
> > Dan Williams wrote:
> > > On Tue, 2011-09-27 at 10:27 -0500, Dan Williams wrote:
> > >> Split config file reading and handling out of main.c so that it's
> > >> easier to extend later on.
> > >
> > > And I accidentally pushed them. So if anyone has comments, let me know
> > > and I'll push fixes.
> >
> > I saw that with the previous way of handling config file the plugins used
> > /etc/NetworkManager/NetworkManager.conf even if NetworkManager is started
> > with '-c' option. Is it still true? If yes, what about passing a 'config
> > file handle' to plugins?
>
> You're right, that's still true. What should probably happen here is
> that NMSettings should pass the config file path to the plugins when
> they get created. Either we modify the factory function
> (nm_system_config_factory) for each plugin, or we make the config file a
> GObject property on the plugins that gets set by NMSettings right after
> creating the plugin. I vote #1. Patches anyone?
>
Solution #1 implemented, please review the patch.
Jirka
From 2d97c5cb0efe9a2917496566b6be0b40cd7f3fce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?=
Date: Tue, 15 Nov 2011 13:30:16 +0100
Subject: [PATCH] settings: pass config file name to settings plugins
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Signed-off-by: Jiří Klimeš
---
src/settings/nm-settings.c|7 +++--
src/settings/nm-system-config-interface.h |2 +-
src/settings/plugins/ifcfg-rh/plugin.c|2 +-
src/settings/plugins/ifcfg-suse/plugin.c |4 +-
src/settings/plugins/ifnet/net_parser.c | 13 +++---
src/settings/plugins/ifnet/plugin.c | 35 -
src/settings/plugins/ifnet/plugin.h |2 +
src/settings/plugins/ifupdown/plugin.c| 28 +++---
src/settings/plugins/keyfile/plugin.c | 29 +++
src/settings/plugins/keyfile/plugin.h |4 +-
10 files changed, 78 insertions(+), 48 deletions(-)
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 7cf930a..141ab53 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -550,6 +550,7 @@ find_plugin (GSList *list, const char *pname)
static gboolean
load_plugins (NMSettings *self, const char **plugins, GError **error)
{
+ NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self);
GSList *list = NULL;
const char **iter;
gboolean success = TRUE;
@@ -559,7 +560,7 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
char *full_name, *path;
const char *pname = *iter;
GObject *obj;
- GObject * (*factory_func) (void);
+ GObject * (*factory_func) (const char *);
/* strip leading spaces */
while (isblank (*pname))
@@ -602,7 +603,7 @@ load_plugins (NMSettings *self, const char **plugins, GError **error)
break;
}
- obj = (*factory_func) ();
+ obj = (*factory_func) (priv->config_file);
if (!obj || !NM_IS_SYSTEM_CONFIG_INTERFACE (obj)) {
g_set_error (error, 0, 0,
"Plugin '%s' returned invalid system config object.",
@@ -1527,7 +1528,7 @@ nm_settings_new (const char *config_file,
}
/* Add the keyfile plugin last */
- keyfile_plugin = nm_settings_keyfile_plugin_new ();
+ keyfile_plugin = nm_settings_keyfile_plugin_new (config_file);
g_assert (keyfile_plugin);
add_plugin (self, NM_SYSTEM_CONFIG_INTERFACE (keyfile_plugin));
diff --git a/src/settings/nm-system-config-interface.h b/src/settings/nm-system-config-interface.h
index 96a6406..bbe74d4 100644
--- a/src/settings/nm-system-config-interface.h
+++ b/src/settings/nm-system-config-interface.h
@@ -39,7 +39,7 @@ G_BEGIN_DECLS
/* Plugin's factory function that returns a GObject that implements
* NMSystemConfigInterface.
*/
-GObject * nm_system_config_factory (void);
+GObject * nm_system_config_factory (const char *config_file);
#define NM_TYPE_SYSTEM_CONFIG_INTERFACE (nm_system_config_interface_get_type ())
#define NM_SYSTEM_CONFIG_INTERFACE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SYSTEM_CONFIG_INTERFACE, NMSystemConfigInterface))
diff --git a/src/settings/plugins/ifcfg-rh/plugin.c b/src/settings/plugins/ifcfg-rh/plugin.c
index b4be4cb..39c323c 100644
--- a/src/settings/plugins/ifcfg-rh/plugin.c
+++ b/src/settings/plugins/ifcfg-rh/plugin.c
@@ -790,7 +790,7 @@ system_config_interface_init (NMSystemConfigInterface *system_config_interface_c
}
G_MODULE_EXPORT GObject *
-nm_system_config_factory (void)
+nm_system_config_factory (const char *config_file)
{
static SCPluginIfcfg *singleton = NULL;
SCPluginIfcfgPrivate *priv;
diff --git a/src/settings/plugins/ifcfg-suse/plugin.c b/src/settings/plugins/ifcfg-suse/plugin.c
index 78db56e..2ec90d8 100644
--- a/src/settings/plugins/ifcfg-suse/plugin.c
+++ b/src/settings/plugins/if
Re: [PATCH 0/3] clean up config file handling
On Thu, 2011-09-29 at 20:59 +0400, Yury G. Kudryashov wrote: > Dan Williams wrote: > > > On Tue, 2011-09-27 at 10:27 -0500, Dan Williams wrote: > >> Split config file reading and handling out of main.c so that it's easier > >> to extend later on. > > > > And I accidentally pushed them. So if anyone has comments, let me know > > and I'll push fixes. > I saw that with the previous way of handling config file the plugins used > /etc/NetworkManager/NetworkManager.conf even if NetworkManager is started > with '-c' option. Is it still true? If yes, what about passing a 'config > file handle' to plugins? You're right, that's still true. What should probably happen here is that NMSettings should pass the config file path to the plugins when they get created. Either we modify the factory function (nm_system_config_factory) for each plugin, or we make the config file a GObject property on the plugins that gets set by NMSettings right after creating the plugin. I vote #1. Patches anyone? Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH 0/3] clean up config file handling
Dan Williams wrote: > On Tue, 2011-09-27 at 10:27 -0500, Dan Williams wrote: >> Split config file reading and handling out of main.c so that it's easier >> to extend later on. > > And I accidentally pushed them. So if anyone has comments, let me know > and I'll push fixes. I saw that with the previous way of handling config file the plugins used /etc/NetworkManager/NetworkManager.conf even if NetworkManager is started with '-c' option. Is it still true? If yes, what about passing a 'config file handle' to plugins? -- Yury G. Kudryashov, mailto: ur...@mccme.ru ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [PATCH 0/3] clean up config file handling
On Tue, 2011-09-27 at 10:27 -0500, Dan Williams wrote: > Split config file reading and handling out of main.c so that it's easier > to extend later on. And I accidentally pushed them. So if anyone has comments, let me know and I'll push fixes. Dan ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
[PATCH 0/3] clean up config file handling
Split config file reading and handling out of main.c so that it's easier to extend later on. ___ networkmanager-list mailing list networkmanager-list@gnome.org http://mail.gnome.org/mailman/listinfo/networkmanager-list
