New subject: Change in vdsm[master]: Add network logs for VDSM network commands and file operatio...

Saturday, 1 September 2012

Antoni Segura Puimedon has uploaded a new change for review.

Change subject: Bug-Id: https://bugzilla.redhat.com/851839
......................................................................

Bug-Id: https://bugzilla.redhat.com/851839

This patch expands the logging done in network and traffic control by means of
using vdsm.storage.misc.execCmd with a tailored logged that indicates which
module is the origin of the logging.

It also substitutes some of the already existant logging uses with the module-
specific logger created for the execCmd use.

Change-Id: I825431e4a2cbb603fea2b5472b072e5d6a3862e9
Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com&gt;
---
M vdsm/configNetwork.py
M vdsm/tc.py
M vdsm/utils.py
3 files changed, 48 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/7662/1

diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 58117f8..5c58e4a 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -17,7 +17,7 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
-import sys, subprocess, os, re, traceback
+import sys, os, re, traceback
 import pipes
 import pwd
 import time
@@ -42,6 +42,9 @@
 MAX_BRIDGE_NAME_LEN = 15
 ILLEGAL_BRIDGE_CHARS = frozenset(':. \t')
 
+configNetworkLogger = logging.getLogger('configNetwork')
+execCmd = utils.makeExecCmd(log=configNetworkLogger)
+
 class ConfigNetworkError(Exception):
     def __init__(self, errCode, message):
         self.errCode = errCode
@@ -50,25 +53,22 @@
 
 def ifdown(iface):
     "Bring down an interface"
-    p = subprocess.Popen([constants.EXT_IFDOWN, iface], stdout=subprocess.PIPE,
-                    stderr=subprocess.PIPE, close_fds=True)
-    out, err = p.communicate()
+    rc, out, err = execCmd([constants.EXT_IFDOWN, iface], raw=True)
     if out.strip():
         logging.info(out)
     if err.strip():
         logging.warn('\n'.join([line for line in err.splitlines()
                                 if not line.endswith(' does not exist!')]))
-    return p.returncode
+    return rc
 
 def ifup(iface):
     "Bring up an interface"
-    p = subprocess.Popen([constants.EXT_IFUP, iface], stdout=subprocess.PIPE,
-                    stderr=subprocess.PIPE, close_fds=True)
-    out, err = p.communicate()
+    rc, out, err = execCmd([constants.EXT_IFUP, iface], raw=True)
     if out.strip():
         logging.info(out)
     if err.strip():
         logging.warn(err)
+    return rc
 
 def ifaceUsers(iface):
     "Returns a list of entities using the interface"
@@ -157,8 +157,9 @@
 
         mounts = open('/proc/mounts').read()
         if ' /config ext3' in mounts and ' %s ext3' % filename in
mounts:
-            subprocess.call([constants.EXT_UMOUNT, '-n', filename])
+            execCmd([constants.EXT_UMOUNT, '-n', filename])
         utils.rmFile(filename)
+        configNetworkLogger.debug("Removed file %s", filename)
 
     def _createNetwork(self, netXml):
         conn = libvirtconnection.get()
@@ -291,6 +292,8 @@
         for confFile, content in self._backups.iteritems():
             if content is None:
                 utils.rmFile(confFile)
+                configNetworkLogger.debug(
+                        'Removing empty configuration backup %s', confFile)
             else:
                 open(confFile, 'w').write(content)
             logging.info('Restored %s', confFile)
@@ -299,9 +302,9 @@
     def _persistentBackup(cls, filename):
         """ Persistently backup ifcfg-* config files """
         if os.path.exists('/usr/libexec/ovirt-functions'):
