Bug 1422610 - NM changes hostname to localhost.localdomain even though no devices are managed by it
Summary: NM changes hostname to localhost.localdomain even though no devices are manag...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: NetworkManager
Version: 7.3
Hardware: x86_64
OS: Linux
urgent
urgent
Target Milestone: rc
: ---
Assignee: Francesco Giudici
QA Contact: Desktop QE
URL:
Whiteboard:
Depends On:
Blocks: 132679 1405275 1419906
TreeView+ depends on / blocked
 
Reported: 2017-02-15 16:48 UTC by Edward Haas
Modified: 2017-08-01 09:22 UTC (History)
17 users (show)

Fixed In Version: NetworkManager-1.8.0-0.4.rc1.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of: 1419906
Environment:
Last Closed: 2017-08-01 09:22:07 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2017:2299 normal SHIPPED_LIVE Moderate: NetworkManager and libnl3 security, bug fix and enhancement update 2017-08-01 12:40:28 UTC

Description Edward Haas 2017-02-15 16:48:40 UTC
+++ This bug was initially created as a clone of Bug #1419906 +++

When all devices are acquired from NM (and unmanaged), the initscripts are used to start dhclient and get a dynamic IP.
Unfortunately, NM is overwriting the hostname fetched from the dhcp response by removing the Transient hostname.

It can be recreated by:
- Unmanage all devices from NM.
- Run "systemctl restart network" and check using hostnamectl that the transient is there.
- After a few seconds the transient hostname is removed by NM.


--- Additional comment from Michael Burman on 2017-02-15 07:22:12 EST ---

The hostname is changed to localhost much earlier, no need to wait for HE setup to end.
Once vdsm configuring the management bridge it's changing to localhost.
If restarting network service, the hostname is back to correct hostname.

