[master] Networking related patches

[Vratislav Podzimek]

PATCH 1/2 is useful for the Anaconda installer and the Initial Setup as the
hostname of the runtime system is updated to a value set by user.

PATCH 2/2 fixes a potential issue that was not hit yet, because we are writing
out ifcfg files only in some special cases (otherwise we copy those from the
runtime system).

Vratislav Podzimek (2):
  Set hostname when leaving network spokes
  Don't use temporary file and move when writing out an ifcfg file

pyanaconda/simpleconfig.py          | 7 ++++---
 pyanaconda/ui/gui/spokes/network.py | 8 ++++++++
 2 files changed, 12 insertions(+), 3 deletions(-)

[PATCH 1/2] Set hostname when leaving network spokes

[Vratislav Podzimek]

Related: rhbz#972362

Signed-off-by: Vratislav Podzimek 
---
 pyanaconda/ui/gui/spokes/network.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
index 43d12e1..2e80a3c 100644
--- a/pyanaconda/ui/gui/spokes/network.py
+++ b/pyanaconda/ui/gui/spokes/network.py
@@ -1275,6 +1275,10 @@ class NetworkSpoke(FirstbootSpokeMixIn, NormalSpoke):
         log.debug("network: apply ksdata %s", self.data.network)
         self.network_control_box.kill_nmce(msg="leaving network spoke")

+    def execute(self):
+        # update system's hostname
+        network.set_hostname(self.data.network.hostname)
+
     @property
     def completed(self):
         # TODO: check also if source requires updates when implemented
@@ -1436,6 +1440,10 @@ class NetworkStandaloneSpoke(StandaloneSpoke):

self.network_control_box.kill_nmce(msg="leaving standalone network spoke")