-            subprocess.call([constants.EXT_SH, '/usr/libexec/ovirt-functions',
+            execCmd([constants.EXT_SH, '/usr/libexec/ovirt-functions',
                              'unmount_config', filename])
-            logging.debug("unmounted %s using ovirt", filename)
+            configNetworkLogger.debug("unmounted %s using ovirt", filename)
 
         (dummy, basename) = os.path.split(filename)
         if os.path.exists(filename):
@@ -309,7 +312,7 @@
         else:
             # For non-exists ifcfg-* file use predefined header
             content = cls.DELETED_HEADER + '\n'
-        logging.debug("backing up %s: %s", basename, content)
+        configNetworkLogger.debug("backing up %s: %s", basename, content)
 
         cls.writeBackupFile(netinfo.NET_CONF_BACK_DIR, basename, content)
 
@@ -349,16 +352,12 @@
         if not self._backups and not self._networksBackups:
             return
 
-        subprocess.Popen(['/etc/init.d/network', 'stop'],
-                         stdout=subprocess.PIPE,
-                         stderr=subprocess.PIPE).communicate()
+        execCmd(['/etc/init.d/network', 'stop'])
 
         self.restoreAtomicNetworkBackup()
         self.restoreAtomicBackup()
 
-        subprocess.Popen(['/etc/init.d/network', 'start'],
-                         stdout=subprocess.PIPE,
-                         stderr=subprocess.PIPE).communicate()
+        execCmd(['/etc/init.d/network', 'start'])
 
     @classmethod
     def clearBackups(cls):
@@ -374,8 +373,8 @@
         try:
             selinux.restorecon(fileName)
         except:
-            logging.debug('ignoring restorecon error in case SElinux is '
-                          'disabled', exc_info=True)
+            configNetworkLogger.debug('ignoring restorecon error in case '
+                                      'SElinux is disabled', exc_info=True)
 
     def _createConfFile(self, conf, name, ipaddr=None, netmask=None,
                 gateway=None, bootproto=None, mtu=None, onboot='yes', **kwargs):
@@ -405,7 +404,7 @@
             if re.match('^[a-zA-Z_]\w*$', k):
                 cfg += '%s=%s\n' % (k.upper(), pipes.quote(kwargs[k]))
             else:
-                logging.debug('ignoring variable %s', k)
+                configNetworkLogger.debug('ignoring variable %s', k)
 
         self.writeConfFile(self.NET_CONF_PREF + name, cfg)
 
@@ -499,8 +498,7 @@
     def removeVlan(self, vlan, iface):
         vlandev = iface + '.' + vlan
         ifdown(vlandev)
-        subprocess.call([constants.EXT_IPROUTE, 'link', 'del', vlandev],
-                        stderr=subprocess.PIPE)
+        execCmd([constants.EXT_IPROUTE, 'link', 'del', vlandev])
         self._backup(self.NET_CONF_PREF + iface + '.' + vlan)
         self._removeFile(self.NET_CONF_PREF + iface + '.' + vlan)
 
@@ -510,7 +508,7 @@
 
     def removeBridge(self, bridge):
         ifdown(bridge)
-        subprocess.call([constants.EXT_BRCTL, 'delbr', bridge])
+        execCmd([constants.EXT_BRCTL, 'delbr', bridge])
         self._backup(self.NET_CONF_PREF + bridge)
         self._removeFile(self.NET_CONF_PREF + bridge)
 
@@ -829,7 +827,7 @@
 
     # Validation
     if not utils.tobool(force):
-        logging.debug('validating network...')
+        configNetworkLogger.debug('validating network...')
         _addNetworkValidation(_netinfo, network=network,
                 vlan=vlan, bonding=bonding, nics=nics, ipaddr=ipaddr,
                 netmask=netmask, gateway=gateway, bondingOptions=bondingOptions,
@@ -1172,7 +1170,8 @@
     for bond, bondAttrs in bondings.items():
         if 'remove' in bondAttrs:
             nics = _netinfo.getNicsForBonding(bond)
-            logger.debug("Removing bond %r with nics = %s", bond, nics)
+            logger.debug("Removing bond %r with nics = %s", bond,
+                                      nics)
             ifdown(bond)
             configWriter.removeBonding(bond)
 
@@ -1312,7 +1311,7 @@
 
 def setSafeNetworkConfig():
     """Declare current network configuration as
'safe'"""
-    subprocess.Popen([constants.EXT_VDSM_STORE_NET_CONFIG])
+    execCmd([constants.EXT_VDSM_STORE_NET_CONFIG])
 
 def usage():
     print """Usage:
diff --git a/vdsm/tc.py b/vdsm/tc.py
index 570c410..83593e9 100644
--- a/vdsm/tc.py
+++ b/vdsm/tc.py
@@ -18,14 +18,18 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
+import logging
 from collections import namedtuple
 
-import storage.misc
+import utils
 from vdsm.constants import EXT_TC, EXT_IFCONFIG
 
 ERR_DEV_NOEXIST = 2
 
 QDISC_INGRESS = 'ffff:'
+
+tcLogger = logging.getLogger('trafficControl')
+execCmd = utils.makeExecCmd(log=tcLogger)
 
 
 class TrafficControlException(Exception):
@@ -81,7 +85,7 @@
 
 
 def _process_request(command):
-    retcode, out, err = storage.misc.execCmd(command, raw=True, sudo=False)
+    retcode, out, err = execCmd(command, raw=True, sudo=False)
     if retcode != 0:
         raise TrafficControlException(retcode, err, command)
     return out
diff --git a/vdsm/utils.py b/vdsm/utils.py
index 5e2d4e5..9383171 100644
--- a/vdsm/utils.py
+++ b/vdsm/utils.py
@@ -596,6 +596,22 @@
     else:
         return val
 
+
+def makeExecCmd(log=None):
+    from storage.misc import execCmd
+    if log is None:
+       return execCmd
+    else:
+        def _execCmd(*args, **kwargs):
+            originalLogger = execCmd.func_globals['execCmdLogger']
+            execCmd.func_globals['execCmdLogger'] = log
+            try:
+                return execCmd(*args, **kwargs)
+            finally:
+                execCmd.func_globals['execCmdLogger'] = originalLogger
+        return _execCmd
+
+
 def execCmd(*args, **kwargs):
     # import only after config as been initialized
     from storage.misc import execCmd


--
To view, visit http://gerrit.ovirt.org/7662
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I825431e4a2cbb603fea2b5472b072e5d6a3862e9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com&gt;

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Change in vdsm[master]: Bug-Id: https://bugzilla.redhat.com/851839