[root@orchid-vds1 ~]# nmcli c s
NAME    UUID                                  TYPE            DEVICE 
enp4s0  d3cf5b62-2164-4100-9008-f227ab96f978  802-3-ethernet  enp4s0 
enp6s0  ded8ca78-089d-4433-b5a3-bc6eb54b9d1d  802-3-ethernet  --     
ens1f0  70b219fa-91cb-4281-a2e1-f63321a8c046  802-3-ethernet  --     
ens1f1  70935ce0-18db-4557-b30a-ae5403fd4b5f  802-3-ethernet  --     
[root@orchid-vds1 ~]# nmcli c s id enp4s0
connection.id:                          enp4s0
connection.uuid:                        d3cf5b62-2164-4100-9008-f227ab96f978
connection.stable-id:                   --
connection.interface-name:              enp4s0
connection.type:                        802-3-ethernet
connection.autoconnect:                 yes
connection.autoconnect-priority:        0
connection.timestamp:                   1487158384
connection.read-only:                   no
connection.permissions:                 
connection.zone:                        --
connection.master:                      --
connection.slave-type:                  --
connection.autoconnect-slaves:          -1 (default)
connection.secondaries:                 
connection.gateway-ping-timeout:        0
connection.metered:                     unknown
connection.lldp:                        -1 (default)
802-3-ethernet.port:                    --
802-3-ethernet.speed:                   0
802-3-ethernet.duplex:                  --
802-3-ethernet.auto-negotiate:          yes
802-3-ethernet.mac-address:             --
802-3-ethernet.cloned-mac-address:      --
802-3-ethernet.generate-mac-address-mask:--
802-3-ethernet.mac-address-blacklist:   
802-3-ethernet.mtu:                     auto
802-3-ethernet.s390-subchannels:        
802-3-ethernet.s390-nettype:            --
802-3-ethernet.s390-options:            
802-3-ethernet.wake-on-lan:             1 (default)
802-3-ethernet.wake-on-lan-password:    --
ipv4.method:                            auto
ipv4.dns:                               
ipv4.dns-search:                        
ipv4.dns-options:                       (default)
ipv4.dns-priority:                      0
ipv4.addresses:                         
ipv4.gateway:                           --
ipv4.routes:                            
ipv4.route-metric:                      -1
ipv4.ignore-auto-routes:                no
ipv4.ignore-auto-dns:                   no
ipv4.dhcp-client-id:                    --
ipv4.dhcp-timeout:                      0
ipv4.dhcp-send-hostname:                yes
ipv4.dhcp-hostname:                     --
ipv4.dhcp-fqdn:                         --
ipv4.never-default:                     no
ipv4.may-fail:                          yes
ipv4.dad-timeout:                       -1 (default)
ipv6.method:                            auto
ipv6.dns:                               
ipv6.dns-search:                        
ipv6.dns-options:                       (default)
ipv6.dns-priority:                      0
ipv6.addresses:                         
ipv6.gateway:                           --
ipv6.routes:                            
ipv6.route-metric:                      -1
ipv6.ignore-auto-routes:                no
ipv6.ignore-auto-dns:                   no
ipv6.never-default:                     no
ipv6.may-fail:                          yes
ipv6.ip6-privacy:                       -1 (unknown)
ipv6.addr-gen-mode:                     eui64
ipv6.dhcp-send-hostname:                yes
ipv6.dhcp-hostname:                     --
ipv6.token:                             --
GENERAL.NAME:                           enp4s0
GENERAL.UUID:                           d3cf5b62-2164-4100-9008-f227ab96f978
GENERAL.DEVICES:                        enp4s0
GENERAL.STATE:                          activated
GENERAL.DEFAULT:                        yes
GENERAL.DEFAULT6:                       yes
GENERAL.VPN:                            no
GENERAL.ZONE:                           --
GENERAL.DBUS-PATH:                      /org/freedesktop/NetworkManager/ActiveConnection/1
GENERAL.CON-PATH:                       /org/freedesktop/NetworkManager/Settings/1
GENERAL.SPEC-OBJECT:                    /
GENERAL.MASTER-PATH:                    --
IP4.ADDRESS[1]:                         10.35.128.22/24
IP4.GATEWAY:                            10.35.128.254
IP4.ROUTE[1]:                           dst = 10.35.28.1/32, nh = 10.35.128.254, mt = 100
IP4.DNS[1]:                             10.35.64.1
IP4.DOMAIN[1]:                          qa.lab.tlv.redhat.com
IP4.DOMAIN[2]:                          lab.tlv.redhat.com
IP4.DOMAIN[3]:                          tlv.redhat.com
IP4.DOMAIN[4]:                          redhat.com
DHCP4.OPTION[1]:                        requested_classless_static_routes = 1
DHCP4.OPTION[2]:                        requested_rfc3442_classless_static_routes = 1
DHCP4.OPTION[3]:                        subnet_mask = 255.255.255.0
DHCP4.OPTION[4]:                        requested_subnet_mask = 1
DHCP4.OPTION[5]:                        domain_name_servers = 10.35.64.1
DHCP4.OPTION[6]:                        ip_address = 10.35.128.22
DHCP4.OPTION[7]:                        filename = pxelinux.0
DHCP4.OPTION[8]:                        requested_static_routes = 1
DHCP4.OPTION[9]:                        dhcp_server_identifier = 10.35.28.1
DHCP4.OPTION[10]:                       requested_nis_servers = 1
DHCP4.OPTION[11]:                       requested_time_offset = 1
DHCP4.OPTION[12]:                       time_offset = 1
DHCP4.OPTION[13]:                       broadcast_address = 10.35.128.255
DHCP4.OPTION[14]:                       requested_interface_mtu = 1
DHCP4.OPTION[15]:                       requested_domain_name_servers = 1
DHCP4.OPTION[16]:                       dhcp_message_type = 5
DHCP4.OPTION[17]:                       requested_broadcast_address = 1
DHCP4.OPTION[18]:                       routers = 10.35.128.254
DHCP4.OPTION[19]:                       requested_domain_name = 1
DHCP4.OPTION[20]:                       domain_name = qa.lab.tlv.redhat.com lab.tlv.redhat.com tlv.redhat.com redhat.com
DHCP4.OPTION[21]:                       requested_routers = 1
DHCP4.OPTION[22]:                       expiry = 1487194733
DHCP4.OPTION[23]:                       requested_wpad = 1
DHCP4.OPTION[24]:                       requested_nis_domain = 1
DHCP4.OPTION[25]:                       requested_ms_classless_static_routes = 1
DHCP4.OPTION[26]:                       network_number = 10.35.128.0
DHCP4.OPTION[27]:                       requested_domain_search = 1
DHCP4.OPTION[28]:                       next_server = 10.35.70.30
DHCP4.OPTION[29]:                       requested_ntp_servers = 1
DHCP4.OPTION[30]:                       ntp_servers = 10.35.28.1 10.35.255.6
DHCP4.OPTION[31]:                       dhcp_lease_time = 43200
DHCP4.OPTION[32]:                       requested_host_name = 1
IP6.ADDRESS[1]:                         2620:52:0:2380:214:5eff:fe17:d5b0/64
IP6.ADDRESS[2]:                         fe80::214:5eff:fe17:d5b0/64
IP6.GATEWAY:                            fe80:52:0:2380::fe
IP6.ROUTE[1]:                           dst = 2620:52:0:2380::/64, nh = ::, mt = 100
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# 
[root@orchid-vds1 ~]# hostnamectl 
   Static hostname: localhost.localdomain
