Antoni Segura Puimedon has uploaded a new change for review.

Change subject: 2 of 2 Add network logs for VDSM network commands and file operations. ......................................................................

2 of 2 Add network logs for VDSM network commands and file operations.

Second patch in the series that creates a module-specific logger and uses it with the utility makeExecCmd to increase the detail level of the debugging information provided by the two modules when calling processes and modifying files.

Bug-Id: https://bugzilla.redhat.com/851839 Change-Id: Iea3738f5ce66d2537c078c452def30fb3feb5390 Signed-off-by: Antoni S. Puimedon asegurap@redhat.com --- M vdsm/configNetwork.py M vdsm/tc.py 2 files changed, 38 insertions(+), 30 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/7730/1

diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py index 74d19c7..ceb846d 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 @@ -32,6 +32,7 @@

from vdsm import constants from vdsm import utils +from storage.misc import enableLogSkip import neterrors as ne from vdsm import define from vdsm import netinfo @@ -42,6 +43,11 @@ MAX_BRIDGE_NAME_LEN = 15 ILLEGAL_BRIDGE_CHARS = frozenset(':. \t')

+configNetworkLogger = enableLogSkip( + logging.getLogger('configNetwork'), ignoreSourceFiles=[__file__], + logSkipName='configNetwork') +execCmd = utils.makeExecCmd(log=configNetworkLogger) + class ConfigNetworkError(Exception): def __init__(self, errCode, message): self.errCode = errCode @@ -50,25 +56,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 +160,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 +295,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 +305,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 +315,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 +355,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 +376,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 +407,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 +501,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 +511,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 +830,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, @@ -1125,8 +1126,8 @@ _netinfo = netinfo.NetInfo()

for bond, bondAttrs in bondings.iteritems(): - logger.debug("Creating/Editing bond %s with attributes %s", - bond, bondAttrs) + logger.debug("Creating/Editing bond %s with attributes %s", bond, + bondAttrs)

brNets = list(_netinfo.getBridgedNetworksForIface(bond)) # Only one bridged-non-VLANed network allowed on same nic/bond @@ -1315,7 +1316,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 a0a612f..44c5bb1 100644 --- a/vdsm/tc.py +++ b/vdsm/tc.py @@ -18,16 +18,23 @@ # Refer to the README and COPYING files for full details of the license #

+import logging from collections import namedtuple

import ethtool

-import storage.misc +import utils +from storage.misc import enableLogSkip from vdsm.constants import EXT_TC, EXT_IFCONFIG

ERR_DEV_NOEXIST = 2

QDISC_INGRESS = 'ffff:' + +tcLogger = enableLogSkip( + logging.getLogger('trafficControl'), ignoreSourceFiles=[__file__], + logSkipName='trafficControl') +execCmd = utils.makeExecCmd(log=tcLogger)

class TrafficControlException(Exception): @@ -95,7 +102,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

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

Gerrit-MessageType: newchange Gerrit-Change-Id: Iea3738f5ce66d2537c078c452def30fb3feb5390 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon asegurap@redhat.com