From c210134bd58ea40ee3458aff27ab41958e1f48b4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 22 Feb 2016 15:53:06 +0100 Subject: [PATCH] device: revert overruling NM_UNMANAGED_USER_SETTINGS decision Since commit 87a3df2e572ed47b5f76f6d1cad63ce622296e21, the unmanaged flag NM_UNMANAGED_USER_SETTINGS could be overwritten via an explict user decision (NM_UNMANAGED_USER_EXPLICIT). It makes sense to allow user configuration from file to be changable by an explict user action via D-Bus at runtime. However, it also changes behavior for devices that are currently explicitly managed. Previously, a reload of the NM_UNMANAGED_USER_SETTINGS would immediately unmanaged the device: - for keyfile: send SIGHUP to reload NetworkManager.conf - for ifcfg-rh: `nmcli connection [re]load` So this change in behavior could negatively affect users who rely on being able to configure "NM_CONTROLLED=no" and expect to unmanaged the device immediately. Thus revert the change. Note that NM_UNMANAGED_USER_SETTINGS is anyway ugly and should be deprecated: - for keyfile, why having the option "keyfile.unmanaged-devices" instead of a generic options? - for ifcfg-rh, why put per-device configuration in a per-connection file? The preferred way is to configure NM_UNMANAGED_USER_UDEV via "ENV{NM_UNMANAGED}". Maybe we should also add a new configuration scheme via NetworkManager.conf. https://bugzilla.gnome.org/show_bug.cgi?id=762331 --- src/devices/nm-device.c | 14 +------------- src/devices/nm-device.h | 6 +++--- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 39c4069bb6..bbad10946b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -9059,23 +9059,12 @@ _get_managed_by_flags(NMUnmanagedFlags flags, NMUnmanagedFlags mask, gboolean fo mask &= ~NM_UNMANAGED_USER_SETTINGS; } - if (NM_FLAGS_ANY (mask, NM_UNMANAGED_USER_UDEV | NM_UNMANAGED_USER_SETTINGS)) { + if (NM_FLAGS_ANY (mask, NM_UNMANAGED_USER_UDEV)) { /* configuration from udev or nm-config overwrites the by-default flag * which is based on the device type. */ flags &= ~NM_UNMANAGED_BY_DEFAULT; } - if (NM_FLAGS_HAS (mask, NM_UNMANAGED_USER_SETTINGS)) { - /* configuration from configuration overwrites the setting - * originating from udev. - * - * Actually, this check has no effect, because at this point, - * the device also is NM_UNMANAGED_USER_SETTINGS. Thus clearing - * NM_UNMANAGED_USER_UDEV doesn't change the outcome. - * Just be explicit about this. */ - flags &= ~NM_UNMANAGED_USER_UDEV; - } - if ( NM_FLAGS_HAS (mask, NM_UNMANAGED_IS_SLAVE) && !NM_FLAGS_HAS (flags, NM_UNMANAGED_IS_SLAVE)) { /* for an enslaved device, by-default doesn't matter */ @@ -9087,7 +9076,6 @@ _get_managed_by_flags(NMUnmanagedFlags flags, NMUnmanagedFlags mask, gboolean fo * are ignored. */ flags &= ~( NM_UNMANAGED_BY_DEFAULT - | NM_UNMANAGED_USER_SETTINGS | NM_UNMANAGED_USER_UDEV | NM_UNMANAGED_EXTERNAL_DOWN); } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index e9f7b909b2..9b9edda494 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -430,11 +430,11 @@ RfKillType nm_device_get_rfkill_type (NMDevice *device); * yet initialized. Unrealized device are also unmanaged for this reason. * @NM_UNMANAGED_USER_EXPLICIT: %TRUE when unmanaged by explicit user decision * (e.g. via a D-Bus command) - * @NM_UNMANAGED_BY_DEFAULT: %TRUE for certain device types where we unmanage - * them by default * @NM_UNMANAGED_USER_SETTINGS: %TRUE when unmanaged by user decision via * the settings plugin (for example keyfile.unmanaged-devices or ifcfg-rh's * NM_CONTROLLED=no) + * @NM_UNMANAGED_BY_DEFAULT: %TRUE for certain device types where we unmanage + * them by default * @NM_UNMANAGED_USER_UDEV: %TRUE when unmanaged by user decision (via UDev rule) * @NM_UNMANAGED_EXTERNAL_DOWN: %TRUE when unmanaged because !IFF_UP and not created by NM * @NM_UNMANAGED_IS_SLAVE: indicates that the device is enslaved. Note that @@ -452,11 +452,11 @@ typedef enum { /*< skip >*/ NM_UNMANAGED_LOOPBACK = (1LL << 3), NM_UNMANAGED_PLATFORM_INIT = (1LL << 4), NM_UNMANAGED_USER_EXPLICIT = (1LL << 5), + NM_UNMANAGED_USER_SETTINGS = (1LL << 6), /* These flags can be non-effective and be overwritten * by other flags. */ NM_UNMANAGED_BY_DEFAULT = (1LL << 8), - NM_UNMANAGED_USER_SETTINGS = (1LL << 9), NM_UNMANAGED_USER_UDEV = (1LL << 10), NM_UNMANAGED_EXTERNAL_DOWN = (1LL << 11), NM_UNMANAGED_IS_SLAVE = (1LL << 12),