Transient hostname: orchid-vds1.qa.lab.tlv.redhat.com
         Icon name: computer-server
           Chassis: server
        Machine ID: 9a1ff3992c28470d992270e45ccda5ca
           Boot ID: 6e5149f0a522471cbe793ede1bd83f63
  Operating System: Red Hat Virtualization Host 4.1 (el7.3)
       CPE OS Name: cpe:/o:redhat:enterprise_linux:7.3:GA:hypervisor
            Kernel: Linux 3.10.0-514.6.1.el7.x86_64
      Architecture: x86-64

Edy, if you were correct, then restart to network wouldn't bring the hostname back.

--- Additional comment from Edward Haas on 2017-02-15 11:29:01 EST ---

A few seconds after "systemctl restart network", it looks like NM overrides the hostname.

From the messages log:

Feb 15 17:58:01 localhost systemd: Started LSB: Bring up/down networking.
Feb 15 17:58:15 localhost dbus-daemon: dbus[862]: [system] Activating via systemd: service name='org.freedesktop.hostname1' unit='dbus-org.freedesktop.hostname1.service'
Feb 15 17:58:15 localhost dbus[862]: [system] Activating via systemd: service name='org.freedesktop.hostname1' unit='dbus-org.freedesktop.hostname1.service'
Feb 15 17:58:15 localhost systemd: Starting Hostname Service...
Feb 15 17:58:15 localhost dbus-daemon: dbus[862]: [system] Successfully activated service 'org.freedesktop.hostname1'
Feb 15 17:58:15 localhost dbus[862]: [system] Successfully activated service 'org.freedesktop.hostname1'
Feb 15 17:58:15 localhost systemd: Started Hostname Service.
Feb 15 17:58:16 localhost NetworkManager[953]: <info>  [1487174296.4969] policy: setting system hostname to 'localhost.localdomain' (no default device)
Feb 15 17:58:16 localhost systemd-hostnamed: Changed host name to 'localhost.localdomain'

Comment 3 Francesco Giudici 2017-02-16 18:34:16 UTC
Hi Edward,
   can you please add some details to the scenario to reproduce the bug?
I tried to create a new connection in RHEL-7.3, restart network service but I was not able to see the issue.

Anyway, I was able to find an issue with this scenario:
1) create ifcfg file with IPv4DHCP with NM_CONTROLLED=NO
2) configure dhcp server to provide hostname option
3) reboot

Then I found the dhclient getting the lease, transient hostname was set accordingly and NM immediately changed it back to localhost.localdomain.
I was able to trigger this only at boot anyway.

