config: refactor change-flags to be a cause/reason which triggered the change

For the most part, this patch just renames some change-flags, but
doesn't change much about them. The new name should better express
what they are.

A config-change signal can be emitted for different reasons:
when we receive a signal (SIGHUP, SIGUSR1, SIGUSR2) or for internal
reasons like resetting of no-auto-default or setting internal
values.

Depending on the reason, we want to perform different actions.
For example:
 - we reload the configuration from disk on SIGHUP, but not for
   SIGUSR1.
 - For SIGUSR1 and SIGHUP, we want to update-dns, but not for SIGUSR2.

Another part of the change-flags encodes which part of the configuration
actually changed. Often, these parts can only change when re-reading
from disk (e.g. a SIGUSR1 will not change any configuration inside
NMConfig).

Later, we will have more causes, and accordingly more fine-grained
effects of what should be done on reload.
This commit is contained in:
Thomas Haller 2016-05-30 14:53:27 +02:00
parent ec89bd5171
commit eb6140a772
8 changed files with 119 additions and 67 deletions

View file

@ -1610,17 +1610,17 @@ config_changed_cb (NMConfig *config,
if (NM_FLAGS_ANY (changes, NM_CONFIG_CHANGE_DNS_MODE |
NM_CONFIG_CHANGE_RC_MANAGER |
NM_CONFIG_CHANGE_SIGHUP)) {
NM_CONFIG_CHANGE_CAUSE_SIGHUP)) {
/* reload the resolv-conf mode also on SIGHUP (when DNS_MODE didn't change).
* The reason is, that the configuration also depends on whether resolv.conf
* is immutable, thus, without the configuration changing, we always want to
* re-configure the mode. */
init_resolv_conf_mode (self,
NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_SIGHUP));
NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_CAUSE_SIGHUP));
}
if (NM_FLAGS_ANY (changes, NM_CONFIG_CHANGE_SIGHUP |
NM_CONFIG_CHANGE_SIGUSR1 |
if (NM_FLAGS_ANY (changes, NM_CONFIG_CHANGE_CAUSE_SIGHUP |
NM_CONFIG_CHANGE_CAUSE_SIGUSR1 |
NM_CONFIG_CHANGE_DNS_MODE |
NM_CONFIG_CHANGE_RC_MANAGER |
NM_CONFIG_CHANGE_GLOBAL_DNS_CONFIG)) {

View file

@ -116,14 +116,31 @@ _init_nm_debug (const char *debug)
void
nm_main_config_reload (int signal)
{
NMConfigChangeFlags reload_flags;
switch (signal) {
case SIGHUP:
reload_flags = NM_CONFIG_CHANGE_CAUSE_SIGHUP;
break;
case SIGUSR1:
reload_flags = NM_CONFIG_CHANGE_CAUSE_SIGUSR1;
break;
case SIGUSR2:
reload_flags = NM_CONFIG_CHANGE_CAUSE_SIGUSR2;
break;
default:
g_return_if_reached ();
}
nm_log_info (LOGD_CORE, "reload configuration (signal %s)...", strsignal (signal));
/* The signal handler thread is only installed after
* creating NMConfig instance, and on shut down we
* no longer run the mainloop (to reach this point).
*
* Hence, a NMConfig singleton instance must always be
* available. */
nm_config_reload (nm_config_get (), signal);
nm_config_reload (nm_config_get (), reload_flags);
}
static void

View file

@ -1201,6 +1201,8 @@ nm_config_data_diff (NMConfigData *old_data, NMConfigData *new_data)
if (!global_dns_equal (priv_old->global_dns, priv_new->global_dns))
changes |= NM_CONFIG_CHANGE_GLOBAL_DNS_CONFIG;
nm_assert (!NM_FLAGS_ANY (changes, NM_CONFIG_CHANGE_CAUSES));
return changes;
}

View file

@ -64,22 +64,51 @@ typedef enum { /*<flags >*/
typedef enum { /*< flags >*/
NM_CONFIG_CHANGE_NONE = 0,
NM_CONFIG_CHANGE_SIGHUP = (1L << 0),
NM_CONFIG_CHANGE_SIGUSR1 = (1L << 1),
NM_CONFIG_CHANGE_SIGUSR2 = (1L << 2),
/**************************************************************************
* The external cause which triggered the reload/configuration-change
*************************************************************************/
NM_CONFIG_CHANGE_CONFIG_FILES = (1L << 3),
NM_CONFIG_CHANGE_VALUES = (1L << 4),
NM_CONFIG_CHANGE_VALUES_USER = (1L << 5),
NM_CONFIG_CHANGE_VALUES_INTERN = (1L << 6),
NM_CONFIG_CHANGE_CONNECTIVITY = (1L << 7),
NM_CONFIG_CHANGE_NO_AUTO_DEFAULT = (1L << 8),
NM_CONFIG_CHANGE_DNS_MODE = (1L << 9),
NM_CONFIG_CHANGE_RC_MANAGER = (1L << 10),
NM_CONFIG_CHANGE_GLOBAL_DNS_CONFIG = (1L << 11),
NM_CONFIG_CHANGE_CAUSE_SIGHUP = (1L << 0),
NM_CONFIG_CHANGE_CAUSE_SIGUSR1 = (1L << 1),
NM_CONFIG_CHANGE_CAUSE_SIGUSR2 = (1L << 2),
NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT = (1L << 3),
NM_CONFIG_CHANGE_CAUSE_SET_VALUES = (1L << 4),
NM_CONFIG_CHANGE_CAUSES = ((1L << 5) - 1),
/**************************************************************************
* Following flags describe which property of the configuration changed:
*************************************************************************/
/* main-file or config-description changed */
NM_CONFIG_CHANGE_CONFIG_FILES = (1L << 10),
/* any configuration on disk changed */
NM_CONFIG_CHANGE_VALUES = (1L << 11),
/* any user configuration on disk changed (NetworkManager.conf) */
NM_CONFIG_CHANGE_VALUES_USER = (1L << 12),
/* any internal configuration on disk changed (NetworkManager-intern.conf) */
NM_CONFIG_CHANGE_VALUES_INTERN = (1L << 13),
/* configuration regarding connectivity changed */
NM_CONFIG_CHANGE_CONNECTIVITY = (1L << 14),
/* configuration regarding no-auto-default changed */
NM_CONFIG_CHANGE_NO_AUTO_DEFAULT = (1L << 15),
/* configuration regarding dns-mode changed */
NM_CONFIG_CHANGE_DNS_MODE = (1L << 16),
/* configuration regarding rc-manager changed */
NM_CONFIG_CHANGE_RC_MANAGER = (1L << 17),
/* configuration regarding global dns-config changed */
NM_CONFIG_CHANGE_GLOBAL_DNS_CONFIG = (1L << 18),
_NM_CONFIG_CHANGE_LAST,
NM_CONFIG_CHANGE_ALL = ((_NM_CONFIG_CHANGE_LAST - 1) << 1) - 1,
} NMConfigChangeFlags;
struct _NMConfigData {

View file

@ -141,7 +141,7 @@ G_DEFINE_TYPE_WITH_CODE (NMConfig, nm_config, G_TYPE_OBJECT,
/************************************************************************/
static void _set_config_data (NMConfig *self, NMConfigData *new_data, int signal);
static void _set_config_data (NMConfig *self, NMConfigData *new_data, NMConfigChangeFlags reload_flags);
/************************************************************************/
@ -425,7 +425,7 @@ nm_config_set_no_auto_default_for_device (NMConfig *self, NMDevice *device)
/* unref no_auto_default_set here. Note that _set_config_data() probably invalidates the content of the array. */
g_ptr_array_unref (no_auto_default_new);
_set_config_data (self, new_data, 0);
_set_config_data (self, new_data, NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT);
}
/************************************************************************/
@ -1674,7 +1674,7 @@ nm_config_set_values (NMConfig *self,
nm_log_dbg (LOGD_CORE, "don't persistate internal configuration (no file set, use --intern-config?)");
}
if (new_data)
_set_config_data (self, new_data, 0);
_set_config_data (self, new_data, NM_CONFIG_CHANGE_CAUSE_SET_VALUES);
g_key_file_unref (keyfile_new);
}
@ -1865,7 +1865,7 @@ _nm_config_state_set (NMConfig *self,
/*****************************************************************************/
void
nm_config_reload (NMConfig *self, int signal)
nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags)
{
NMConfigPrivate *priv;
GError *error = NULL;
@ -1877,11 +1877,16 @@ nm_config_reload (NMConfig *self, int signal)
gboolean intern_config_needs_rewrite;
g_return_if_fail (NM_IS_CONFIG (self));
g_return_if_fail ( reload_flags
&& !NM_FLAGS_ANY (reload_flags, ~NM_CONFIG_CHANGE_CAUSES)
&& !NM_FLAGS_ANY (reload_flags, NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT
| NM_CONFIG_CHANGE_CAUSE_SET_VALUES));
priv = NM_CONFIG_GET_PRIVATE (self);
if (signal != SIGHUP) {
_set_config_data (self, NULL, signal);
if (reload_flags != NM_CONFIG_CHANGE_CAUSE_SIGHUP) {
/* unless SIGHUP is specified, we don't reload the configuration from disc. */
_set_config_data (self, NULL, reload_flags);
return;
}
@ -1898,7 +1903,7 @@ nm_config_reload (NMConfig *self, int signal)
if (!keyfile) {
nm_log_err (LOGD_CORE, "Failed to reload the configuration: %s", error->message);
g_clear_error (&error);
_set_config_data (self, NULL, signal);
_set_config_data (self, NULL, reload_flags);
return;
}
@ -1920,13 +1925,17 @@ nm_config_reload (NMConfig *self, int signal)
if (keyfile_intern)
g_key_file_unref (keyfile_intern);
_set_config_data (self, new_data, signal);
_set_config_data (self, new_data, reload_flags);
}
NM_UTILS_FLAGS2STR_DEFINE (nm_config_change_flags_to_string, NMConfigChangeFlags,
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_SIGHUP, "SIGHUP"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_SIGUSR1, "SIGUSR1"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_SIGUSR2, "SIGUSR2"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_CAUSE_SIGHUP, "SIGHUP"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_CAUSE_SIGUSR1, "SIGUSR1"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_CAUSE_SIGUSR2, "SIGUSR2"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT, "NO_AUTO_DEFAULT"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_CAUSE_SET_VALUES, "SET_VALUES"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_CONFIG_FILES, "config-files"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_VALUES, "values"),
NM_UTILS_FLAGS2STR (NM_CONFIG_CHANGE_VALUES_USER, "values-user"),
@ -1939,27 +1948,19 @@ NM_UTILS_FLAGS2STR_DEFINE (nm_config_change_flags_to_string, NMConfigChangeFlags
);
static void
_set_config_data (NMConfig *self, NMConfigData *new_data, int signal)
_set_config_data (NMConfig *self, NMConfigData *new_data, NMConfigChangeFlags reload_flags)
{
NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (self);
NMConfigData *old_data = priv->config_data;
NMConfigChangeFlags changes, changes_diff;
gboolean had_new_data = !!new_data;
switch (signal) {
case SIGHUP:
changes = NM_CONFIG_CHANGE_SIGHUP;
break;
case SIGUSR1:
changes = NM_CONFIG_CHANGE_SIGUSR1;
break;
case SIGUSR2:
changes = NM_CONFIG_CHANGE_SIGUSR2;
break;
default:
changes = NM_CONFIG_CHANGE_NONE;
break;
}
nm_assert (reload_flags);
nm_assert (!NM_FLAGS_ANY (reload_flags, ~NM_CONFIG_CHANGE_CAUSES));
nm_assert ( NM_IN_SET (reload_flags, NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT, NM_CONFIG_CHANGE_CAUSE_SET_VALUES)
|| !NM_FLAGS_ANY (reload_flags, NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT | NM_CONFIG_CHANGE_CAUSE_SET_VALUES));
changes = reload_flags;
if (new_data) {
changes_diff = nm_config_data_diff (old_data, new_data);
@ -1969,8 +1970,11 @@ _set_config_data (NMConfig *self, NMConfigData *new_data, int signal)
changes |= changes_diff;
}
if (changes == NM_CONFIG_CHANGE_NONE)
if ( NM_IN_SET (reload_flags, NM_CONFIG_CHANGE_CAUSE_NO_AUTO_DEFAULT, NM_CONFIG_CHANGE_CAUSE_SET_VALUES)
&& !new_data) {
/* no relevant changes that should be propagated. Return silently. */
return;
}
if (new_data) {
nm_log_info (LOGD_CORE, "config: update %s (%s)", nm_config_data_get_config_description (new_data),

View file

@ -147,7 +147,7 @@ void nm_config_set_no_auto_default_for_device (NMConfig *config, NMDevice *devi
NMConfig *nm_config_new (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error);
NMConfig *nm_config_setup (const NMConfigCmdLineOptions *cli, char **atomic_section_prefixes, GError **error);
void nm_config_reload (NMConfig *config, int signal);
void nm_config_reload (NMConfig *config, NMConfigChangeFlags reload_flags);
const NMConfigState *nm_config_state_get (NMConfig *config);

View file

@ -942,8 +942,8 @@ config_changed_cb (NMConfig *config,
* On SIGHUP and SIGUSR1 try to re-connect to D-Bus. So in the unlikely
* event that the D-Bus conneciton is broken, that allows for recovery
* without need for restarting NetworkManager. */
if ( NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_SIGHUP)
|| NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_SIGUSR1)) {
if (NM_FLAGS_ANY (changes, NM_CONFIG_CHANGE_CAUSE_SIGHUP
| NM_CONFIG_CHANGE_CAUSE_SIGUSR1)) {
if (!SETTINGS_PLUGIN_IFCFG_GET_PRIVATE (self)->dbus.connection)
_dbus_setup (self);
}

View file

@ -343,7 +343,7 @@ test_config_no_auto_default (void)
g_assert (!nm_config_get_no_auto_default_for_device (config, dev3));
g_assert (nm_config_get_no_auto_default_for_device (config, dev4));
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_INFO, "*config: update * (no-auto-default)*");
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_INFO, "*config: update * (NO_AUTO_DEFAULT,no-auto-default)*");
nm_config_set_no_auto_default_for_device (config, dev3);
g_test_assert_expected_messages ();
@ -511,9 +511,9 @@ _set_values_config_changed_cb (NMConfig *config,
g_assert (config_changed_data);
g_assert (config_changed_data->changes == NM_CONFIG_CHANGE_NONE);
if (changes == NM_CONFIG_CHANGE_SIGHUP)
if (changes == NM_CONFIG_CHANGE_CAUSE_SIGHUP)
return;
changes &= ~NM_CONFIG_CHANGE_SIGHUP;
changes &= ~NM_CONFIG_CHANGE_CAUSE_SIGHUP;
config_changed_data->changes = changes;
@ -560,7 +560,7 @@ _set_values_user (NMConfig *config,
else
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_INFO, "*config: signal SIGHUP (no changes from disk)*");
nm_config_reload (config, SIGHUP);
nm_config_reload (config, NM_CONFIG_CHANGE_CAUSE_SIGHUP);
g_test_assert_expected_messages ();
@ -649,14 +649,14 @@ static void
_set_values_intern_internal_set (NMConfig *config, gboolean set_user, GKeyFile *keyfile, NMConfigChangeFlags *out_expected_changes)
{
g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"section1", "key", "internal-section");
*out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN;
*out_expected_changes = NM_CONFIG_CHANGE_CAUSE_SET_VALUES | NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN;
}
static void
_set_values_intern_internal_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data)
{
if (is_change_event)
g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN));
g_assert (changes == (NM_CONFIG_CHANGE_CAUSE_SET_VALUES | NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN));
assert_config_value (config_data, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"section1", "key", "internal-section");
}
@ -690,14 +690,14 @@ _set_values_intern_atomic_section_1_set (NMConfig *config, gboolean set_user, GK
g_key_file_set_string (keyfile, "atomic-prefix-1.section-a", "key3", "intern-value3");
g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key1", "intern-value1");
g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key3", "intern-value3");
*out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN;
*out_expected_changes = NM_CONFIG_CHANGE_CAUSE_SET_VALUES | NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN;
}
static void
_set_values_intern_atomic_section_1_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data)
{
if (is_change_event)
g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN));
g_assert (changes == (NM_CONFIG_CHANGE_CAUSE_SET_VALUES | NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN));
assert_config_value (config_data, "atomic-prefix-1.section-a", "key1", "intern-value1");
assert_config_value (config_data, "atomic-prefix-1.section-a", "key2", NULL);
assert_config_value (config_data, "atomic-prefix-1.section-a", "key3", "intern-value3");
@ -744,14 +744,14 @@ _set_values_intern_atomic_section_2_set (NMConfig *config, gboolean set_user, GK
g_key_file_set_string (keyfile, "non-atomic-prefix-1.section-a", "nap1-key3", "intern-value3");
g_key_file_set_string (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"with-whitespace", "key1", " b c\\, d ");
g_key_file_set_value (keyfile, NM_CONFIG_KEYFILE_GROUPPREFIX_INTERN"with-whitespace", "key2", " b c\\, d ");
*out_expected_changes = NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN;
*out_expected_changes = NM_CONFIG_CHANGE_CAUSE_SET_VALUES | NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN;
}
static void
_set_values_intern_atomic_section_2_check (NMConfig *config, NMConfigData *config_data, gboolean is_change_event, NMConfigChangeFlags changes, NMConfigData *old_data)
{
if (is_change_event)
g_assert (changes == (NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN));
g_assert (changes == (NM_CONFIG_CHANGE_CAUSE_SET_VALUES | NM_CONFIG_CHANGE_VALUES | NM_CONFIG_CHANGE_VALUES_INTERN));
g_assert (!nm_config_data_has_group (config_data, "atomic-prefix-1.section-a"));
assert_config_value (config_data, "atomic-prefix-1.section-b", "key1", "user-value1");
assert_config_value (config_data, "non-atomic-prefix-1.section-a", "nap1-key1", NULL);
@ -862,17 +862,17 @@ test_config_signal (void)
G_CALLBACK (_test_signal_config_changed_cb),
&expected);
expected = NM_CONFIG_CHANGE_SIGUSR1;
expected = NM_CONFIG_CHANGE_CAUSE_SIGUSR1;
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_INFO, "*config: signal SIGUSR1");
nm_config_reload (config, SIGUSR1);
nm_config_reload (config, expected);
expected = NM_CONFIG_CHANGE_SIGUSR2;
expected = NM_CONFIG_CHANGE_CAUSE_SIGUSR2;
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_INFO, "*config: signal SIGUSR2");
nm_config_reload (config, SIGUSR2);
nm_config_reload (config, expected);
expected = NM_CONFIG_CHANGE_SIGHUP;
expected = NM_CONFIG_CHANGE_CAUSE_SIGHUP;
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_INFO, "*config: signal SIGHUP (no changes from disk)*");
nm_config_reload (config, SIGHUP);
nm_config_reload (config, expected);
/* test with subscribing two signals...
@ -883,9 +883,9 @@ test_config_signal (void)
NM_CONFIG_SIGNAL_CONFIG_CHANGED,
G_CALLBACK (_test_signal_config_changed_cb2),
&expected);
expected = NM_CONFIG_CHANGE_SIGUSR2;
expected = NM_CONFIG_CHANGE_CAUSE_SIGUSR2;
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_INFO, "*config: signal SIGUSR2");
nm_config_reload (config, SIGUSR2);
nm_config_reload (config, NM_CONFIG_CHANGE_CAUSE_SIGUSR2);
g_signal_handlers_disconnect_by_func (config, _test_signal_config_changed_cb2, &expected);