Still logging into /tmp/ifcfg.log, but now - standard format is used, - values of IfcfgFile are removed (only what would be written-out, that is __str__() output is kept) - it was only confusing, should be part of unit tests if anything - logging of some events is added (e.g. Network.write(), Network.update()) --- pyanaconda/gui.py | 6 ++- pyanaconda/network.py | 105 ++++++++++++++++++++++++------------------------- 2 files changed, 56 insertions(+), 55 deletions(-)
diff --git a/pyanaconda/gui.py b/pyanaconda/gui.py index 1853b2d..0b593b6 100755 --- a/pyanaconda/gui.py +++ b/pyanaconda/gui.py @@ -942,9 +942,11 @@ class InstallInterface(InstallInterfaceBase): self.anaconda.network.updateIfcfgsSSID(dev_ssids)
self.anaconda.network.writeIfcfgFiles() - network.logIfcfgFiles(header="========== before nm-c-e run\n") + # Logging can race here with ifcfg-rh updating the file + network.logIfcfgFiles(message="Dump before nm-c-e (can race " + "with ifcfg updating). ") runNMCE(self.anaconda) - network.logIfcfgFiles(header="========== after nm-c-e run\n") + network.logIfcfgFiles(message="Dump after nm-c-e. ")
self.anaconda.network.update()
diff --git a/pyanaconda/network.py b/pyanaconda/network.py index 5dc81cb..8998759 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -49,6 +49,19 @@ networkConfFile = "%s/network" % (sysconfigDir) ifcfgLogFile = "/tmp/ifcfg.log" CONNECTION_TIMEOUT = 45
+# Setup special logging for ifcfg NM interface +from pyanaconda import anaconda_log +logger = logging.getLogger("ifcfg") +logger.setLevel(logging.DEBUG) +anaconda_log.logger.addFileHandler(ifcfgLogFile, logger, logging.DEBUG) +anaconda_log.logger.addFileHandler("/dev/tty3", logger, + anaconda_log.DEFAULT_TTY_LEVEL, + anaconda_log.TTY_FORMAT, + autoLevel=True) +anaconda_log.logger.forwardToSyslog(logger) + +ifcfglog = logging.getLogger("ifcfg") + class IPError(Exception): pass
@@ -175,33 +188,24 @@ def getActiveNetDevs(): ret.sort() return ret
-def logIfcfgFile(path, header="\n"): - logfile = ifcfgLogFile - if not os.access(path, os.R_OK): - return - f = open(path, 'r') - lf = open(logfile, 'a') - lf.write(header) - lf.write(f.read()) - lf.close() - f.close() - -def logIfcfgFiles(header="\n"): - - lf = open(ifcfgLogFile, 'a') - lf.write(header) - lf.close() +def logIfcfgFile(path, message=""): + content = "" + if os.access(path, os.R_OK): + f = open(path, 'r') + content = f.read() + f.close() + ifcfglog.debug("%s%s:\n%s" % (message, path, content))
+def logIfcfgFiles(message=""): devprops = isys.getDeviceProperties(dev=None) for device in devprops: path = "%s/ifcfg-%s" % (netscriptsDir, device) - logIfcfgFile(path, "===== %s\n" % (path,)) + logIfcfgFile(path, message)
class NetworkDevice(IfcfgFile):
- def __init__(self, dir, iface, logfile='/tmp/ifcfg.log'): + def __init__(self, dir, iface): IfcfgFile.__init__(self, dir, iface) - self.logfile = logfile self.description = "" if iface.startswith('ctc'): self.info["TYPE"] = "CTC" @@ -218,8 +222,9 @@ class NetworkDevice(IfcfgFile): s = "" keys = self.info.keys() keys.sort() - keys.remove("DEVICE") - keys.insert(0, "DEVICE") + if ("DEVICE" in keys): + keys.remove("DEVICE") + keys.insert(0, "DEVICE") if iutil.isS390() and ("HWADDR" in keys): keys.remove("HWADDR") # make sure we include autoneg in the ethtool line @@ -234,23 +239,39 @@ class NetworkDevice(IfcfgFile):
return s
+ def loadIfcfgFile(self): + ifcfglog.debug("%s:\n%s" % (self.path, self.fileContent())) + ifcfglog.debug("NetworkDevice %s:\n%s" % (self.iface, self.__str__())) + ifcfglog.debug("loadIfcfgFile %s from %s" % (self.iface, self.path)) + self.clear() IfcfgFile.read(self) - self.log("NetworkDevice read from %s\n" % self.path) self._dirty = False
+ ifcfglog.debug("NetworkDevice %s:\n%s" % (self.iface, self.__str__())) + def writeIfcfgFile(self): # Write out the file only if there is a key whose # value has been changed since last load of ifcfg file. + ifcfglog.debug("%s:\n%s" % (self.path, self.fileContent())) + ifcfglog.debug("NetworkDevice %s:\n%s" % (self.iface, self.__str__())) + ifcfglog.debug("writeIfcfgFile %s to %s%s" % (self.iface, self.path, + (self._dirty and "" or " not needed"))) + if self._dirty: IfcfgFile.write(self) - self.log("NetworkDevice written to %s\n" % self.path) self._dirty = False
+ # We can't read the file right now racing with ifcfg-rh update + #ifcfglog.debug("%s:\n%s" % (device.path, device.fileContent())) + def set(self, *args): # If we are changing value of a key set _dirty flag # informing that ifcfg file needs to be synced. + s = " ".join(["%s=%s" % key_val for key_val in args]) + ifcfglog.debug("NetworkDevice %s set: %s" % + (self.iface, s)) for (key, data) in args: if self.get(key) != data: break @@ -286,37 +307,11 @@ class NetworkDevice(IfcfgFile): raise shutil.move(newifcfg, keyfile)
- - def log(self, header="\n"): - lf = open(self.logfile, 'a') - lf.write(header) - lf.close() - self.log_file() - self.log_write_file() - self.log_values() - - def log_values(self, header="\n"): - lf = open(self.logfile, 'a') - lf.write(header) - lf.write("== values for file %s\n" % self.path) - lf.write(IfcfgFile.__str__(self)) - lf.close() - - def log_write_file(self, header="\n"): - lf = open(self.logfile, 'a') - lf.write(header) - lf.write("== file to be written for %s\n" % self.path) - lf.write(self.__str__()) - lf.close() - - def log_file(self, header="\n"): + def fileContent(self): f = open(self.path, 'r') - lf = open(self.logfile, 'a') - lf.write(header) - lf.write("== file %s\n" % self.path) - lf.write(f.read()) - lf.close() + content = f.read() f.close() + return content
def usedByFCoE(self, anaconda): import storage @@ -365,6 +360,8 @@ class Network:
def update(self):
+ ifcfglog.debug("Network.update() called") + self.netdevices = {} self.ksdevice = None self.domains = [] @@ -372,7 +369,7 @@ class Network: # populate self.netdevices devhash = isys.getDeviceProperties(dev=None) for iface in devhash.keys(): - device = NetworkDevice(netscriptsDir, iface, logfile=ifcfgLogFile) + device = NetworkDevice(netscriptsDir, iface) if os.access(device.path, os.R_OK): device.loadIfcfgFile() else: @@ -637,6 +634,8 @@ class Network:
def write(self):
+ ifcfglog.debug("Network.write() called") + devices = self.netdevices.values()
if len(devices) == 0:
--- pyanaconda/exception.py | 4 ++-- pyanaconda/packages.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py index 1075717..e686427 100644 --- a/pyanaconda/exception.py +++ b/pyanaconda/exception.py @@ -91,8 +91,8 @@ class AnacondaExceptionHandler(ExceptionHandler):
def initExceptionHandling(anaconda): fileList = [ "/tmp/anaconda.log", "/tmp/lvmout", "/tmp/resize.out", - "/tmp/program.log", "/tmp/storage.log", "/tmp/yum.log", - anaconda.rootPath + "/root/install.log", + "/tmp/program.log", "/tmp/storage.log", "/tmp/ifcfg.log", + "/tmp/yum.log", anaconda.rootPath + "/root/install.log", anaconda.rootPath + "/root/upgrade.log", "/proc/cmdline" ] if flags.livecdInstall: fileList.extend(["/var/log/dmesg"]) diff --git a/pyanaconda/packages.py b/pyanaconda/packages.py index 7d88d06..079dd67 100644 --- a/pyanaconda/packages.py +++ b/pyanaconda/packages.py @@ -72,6 +72,7 @@ def copyAnacondaLogs(anaconda): ("/tmp/X.log", "anaconda.xlog"), ("/tmp/program.log", "anaconda.program.log"), ("/tmp/storage.log", "anaconda.storage.log"), + ("/tmp/ifcfg.log", "anaconda.ifcfg.log"), ("/tmp/yum.log", "anaconda.yum.log")): if os.access(fn, os.R_OK): try:
diff --git a/pyanaconda/packages.py b/pyanaconda/packages.py index 7d88d06..079dd67 100644 --- a/pyanaconda/packages.py +++ b/pyanaconda/packages.py @@ -72,6 +72,7 @@ def copyAnacondaLogs(anaconda): ("/tmp/X.log", "anaconda.xlog"), ("/tmp/program.log", "anaconda.program.log"), ("/tmp/storage.log", "anaconda.storage.log"),
("/tmp/ifcfg.log", "anaconda.ifcfg.log"), ("/tmp/yum.log", "anaconda.yum.log")): if os.access(fn, os.R_OK): try:
Do we really need this log on the filesystem post-installation?
Also, it might be worth considering making /var/log/anaconda/ and putting all this stuff in there. We're starting to make a bigger and bigger mess in /var/log.
- Chris
On 08/23/2010 09:17 PM, Chris Lumens wrote:
diff --git a/pyanaconda/packages.py b/pyanaconda/packages.py index 7d88d06..079dd67 100644 --- a/pyanaconda/packages.py +++ b/pyanaconda/packages.py @@ -72,6 +72,7 @@ def copyAnacondaLogs(anaconda): ("/tmp/X.log", "anaconda.xlog"), ("/tmp/program.log", "anaconda.program.log"), ("/tmp/storage.log", "anaconda.storage.log"),
("/tmp/ifcfg.log", "anaconda.ifcfg.log"), ("/tmp/yum.log", "anaconda.yum.log")): if os.access(fn, os.R_OK): try:
Do we really need this log on the filesystem post-installation?
Yes, it's easier to just ask for logs from installed system then to ask to reproduce - there are cases when something going wrong with networking doesn't prevent finishing install (like no networking after boot, wrong configuration of net devices not used for installing, esp. with kickstart). It's based on my experience with bug reports I met. And there is no cost except for /var/log being cluttered with our logs - there you are right.
Also, it might be worth considering making /var/log/anaconda/ and putting all this stuff in there. We're starting to make a bigger and bigger mess in /var/log.
I agree, I'll talk with Ales about it.
Radek
On 08/23/2010 01:05 PM, Radek Vykydal wrote:
Still logging into /tmp/ifcfg.log, but now
- standard format is used,
- values of IfcfgFile are removed (only what would be written-out, that is __str__() output is kept) - it was only confusing, should be part of unit tests if anything
- logging of some events is added (e.g. Network.write(), Network.update())
pyanaconda/gui.py | 6 ++- pyanaconda/network.py | 105 ++++++++++++++++++++++++------------------------- 2 files changed, 56 insertions(+), 55 deletions(-)
Attaching example of old and new log of the same scenario.
On 08/23/2010 01:05 PM, Radek Vykydal wrote:
Still logging into /tmp/ifcfg.log, but now
- standard format is used,
- values of IfcfgFile are removed (only what would be written-out, that is __str__() output is kept) - it was only confusing, should be part of unit tests if anything
- logging of some events is added (e.g. Network.write(), Network.update())
pyanaconda/gui.py | 6 ++- pyanaconda/network.py | 105 ++++++++++++++++++++++++------------------------- 2 files changed, 56 insertions(+), 55 deletions(-)
Ack, with two comments:
diff --git a/pyanaconda/network.py b/pyanaconda/network.py index 5dc81cb..8998759 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -49,6 +49,19 @@ networkConfFile = "%s/network" % (sysconfigDir) ifcfgLogFile = "/tmp/ifcfg.log" CONNECTION_TIMEOUT = 45
+# Setup special logging for ifcfg NM interface +from pyanaconda import anaconda_log +logger = logging.getLogger("ifcfg") +logger.setLevel(logging.DEBUG) +anaconda_log.logger.addFileHandler(ifcfgLogFile, logger, logging.DEBUG) +anaconda_log.logger.addFileHandler("/dev/tty3", logger,
anaconda_log.DEFAULT_TTY_LEVEL,
anaconda_log.TTY_FORMAT,
autoLevel=True)
+anaconda_log.logger.forwardToSyslog(logger)
+ifcfglog = logging.getLogger("ifcfg")
1) for a lack of better places to put this.. One day I'd like to move storage log init and now this log init too into anaconda_log.py.
2) if you want ifcfg messages to be correctly filed during remote logging, you need to update scripts/analog too. You can inspire yourself with how we handle "storage" there. Please either do this (the recommended option) or remove the "forwardToSyslog" call.
Ales
On 09/08/2010 01:34 PM, Ales Kozumplik wrote:
On 08/23/2010 01:05 PM, Radek Vykydal wrote:
Still logging into /tmp/ifcfg.log, but now
- standard format is used,
- values of IfcfgFile are removed (only what would be written-out, that is __str__() output is kept) - it was only confusing, should be part of unit tests if anything
- logging of some events is added (e.g. Network.write(), Network.update())
pyanaconda/gui.py | 6 ++- pyanaconda/network.py | 105 ++++++++++++++++++++++++------------------------- 2 files changed, 56 insertions(+), 55 deletions(-)
Ack, with two comments:
diff --git a/pyanaconda/network.py b/pyanaconda/network.py index 5dc81cb..8998759 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -49,6 +49,19 @@ networkConfFile = "%s/network" % (sysconfigDir) ifcfgLogFile = "/tmp/ifcfg.log" CONNECTION_TIMEOUT = 45
+# Setup special logging for ifcfg NM interface +from pyanaconda import anaconda_log +logger = logging.getLogger("ifcfg") +logger.setLevel(logging.DEBUG) +anaconda_log.logger.addFileHandler(ifcfgLogFile, logger, logging.DEBUG) +anaconda_log.logger.addFileHandler("/dev/tty3", logger,
anaconda_log.DEFAULT_TTY_LEVEL,
anaconda_log.TTY_FORMAT,
autoLevel=True)
+anaconda_log.logger.forwardToSyslog(logger)
+ifcfglog = logging.getLogger("ifcfg")
- for a lack of better places to put this.. One day I'd like to move
storage log init and now this log init too into anaconda_log.py.
Yeah, I was imagining something like that so I followed storage logging here.
- if you want ifcfg messages to be correctly filed during remote
logging, you need to update scripts/analog too. You can inspire yourself with how we handle "storage" there. Please either do this (the recommended option) or remove the "forwardToSyslog" call.
I will send update of analog in another patch.
Thanks for the review.
On 09/08/2010 01:46 PM, Radek Vykydal wrote:
On 09/08/2010 01:34 PM, Ales Kozumplik wrote:
- if you want ifcfg messages to be correctly filed during remote
logging, you need to update scripts/analog too. You can inspire yourself with how we handle "storage" there. Please either do this (the recommended option) or remove the "forwardToSyslog" call.
I will send update of analog in another patch.
Here you are, seems pretty easy:
diff --git a/scripts/analog b/scripts/analog index 1041372..3406a97 100755 --- a/scripts/analog +++ b/scripts/analog @@ -68,6 +68,7 @@ $template path_syslog, "%(directory)s/%(subdirectory)s/syslog $template path_anaconda, "%(directory)s/%(subdirectory)s/anaconda.log" $template path_program, "%(directory)s/%(subdirectory)s/program.log" $template path_storage, "%(directory)s/%(subdirectory)s/storage.log" +$template path_ifcfg, "%(directory)s/%(subdirectory)s/ifcfg.log" $template path_sysimage, "%(directory)s/%(subdirectory)s/install.log.syslog"
*.* %(directory)s/debug_all.log;anaconda_debug @@ -80,6 +81,7 @@ authpriv.* ?path_syslog;anaconda_syslo :programname, contains, "anaconda" ?path_anaconda;anaconda_syslog :programname, contains, "program" ?path_program;anaconda_syslog :programname, contains, "storage" ?path_storage;anaconda_syslog +:programname, contains, "ifcfg" ?path_ifcfg;anaconda_syslog :hostname, contains, "sysimage" ?path_sysimage;anaconda_syslog
# discard those that we logged @@ -88,6 +90,7 @@ authpriv.* ?path_syslog;anaconda_syslo :programname, contains, "anaconda" ~ :programname, contains, "program" ~ :programname, contains, "storage" ~ +:programname, contains, "ifcfg" ~ :hostname, contains, "sysimage" ~ kern.* ~ daemon.* ~
On 09/08/2010 01:58 PM, Radek Vykydal wrote:
On 09/08/2010 01:46 PM, Radek Vykydal wrote:
On 09/08/2010 01:34 PM, Ales Kozumplik wrote:
- if you want ifcfg messages to be correctly filed during remote
logging, you need to update scripts/analog too. You can inspire yourself with how we handle "storage" there. Please either do this (the recommended option) or remove the "forwardToSyslog" call.
I will send update of analog in another patch.
Here you are, seems pretty easy:
diff --git a/scripts/analog b/scripts/analog index 1041372..3406a97 100755
Ack.
anaconda-devel@lists.stg.fedoraproject.org