If this is not the scenario you are addressing, please provide more information on how to reproduce it.
Some more extensive log could be of help too.

Thanks

Francesco

Comment 4 Francesco Giudici 2017-02-17 18:40:58 UTC
The issue is probably due to the fact that NetworkManager ignores external hostname changes after startup: it never checks if someone else has changed the hostname.
When sooner or later something triggers NetworkManger to check / update the hostname (e.g., a connection is teared down or devices are set to unmanaged) it will just update the hostname as best as it can, leveraging its internal information only.
In case all devices are unmanaged, it will try to restore the hostname that was in place when it started: if was unset, it will set just a default "localhost".

So, proposed patch will let NetworkManager check if someone else has changed the hostname before trying to update it.
It will still overwrite it but only if the hostname value comes from a DHCP hostname option in an active NM connection. Otherwise it will just skip the update.
In both the cases, it will record the hostname as fallback for future updates.

Please, review upstream branch:
keep_externally_set_hostname-rh1422610

Comment 6 Francesco Giudici 2017-02-19 21:25:50 UTC
Well, asking review mainly to NetworkManager developers.
Any other review anyway much appreciated!
Edward, if you need a build to test just ask. I'm anyway quite confident this should fix the issue.

Thanks!

(Moving bug to POST)

Comment 7 Francesco Giudici 2017-02-20 18:40:54 UTC
Got review from Beniamino (thanks!), repushed upstream on branch:

fg/keep_externally_set_hostname-rh1422610

Comment 8 Nikolai Sednev 2017-02-21 14:53:03 UTC
Isn't this bug is a duplicate of 1419906?

Comment 9 Dan Kenigsberg 2017-02-21 15:08:32 UTC
(In reply to Nikolai Sednev from comment #8)
> Isn't this bug is a duplicate of 1419906?

This bug is short and clean and is opened on the correct component. Your bug 1419906 should stay as it is, hopefully becoming a TestOnly bug.

Comment 10 Yaniv Lavi 2017-03-01 11:08:05 UTC
What is the status of this bug?

Comment 11 Francesco Giudici 2017-03-02 07:39:28 UTC
(In reply to Yaniv Dary from comment #10)
> What is the status of this bug?

Is in progress.
The provided patch aims to detect external changes to the hostname by NetworkManager. The problem is that there is a race between NM reading the hostname and then setting it and an external set. So there is no guarantee with this approach that NetworkManager will not override the hostname.
While hostname management needs some deep rework (will be addressed in bug #1405275) I would like to bring here some solution that can be easily backported to 7.3.
Two solutions could be viable:
1) Add a global option to disable (transient) hostname management in NM.
2) Disable hostname changes when there are no active connections.

In the meanwhile the branch fg/keep_externally_set_hostname-rh1422610 got updated: it fixes a race in the get/set of the hostname inside NetworkManager itself on the current branch (#1405275 rework will solve this by design).
The updated branch would probably work in many cases but still cannot guarantee that hostname is not changed by NetworkManager when externally set, due to the race (with the EXTERNAL setter) I mentioned above.
WiP is to add to the branch option 1) OR 2).
I'm currently looking at option 2 but is a bit more complex than option 1.
Do you think option 1) could be viable too for this use case?

Comment 12 Yaniv Lavi 2017-03-02 09:58:32 UTC
The main issue is that NM is trying to manage a device that is not managed by NM. Can't we fix that? Have NM not control things that are managed by another service?