+    def execute(self):
+        # update system's hostname
+        network.set_hostname(self.data.network.hostname)
+
     @property
     def completed(self):
         return (not can_touch_runtime_system("require network connection")

[PATCH 1/2] Set hostname when leaving network spokes

[Brian C. Lane]

On Tue, Sep 10, 2013 at 11:56:55AM +0200, Vratislav Podzimek wrote:
> Related: rhbz#972362
> Signed-off-by: Vratislav Podzimek
>
> pyanaconda/ui/gui/spokes/network.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
> index 43d12e1..2e80a3c 100644
> --- a/pyanaconda/ui/gui/spokes/network.py
> +++ b/pyanaconda/ui/gui/spokes/network.py
> @@ -1275,6 +1275,10 @@ class NetworkSpoke(FirstbootSpokeMixIn, NormalSpoke):
>          log.debug("network: apply ksdata %s", self.data.network)
>          self.network_control_box.kill_nmce(msg="leaving network spoke")
> def execute(self):
>    # update system's hostname
>    network.set_hostname(self.data.network.hostname)
> @property
>    def completed(self):
>        # TODO: check also if source requires updates when implemented
>
> @@ -1436,6 +1440,10 @@ class NetworkStandaloneSpoke(StandaloneSpoke):
>      self.network_control_box.kill_nmce(msg="leaving standalone network spoke")
>
> def execute(self):
>    # update system's hostname
>    network.set_hostname(self.data.network.hostname)
> @property
>    def completed(self):
>        return (not can_touch_runtime_system("require network connection")
>
> -- 
> 1.7.11.7

I noticed that set_hostname is only blocking hostname setting on image
installs. It also needs to make sure it doesn't set it for dirinstall
since that could also be on a host not related to the target.

[PATCH 1/2] Set hostname when leaving network spokes

[Vratislav Podzimek]

On Tue, 2013-09-10 at 11:39 -0700, Brian C. Lane wrote:
> On Tue, Sep 10, 2013 at 11:56:55AM +0200, Vratislav Podzimek wrote:
> > Related: rhbz#972362
> > Signed-off-by: Vratislav Podzimek
> >
> > pyanaconda/ui/gui/spokes/network.py | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
> > index 43d12e1..2e80a3c 100644
> > --- a/pyanaconda/ui/gui/spokes/network.py
> > +++ b/pyanaconda/ui/gui/spokes/network.py
> > @@ -1275,6 +1275,10 @@ class NetworkSpoke(FirstbootSpokeMixIn, NormalSpoke):
> >          log.debug("network: apply ksdata %s", self.data.network)
> >          self.network_control_box.kill_nmce(msg="leaving network spoke")
> > def execute(self):
> >    # update system's hostname
> >    network.set_hostname(self.data.network.hostname)
> > @property
> >    def completed(self):
> >        # TODO: check also if source requires updates when implemented
> >
> > @@ -1436,6 +1440,10 @@ class NetworkStandaloneSpoke(StandaloneSpoke):
> >      self.network_control_box.kill_nmce(msg="leaving standalone network spoke")
> >
> > def execute(self):
> >    # update system's hostname
> >    network.set_hostname(self.data.network.hostname)
> > @property
> >    def completed(self):
> >        return (not can_touch_runtime_system("require network connection")
> >
> > -- 
> > 1.7.11.7
>
> I noticed that set_hostname is only blocking hostname setting on image
> installs. It also needs to make sure it doesn't set it for dirinstall
> since that could also be on a host not related to the target.

Yeah, I've noticed that too. Now we can simply use
can_touch_runtime_system with touch_live set to True. That will also
cover testing environment and whatever comes in the future. Maybe I
could look at flags.*install usage in general and replace it with
can_touch_runtime_system where possible.

[PATCH 2/2] Don't use temporary file and move when writing out an ifcfg file

[Vratislav Podzimek]

NetworkManager uses IN_CLOSE_WRITE inotify event which is not triggered if a new
file is moved to the place of the watched file. Also the SELinux context is
wrong when a file from /tmp is used to the /etc tree.

Related: rhbz#972362

Based on the patch from Hans de Goede .

Signed-off-by: Vratislav Podzimek 
---
 pyanaconda/simpleconfig.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py
index d0757cf..8d51db3 100644
--- a/pyanaconda/simpleconfig.py
+++ b/pyanaconda/simpleconfig.py
@@ -176,8 +176,6 @@ class IfcfgFile(SimpleConfigFile):
         SimpleConfigFile.read(self, self.path)
         return len(self.info)

-    # ifcfg-rh is using inotify IN_CLOSE_WRITE event
-    # so we don't use temporary file for new configuration.
     def write(self, directory=None):
         """ Writes values into ifcfg file.
         """
@@ -187,5 +185,8 @@ class IfcfgFile(SimpleConfigFile):
         else:
             path = os.path.join(directory, os.path.basename(self.path))

-        SimpleConfigFile.write(self, path)
+        # ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use
+        # temporary file for new configuration
+        with open(path, "w") as fobj:
+            fobj.write(str(self))

[PATCH 2/2] Don't use temporary file and move when writing out an ifcfg file

[Brian C. Lane]

On Tue, Sep 10, 2013 at 11:56:56AM +0200, Vratislav Podzimek wrote:
> NetworkManager uses IN_CLOSE_WRITE inotify event which is not triggered if a new
> file is moved to the place of the watched file. Also the SELinux context is
> wrong when a file from /tmp is used to the /etc tree.
>
> Related: rhbz#972362
>
> Based on the patch from Hans de Goede .
> Signed-off-by: Vratislav Podzimek
>
> pyanaconda/simpleconfig.py | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py
> index d0757cf..8d51db3 100644
> --- a/pyanaconda/simpleconfig.py
> +++ b/pyanaconda/simpleconfig.py
> @@ -176,8 +176,6 @@ class IfcfgFile(SimpleConfigFile):
>          SimpleConfigFile.read(self, self.path)
>          return len(self.info)
> # ifcfg-rh is using inotify IN_CLOSE_WRITE event
> # so we don't use temporary file for new configuration.
> def write(self, directory=None):
>     """ Writes values into ifcfg file.
>     """
>
> @@ -187,5 +185,8 @@ class IfcfgFile(SimpleConfigFile):
>          else:
>              path = os.path.join(directory, os.path.basename(self.path))
>    SimpleConfigFile.write(self, path)
>    # ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use
>    # temporary file for new configuration
>    with open(path, "w") as fobj:
>        fobj.write(str(self))
>
> -- 
> 1.7.11.7

I think I'd rather see SimpleConfigFile.write gain an option to skip
using temporary files. eg.

def write(self, filename=None, use_tmp=True)

that capability may be useful to other callers in the future.

[PATCH 2/2 v2] Don't use temporary file and move when writing out an ifcfg file

[Vratislav Podzimek]

NetworkManager uses IN_CLOSE_WRITE inotify event which is not triggered if a new
file is moved to the place of the watched file. Also the SELinux context is
wrong when a file from /tmp is used to the /etc tree.

Related: rhbz#972362

Based on the patch from Hans de Goede .

Signed-off-by: Vratislav Podzimek 
---
 pyanaconda/simpleconfig.py | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py
index d0757cf..ae2e3dc 100644
--- a/pyanaconda/simpleconfig.py
+++ b/pyanaconda/simpleconfig.py
@@ -70,26 +70,31 @@ class SimpleConfigFile(object):
                 if key:
                     self.info[key] = value

-    def write(self, filename=None):
+    def write(self, filename=None, use_tmp=True):
         """ passing filename will override the filename passed to init.
         """
         filename = filename or self.filename
         if not filename:
             return None

-        tmpf = tempfile.NamedTemporaryFile(mode="w", delete=False)
-        tmpf.write(str(self))
-        tmpf.close()
+        if use_tmp:
+            tmpf = tempfile.NamedTemporaryFile(mode="w", delete=False)
+            tmpf.write(str(self))
+            tmpf.close()

-        # Move the temporary file (with 0600 permissions) over the top of the
-        # original and preserve the original's permissions
-        filename = os.path.realpath(filename)
-        if os.path.exists(filename):
-            m = os.stat(filename).st_mode
+            # Move the temporary file (with 0600 permissions) over the top of the
+            # original and preserve the original's permissions
+            filename = os.path.realpath(filename)
+            if os.path.exists(filename):
+                m = os.stat(filename).st_mode
+            else:
+                m = int('0100644', 8)
+            shutil.move(tmpf.name, filename)
+            os.chmod(filename, m)
         else:
-            m = int('0100644', 8)
-        shutil.move(tmpf.name, filename)
-        os.chmod(filename, m)
+            # write directly to the file
+            with open(filename, "w") as fobj:
+                fobj.write(str(self))

def set(self, *args):
         for key, value in args:
@@ -176,8 +181,6 @@ class IfcfgFile(SimpleConfigFile):
         SimpleConfigFile.read(self, self.path)
         return len(self.info)

-    # ifcfg-rh is using inotify IN_CLOSE_WRITE event
-    # so we don't use temporary file for new configuration.
     def write(self, directory=None):
         """ Writes values into ifcfg file.
         """
@@ -187,5 +190,6 @@ class IfcfgFile(SimpleConfigFile):
         else:
             path = os.path.join(directory, os.path.basename(self.path))

-        SimpleConfigFile.write(self, path)
-
+        # ifcfg-rh is using inotify IN_CLOSE_WRITE event so we don't use
+        # temporary file for new configuration
+        SimpleConfigFile.write(self, path, use_tmp=False)

[PATCH 3/3] set_hostname should proceed only on DVD and live installations

[Vratislav Podzimek]

Signed-off-by: Vratislav Podzimek 
---
 pyanaconda/network.py | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/pyanaconda/network.py b/pyanaconda/network.py
index 692d655..d21e9b0 100644
--- a/pyanaconda/network.py
+++ b/pyanaconda/network.py
@@ -34,7 +34,6 @@ import threading
 import re
 import dbus
 import IPy
-from flags import flags

from simpleconfig import IfcfgFile
 from blivet.devices import FcoeDiskDevice, iScsiDiskDevice
@@ -42,6 +41,7 @@ import blivet.arch

from pyanaconda import nm
 from pyanaconda import constants
+from pyanaconda.flags import flags, can_touch_runtime_system
 from pyanaconda.i18n import _

import logging
@@ -751,12 +751,9 @@ def get_ksdevice_name(ksspec=""):
     return ksdevice

def set_hostname(hn):
-    if flags.imageInstall:
-        log.info("image install -- not setting hostname")
-        return
-
-    log.info("setting installation environment hostname to %s", hn)
-    iutil.execWithRedirect("hostnamectl", ["set-hostname", hn])
+    if can_touch_runtime_system("set hostname", touch_live=True):
+        log.info("setting installation environment hostname to %s", hn)
+        iutil.execWithRedirect("hostnamectl", ["set-hostname", hn])

def write_hostname(rootpath, ksdata, overwrite=False):
     cfgfile = os.path.normpath(rootpath + hostnameFile)

[PATCH 3/3] set_hostname should proceed only on DVD and live installations

[Brian C. Lane]

Ack to all of these, thanks.