[PATCH 1/2] network: set ONBOOT=yes for the device used for installation (#1002544).

Vratislav Podzimek vpodzime at redhat.com
Mon May 26 06:15:30 UTC 2014


On Fri, 2014-05-23 at 15:36 +0200, Radek Vykydal wrote:
> On 05/23/2014 12:25 PM, Vratislav Podzimek wrote:
> > On Thu, 2014-05-22 at 13:19 +0200, Radek Vykydal wrote:
> >> Resolves: rhbz#1002544
> >>
> >> This addresses the case when the device is activated in GUI or TUI
> >> (default ifcfg files created when anaconda starts have ONBOOT=no)
> >> ---
> >>   pyanaconda/installclasses/rhel.py | 39 +++++++++++++++++++++++++++++++++++++++
> >>   pyanaconda/network.py             | 17 +++++++++++++++++
> >>   2 files changed, 56 insertions(+)
> >>
> >> diff --git a/pyanaconda/installclasses/rhel.py b/pyanaconda/installclasses/rhel.py
> >> index d3feed3..39c959b 100644
> >> --- a/pyanaconda/installclasses/rhel.py
> >> +++ b/pyanaconda/installclasses/rhel.py
> >> @@ -20,6 +20,9 @@
> >>   from pyanaconda.i18n import N_
> >>   from pyanaconda.installclass import BaseInstallClass
> >>   from pyanaconda.product import productName
> >> +from pyanaconda import network
> >> +from pyanaconda import iutil
> >> +from pyanaconda import nm
> >>   
> >>   class InstallClass(BaseInstallClass):
> >>       # name has underscore used for mnemonics, strip if you dont need it
> >> @@ -45,5 +48,41 @@ class InstallClass(BaseInstallClass):
> >>           BaseInstallClass.configure(self, anaconda)
> >>           BaseInstallClass.setDefaultPartitioning(self, anaconda.storage)
> >>   
> >> +    def setNetworkOnbootDefault(self, ksdata):
> >> +        # for installations using network
> >> +        if ksdata.method.method not in ("url", "nfs"):
> >> +            return
> >> +
> >> +        # if there is no device to be autoactivated after reboot (we set all
> >> +        # devices not used in initramfs to ONBOOT=no by default)
> >> +        for devName in nm.nm_devices():
> >> +            if nm.nm_device_type_is_wifi(devName):
> >> +                continue
> >> +            try:
> >> +                onboot = nm.nm_device_setting_value(devName, "connection", "autoconnect")
> >> +            except nm.SettingsNotFoundError:
> >> +                continue
> >> +            if not onboot == False:
> > Any reason for not writing this as 'if onboot == True' or just 'if
> > onboot'?
> 
> None means the option was not found which means default value (True) is 
> used by NM.
> I'll better add a comment, (and change to "if onboot or onboot == None:" ?)
A comment would be nice. And if you replace '== None' with 'is None' in
the above it would be better readable than 'not onboot == False' I
guess.


> >
> >> +                return
> >> +
> >> +        # set ONBOOT=yes for the device used during installation
> >> +        # (ie for majority of cases the one having the default route)
> >> +        devName = network.default_route_device()
> >> +        if not devName:
> >> +            return
> >> +        if nm.nm_device_type_is_wifi(devName):
> >> +            return
> >> +        ifcfg_path = network.find_ifcfg_file_of_device(devName, root_path=iutil.getSysroot())
> >> +        if not ifcfg_path:
> >> +            return
> >> +        ifcfg = network.IfcfgFile(ifcfg_path)
> >> +        ifcfg.read()
> >> +        ifcfg.set(('ONBOOT', 'yes'))
> >> +        ifcfg.write()
> >> +        for nd in ksdata.network.network:
> >> +            if nd.device == devName:
> >> +                nd.onboot = True
> >> +                break
> >> +
> >>       def __init__(self):
> >>           BaseInstallClass.__init__(self)
> >> diff --git a/pyanaconda/network.py b/pyanaconda/network.py
> >> index 4b52548..fd8d092 100644
> >> --- a/pyanaconda/network.py
> >> +++ b/pyanaconda/network.py
> >> @@ -846,6 +846,23 @@ def ifaceForHostIP(host):
> >>   
> >>       return routeInfo[routeInfo.index("dev") + 1]
> >>   
> >> +def default_route_device():
> >> +    routes = iutil.execWithCapture("ip", [ "route", "show"])
> > I believe you could use execReadlines here.
> 
> Hm, is this a case the function was intended to be used for? I mean the 
> real-time
> stuff and running a thread? I'd prefer the old good Capture in this case.
That's your choice, it would just spare you splitting the output.

> 
> >> +    if not routes:
> >> +        log.error("Could not get default route device")
> >> +        return None
> >> +
> >> +    for line in routes.split("\n"):
> >> +        if line.startswith("default"):
> >> +            parts = line.split()
> >> +            if parts[3] == "dev":
> >> +                return parts[4]
> > Some check of len(parts) could be nice here.
> 
> It is a question whether we want to traceback or just log
> when output of the called command changes.
> I'll go with the latter and just log.error.
Sounds good to me.

-- 
Vratislav Podzimek

Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic




More information about the anaconda-patches mailing list