Comment 13 Francesco Giudici 2017-03-02 11:17:12 UTC
(In reply to Yaniv Dary from comment #12)
> The main issue is that NM is trying to manage a device that is not managed
> by NM.

NetworkManager manages the hostname and on some triggering events it will update it at the best of its knowledge. I think the issue was that the hostname was overwritten.
If there is more, and NetworkManager manages devices it was told not to manage, please provide some logs... this was not clear at all till now.

Comment 14 Edward Haas 2017-03-02 11:43:29 UTC
(In reply to Francesco Giudici from comment #13)
> (In reply to Yaniv Dary from comment #12)
> > The main issue is that NM is trying to manage a device that is not managed
> > by NM.
> 
> NetworkManager manages the hostname and on some triggering events it will
> update it at the best of its knowledge. I think the issue was that the
> hostname was overwritten.
> If there is more, and NetworkManager manages devices it was told not to
> manage, please provide some logs... this was not clear at all till now.

I think that Yaniv Dary was actually asking for something similar to option 2, when there are no active connections.
Yaniv, the hostname is a host level configuration and currently NM tries to control it, regardless of connection profiles (which are at the device level).

The thing is, that it may not cover all the scenarios, as there may be active connections that have static settings (no DHCP), which should not interfere with the hostname. VDSM specifically, acquires from NM only the devices it wants to managed, not all.

Comment 15 Francesco Giudici 2017-03-02 12:18:26 UTC
(In reply to Edward Haas from comment #14)
> 
> The thing is, that it may not cover all the scenarios, as there may be
> active connections that have static settings (no DHCP), which should not
> interfere with the hostname. VDSM specifically, acquires from NM only the
> devices it wants to managed, not all.

Thanks Edward, I see. So, here the ideal solution seems to me to have NetworkManager updating the hostname only if it has an hostname option coming from DHCP.
This could be done making NM hostname management globally configurable.
I will work on this part first.

Comment 16 Yaniv Lavi 2017-03-08 09:26:45 UTC
What is the status of this bug?

Comment 17 Francesco Giudici 2017-03-08 17:26:45 UTC
(In reply to Yaniv Dary from comment #16)
> What is the status of this bug?

Hi Yaniv,
just updated the branch fg/keep_externally_set_hostname-rh1422610.
Taken some time to do some basic tests on it.

Here what has been put into:
1) detection of external hostname change
this will prevent NM to overwrite the hostname with locahost if the hostname was set outside NM. This could not be granted anyway if the external set and the hostname update in NM happens at almost the same time (get and set of the hostname from NM cannot be atomic).
2) don't trigger hostname update when events are detected on unmanged devices
3) add a new global configure option that allows to change the hostname update behavior of NM.
Three options are available:
 - none (NM will not update the hostname)
 - dhcp (NM will update the hostname only from options in DHCP)
 - full (full update policy, as before)

Please, review the branch

Comment 19 Thomas Haller 2017-03-15 14:43:16 UTC
> policy: remove useless checks and redundant var when setting the hostname

I don't understand this. Could you explain better in the commit message?

> policy: detect if the hostname was changed outside NetworkManager
    
+         /* Cancel ongoing lookup first*/
+         priv->lookup_cancellable = g_cancellable_new ();

the comment seems wrong. Maybe also not move creation of lookup_cancellable before /* Check if the hostname was externally set */ ?

> config: add new hostname_mode configuration

nm_config_data_get_hostname_mode() is only used once, inside nm_policy_init(). There is no need to add an API for that and cache the value for the entire duration. Just use nm_config_data_get_value() in nm_policy_init().

But let's use a define 
#define NM_CONFIG_KEYFILE_KEY_MAIN_HOSTNAME "hostname"
in nm-config.h

> policy: add support to configurable hostname mode

I am not convinced about the option name

  [main]
  hostname=

it's not really the "hostname" but the "hostname-mode". The rest of the code also calls it "hostname-mode".


also:
+    if nm_streq0 (hostname_mode, "none")
+         priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_UNMANAGED;
+    else if nm_streq0 (hostname_mode, "dhcp")
+         priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_DHCP;
+    else /* default - full mode */
+         priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_FULL;

let's not do "none" vs. UNMANAGED? The names of NM_POLICY_HOSTNAME_MODE_* should correspond to the configuration options.


+    hostname_mode = nm_config_data_get_hostname_mode (nm_config_get_data_orig (config));

let's use NM_CONFIG_GET_DATA_ORIG.


> policy: add some verbose logging for tracking hostname management

all logging related to the same topic should have a common prefix (preferably, unique, so you can grep for it).
Maybe
  "set-hostname:"

Comment 20 Francesco Giudici 2017-03-16 10:24:15 UTC
(In reply to Thomas Haller from comment #19)
> > policy: remove useless checks and redundant var when setting the hostname
> 
> I don't understand this. Could you explain better in the commit message?

Changed with:
"policy: remove redundant check in _set_hostname"

What I mean is that there are 2 checks and one "else" branch:

1)  if (   priv->orig_hostname
        && (priv->hostname_changed == FALSE)
        && g_strcmp0 (priv->orig_hostname, new_hostname) == 0) {
             /* HERE NOTHING IS DONE */
2)     } else if (g_strcmp0 (priv->cur_hostname, new_hostname) == 0) {
            /* HERE NOTHING IS DONE */
3)     } else {
            /* HERE SOME "ACTION" TAKES PLACE */


As condition 2 is a subset of condition 1, when 1 is true, 2 will always be true too. And as the action taken for condition 1) and condition 2) is the same (NOTHING), condition 1 can be removed without functional change. This means also that "hostname_changed" var is now useless.

