-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Comments below.
On Tue, 27 Apr 2010, Radek Vykydal wrote:
Remove reading of some keys from NM via dbus, count on ifcfg
file contents (written by stage 1, ks, or nm-c-e). This should
also ensure support for ipv6 in stage2 using nm-c-e.
Remove available() method - it is useless - HWADDR is read from ifcfg
file, description is set in constructor, and Network class will be
kept up-to-date (e.g. after configuring with nm-c-e) by explicit
call to added update() method.
readIfcfgContets became useless, we can use (now ifcfg-backed)
NetworkDevices' method loadIfcfgFile instead.
Hunk starting with if len(available_devices) > 0: is a noop,
probably some leftover - removing.
---
iw/advanced_storage.py | 2 +-
network.py | 159 ++++++++++------------------------------------
textw/add_drive_text.py | 2 +-
textw/netconfig_text.py | 4 +-
textw/partition_text.py | 1 +
5 files changed, 40 insertions(+), 128 deletions(-)
diff --git a/iw/advanced_storage.py b/iw/advanced_storage.py
index d3d4360..c03b3ab 100644
--- a/iw/advanced_storage.py
+++ b/iw/advanced_storage.py
@@ -43,7 +43,7 @@ def addFcoeDrive(anaconda):
store = gtk.TreeStore(gobject.TYPE_STRING, gobject.TYPE_STRING)
combo.set_model(store)
- netdevs = anaconda.network.available()
+ netdevs = anaconda.network.netdevices
keys = netdevs.keys()
keys.sort()
selected_interface = None
diff --git a/network.py b/network.py
index 20a21b2..14af8c8 100644
--- a/network.py
+++ b/network.py
@@ -315,145 +315,56 @@ class NetworkDevice(IfcfgFile):
class Network:
+
def __init__(self):
+
+ self.hostname = socket.gethostname()
+ self.overrideDHCPhostname = False
+ self.update()
+
+ def update(self):
+
self.netdevices = {}
self.ksdevice = None
self.domains = []
- self.hostname = socket.gethostname()
- self.overrideDHCPhostname = False
See below.
# populate self.netdevices
devhash = isys.getDeviceProperties(dev=None)
- for dev in devhash.keys():
- self.netdevices[dev] = NetworkDevice(dev)
- ifcfg_contents = self.readIfcfgContents(dev)
-
- # if NM_CONTROLLED is set to yes, we read in settings from
- # NetworkManager first, then fill in the gaps with the data
- # from the ifcfg file
- useNetworkManager = False
- if ifcfg_contents.has_key('NM_CONTROLLED') and \
- not ifcfg_contents['NM_CONTROLLED'].lower() == 'no':
- useNetworkManager = True
-
- # this interface is managed by NetworkManager, so read from
- # NetworkManager first
- if useNetworkManager:
- props = devhash[dev]
-
- if isys.isDeviceDHCP(dev):
- self.netdevices[dev].set(('BOOTPROTO', 'dhcp'))
- else:
- self.netdevices[dev].unset('BOOTPROTO')
- bus = dbus.SystemBus()
- config_path = props.Get(isys.NM_MANAGER_IFACE, 'Ip4Config')
- config = bus.get_object(isys.NM_SERVICE, config_path)
- config_props = dbus.Interface(config, isys.DBUS_PROPS_IFACE)
-
- # addresses (3-element list: ipaddr, netmask, gateway)
- addrs = config_props.Get(isys.NM_MANAGER_IFACE, 'Addresses')[0]
- try:
- tmp = struct.pack('I', addrs[0])
- ipaddr = socket.inet_ntop(socket.AF_INET, tmp)
- self.netdevices[dev].set(('IPADDR', ipaddr))
- except:
- pass
-
- try:
- tmp = struct.pack('I', addrs[1])
- netmask = socket.inet_ntop(socket.AF_INET, tmp)
- self.netdevices[dev].set(('NETMASK', netmask))
- except:
- pass
-
- try:
- tmp = struct.pack('I', addrs[2])
- gateway = socket.inet_ntop(socket.AF_INET, tmp)
- self.netdevices[dev].set(('GATEWAY', gateway))
- except:
- pass
-
- self.hostname = socket.gethostname()
-
- # read in remaining settings from ifcfg file
- for key in ifcfg_contents.keys():
- if key == 'GATEWAY':
- self.netdevices[dev].set((key, ifcfg_contents[key]))
- elif key == 'DOMAIN':
- self.domains.append(ifcfg_contents[key])
- elif key == 'HOSTNAME':
- self.hostname = ifcfg_contents[key]
- elif self.netdevices[dev].get(key) == '':
- self.netdevices[dev].set((key, ifcfg_contents[key]))
-
- # now initialize remaining devices
- # XXX we just throw return away, the method initialize a
- # object member so we dont need to
- available_devices = self.available()
-
- if len(available_devices) > 0:
- # set first device to start up onboot
- oneactive = 0
- for dev in available_devices.keys():
- try:
- if available_devices[dev].get("ONBOOT") == "yes":
- oneactive = 1
- break
- except:
- continue
+ for iface in devhash.keys():
+ device = NetworkDevice(netscriptsDir, iface, logfile=ifcfgLogFile)
+ device.loadIfcfgFile()
+ device.log("===== Network.update\n")
Just concerns...
The concern I have with eliminating this loop is for upgrade mode and rescue
mode, but maybe we have eliminated this code path for those modes. I do not
know. Based on your other patches in this series, handling interfaces that
are NM_CONTROLLED 'yes' or 'no' seems pointless if NM can use the ifcfg file.
D-Bus is, in theory, the preferred way to talk to NetworkManager. However,
since the move to using NetworkManager in the installer, the D-Bus API has
been extremely unreliable, so I have no problems eliminating our usage of it
here and relying on the ifcfg file contents instead.
- def readIfcfgContents(self, dev):
- ifcfg = "/etc/sysconfig/network-scripts/ifcfg-%s" % (dev,)
- contents = {}
+ if device.get('DOMAIN'):
+ self.domains.append(device.get('DOMAIN'))
+ # TODORV - the last iface in loop wins, might be ok,
+ # not worthy of special juggling
+ if device.get('HOSTNAME'):
+ self.hostname = device.get('HOSTNAME')
The HOSTNAME value in /etc/sysconfig/network should always override HOSTNAME
from an ifcfg file. Likewise, if we can do a reverse DNS lookup and get a
hostname, we should use that if the ifcfg file and network file did not
provide us with one.
- try:
- f = open(ifcfg, "r")
- lines = f.readlines()
- f.close()
+ device.description = isys.getNetDevDesc(iface)
- for line in lines:
- line = line.strip()
- if line.startswith('#') or line == '':
- continue
+ self.netdevices[iface] = device
- var = string.splitfields(line, '=', 1)
- if len(var) == 2:
- var[1] = var[1].replace('"', '')
- contents[var[0]] = string.strip(var[1])
- except:
- return {}
-
- return contents
-
- def getDevice(self, device):
- return self.netdevices[device]
-
- def available(self):
- ksdevice = None
- if flags.cmdline.has_key('ksdevice'):
- ksdevice = flags.cmdline['ksdevice']
-
- for dev in isys.getDeviceProperties().keys():
- if not self.netdevices.has_key(dev):
- self.netdevices[dev] = NetworkDevice(dev)
-
- hwaddr = isys.getMacAddress(dev)
- self.netdevices[dev].set(('HWADDR', hwaddr))
- self.netdevices[dev].set(('DESC', isys.getNetDevDesc(dev)))
+ ksdevice = flags.cmdline.get('ksdevice', None)
+ if ksdevice:
+ for dev in self.netdevices:
+ if ksdevice == 'link' and isys.getLinkStatus(dev):
+ self.ksdevice = dev
+ break
+ elif ksdevice == dev:
+ self.ksdevice = dev
+ break
+ elif ':' in ksdevice:
+ if ksdevice.upper() == self.netdevices[dev].get('HWADDR'):
+ self.ksdevice = dev
+ break
- if not ksdevice:
- continue
- if ksdevice == 'link' and isys.getLinkStatus(dev):
- self.ksdevice = dev
- elif ksdevice == dev:
- self.ksdevice = dev
- elif ksdevice.find(':') != -1:
- if ksdevice.upper() == hwaddr:
- self.ksdevice = dev
The available() method is meant to give us a list of usable interfaces during
installation, not of all interfaces. Are you accounting for that elsewhere?
Things like the advanced network storage configuration dialog or the add repo
dialog present a list of usable network devices, but on the network screen in
anaconda we do want people to be able to configure all network interfaces in
the system and not just the ones we have found to be usable during
installation.
- return self.netdevices
+ def getDevice(self, device):
+ return self.netdevices[device]
def getKSDevice(self):
if self.ksdevice is None:
diff --git a/textw/add_drive_text.py b/textw/add_drive_text.py
index 0fbd86c..663d45d 100644
--- a/textw/add_drive_text.py
+++ b/textw/add_drive_text.py
@@ -78,7 +78,7 @@ class addDriveDialog(object):
return INSTALL_OK
def addFcoeDriveDialog(self, screen):
- netdevs = self.anaconda.network.available()
+ netdevs = self.anaconda.network.netdevices
devs = netdevs.keys()
devs.sort()
diff --git a/textw/netconfig_text.py b/textw/netconfig_text.py
index 4cea192..fb37429 100644
--- a/textw/netconfig_text.py
+++ b/textw/netconfig_text.py
@@ -96,7 +96,7 @@ class NetworkConfiguratorText:
self.interfaceList = CheckboxTree(height=3, scroll=1)
- netdevs = self.anaconda.network.available()
+ netdevs = self.anaconda.network.netdevices
devs = netdevs.keys()
devs.sort()
ksdevice = self.anaconda.network.getKSDevice()
@@ -178,7 +178,7 @@ class NetworkConfiguratorText:
#self._ipv6Toggled()
self._dhcpToggled()
- netdevs = self.anaconda.network.available()
+ netdevs = self.anaconda.network.netdevices
while True:
result = grid.run()
diff --git a/textw/partition_text.py b/textw/partition_text.py
index 193ff21..c9cac6f 100644
--- a/textw/partition_text.py
+++ b/textw/partition_text.py
@@ -160,3 +160,4 @@ class PartitionTypeWindow:
return INSTALL_OK
+
Unnecessary whitespace change in textw/partition_text.py
- --
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iEYEARECAAYFAkvfjKcACgkQ5hsjjIy1VklT3gCg9DA6gDrTXnSKN7rTTDc8hGIv
v4IAnRHMKysyL97ynGctnsSvBBOvtTuQ
=GnFq
-----END PGP SIGNATURE-----
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