Then, condition 2 can be reverted doing there the action of the "else" branch (3). So the whole "if/else if/else"  can be substituted with a single "if" construct.
+       /* Update the DNS only if the hostname isn't actually
+        * going to change.
+        */
+       if (!nm_streq0 (priv->cur_hostname, new_hostname)) {

I don't want to be overkill in the commit msg... but I can put something like this ^^^ in if you think would be better.


> 
> > policy: detect if the hostname was changed outside NetworkManager
>     
> +         /* Cancel ongoing lookup first*/
> +         priv->lookup_cancellable = g_cancellable_new ();
> 
> the comment seems wrong. Maybe also not move creation of lookup_cancellable
> before /* Check if the hostname was externally set */ ?

Sure, those are both wrong. Removing the comment and moving the lookup_cancellable creation back in the right place.

> 
> > config: add new hostname_mode configuration
> 
> nm_config_data_get_hostname_mode() is only used once, inside
> nm_policy_init(). There is no need to add an API for that and cache the
> value for the entire duration. Just use nm_config_data_get_value() in
> nm_policy_init().
> 
> But let's use a define 
> #define NM_CONFIG_KEYFILE_KEY_MAIN_HOSTNAME "hostname"
> in nm-config.h

Done, thanks

> 
> > policy: add support to configurable hostname mode
> 
> I am not convinced about the option name
> 
>   [main]
>   hostname=
> 
> it's not really the "hostname" but the "hostname-mode". The rest of the code
> also calls it "hostname-mode".

Yeah, seems better, changed
> 
> 
> also:
> +    if nm_streq0 (hostname_mode, "none")
> +         priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_UNMANAGED;
> +    else if nm_streq0 (hostname_mode, "dhcp")
> +         priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_DHCP;
> +    else /* default - full mode */
> +         priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_FULL;
> 
> let's not do "none" vs. UNMANAGED? The names of NM_POLICY_HOSTNAME_MODE_*
> should correspond to the configuration options.

Changed NM_POLICY_HOSTNAME_MODE_UNMANAGED --> NM_POLICY_HOSTNAME_MODE_NONE
("none" is coherent with "rc-managed" options)

> 
> 
> +    hostname_mode = nm_config_data_get_hostname_mode
> (nm_config_get_data_orig (config));
> 
> let's use NM_CONFIG_GET_DATA_ORIG.

Done

> 
> 
> > policy: add some verbose logging for tracking hostname management
> 
> all logging related to the same topic should have a common prefix
> (preferably, unique, so you can grep for it).
> Maybe
>   "set-hostname:"

Done
Thanks.

Updated branch fg/keep_externally_set_hostname-rh1422610

Comment 21 Beniamino Galvani 2017-03-23 10:11:02 UTC
> policy: remove redundant check in _set_hostname

+       /* Update the DNS only if the hostname isn't actually
+        * going to change.


s/isn't/is/ ?

Also, it seems the check was introduced in e04cbae to avoid spurious
rewrites of resolv.conf. At start priv->cur_hostname is NULL, and we
don't want to notify the DNS manager if @new_hostname is equal to
priv->orig_hostname, right?


> policy: detect if the hostname was changed outside NetworkManager

	/* Ask NMSettings to update the transient hostname using its
         * systemd-hostnamed proxy */
	nm_settings_set_transient_hostname (priv->settings,
                                            name,
                                            settings_set_hostname_cb,
-                                           NULL);
+                                           self);

I think you need to g_object_ref(self) to avoid that @self gets
disposed before the callback is run.


> policy: add support to configurable hostname mode

+       if nm_streq0 (hostname_mode, "none")
+               priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_NONE;
+       else if nm_streq0 (hostname_mode, "dhcp")
+               priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_DHCP;
+       else /* default - full mode */
+               priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_FULL;

Add parenthesis around conditions after if.


> policy: allow reset of dhcp hostname in "dhcp" hostname mode config

Can you add a comment or commit message (or both) to explain why this
is needed?

Comment 22 Francesco Giudici 2017-03-23 16:37:19 UTC
(In reply to Beniamino Galvani from comment #21)
> > policy: remove redundant check in _set_hostname
> 
> +       /* Update the DNS only if the hostname isn't actually
> +        * going to change.
> 
> 
> s/isn't/is/ ?

ops.. fixed, thanks

> 
> Also, it seems the check was introduced in e04cbae to avoid spurious
> rewrites of resolv.conf. At start priv->cur_hostname is NULL, and we
> don't want to notify the DNS manager if @new_hostname is equal to
> priv->orig_hostname, right?

Yeah... a little bit convoluted but the DNS manager will skip the update if it gets an hostname it is already aware of. So, also this case should be safe.

> 
> 
> > policy: detect if the hostname was changed outside NetworkManager
> 
> 	/* Ask NMSettings to update the transient hostname using its
>          * systemd-hostnamed proxy */
> 	nm_settings_set_transient_hostname (priv->settings,
>                                             name,
>                                             settings_set_hostname_cb,
> -                                           NULL);
> +                                           self);
> 
> I think you need to g_object_ref(self) to avoid that @self gets
> disposed before the callback is run.

Fixed, thanks

> 
> 
> > policy: add support to configurable hostname mode
> 
> +       if nm_streq0 (hostname_mode, "none")
> +               priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_NONE;
> +       else if nm_streq0 (hostname_mode, "dhcp")
> +               priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_DHCP;
> +       else /* default - full mode */
> +               priv->hostname_mode = NM_POLICY_HOSTNAME_MODE_FULL;
> 
> Add parenthesis around conditions after if.

Fixed

> 
> 
> > policy: allow reset of dhcp hostname in "dhcp" hostname mode config
> 
> Can you add a comment or commit message (or both) to explain why this
> is needed?

Sure, done.

Updated branch fg/keep_externally_set_hostname-rh1422610

Comment 23 Francesco Giudici 2017-03-24 16:14:06 UTC
Merged upstream: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=017b7c2bb6545f48bfc9c0357769801a488a619a

Note that the configuration key name as changed to "hostname-mode".

Comment 24 Yaniv Lavi 2017-04-05 07:56:12 UTC
We do not need the backport, please remove the z-stream flag unless you need it for anything else.

Comment 25 Francesco Giudici 2017-04-05 08:08:23 UTC
(In reply to Yaniv Dary from comment #24)
> We do not need the backport, please remove the z-stream flag unless you need
> it for anything else.
I don't need it too, thanks (and I cannot remove it).

Comment 27 errata-xmlrpc 2017-08-01 09:22:07 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2017:2299


Note You need to log in before you can comment on or make changes to this bug.