Change in vdsm[master]: Ifcfg: Fix file writing races

asegurap at redhat.com asegurap at redhat.com
Wed Oct 9 23:27:11 UTC 2013


Antoni Segura Puimedon has uploaded a new change for review.

Change subject: Ifcfg: Fix file writing races
......................................................................

Ifcfg: Fix file writing races

On a fast enough computer, it could very well happen that we'd do:
    open('foo', 'w').write('moo')
    execCmd(['cat', 'foo'])

And that the content that 'cat' would show for foo would not be 'moo'
but whatever was there before the write, due to the file closing
happening at a non deterministic point of time.

How was this horribly edgy case unearthed? Good question!

Let's say we have a bond0 with slaves em2 and em1. We are in the
process of removing the bond completely.

removeNic basically stripes the nic ifcfg file of everything except
the hwaddr, device type, etc. So, if there was previously for nic em1
MASTER=bond0

after removeNic, there is an ifup of em1 (to leave the link state up)
that would still see bond0 as master. That in turn would make
initscripts trigger ifup of bond0, but that ifcfg file no longer
exists, ifup fails, raises an exception, and the whole removal is
rolled back.

Needless to say, this was unfindable with the debugger, as the file
close would be fast enough due to the debugger slowing down operation.

Change-Id: I6aabfbba44f1250f305ef6ec06f715e4782c4d1b
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
---
M vdsm/netconf/ifcfg.py
1 file changed, 14 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/50/20050/1

diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py
index d2032a7..b278183 100644
--- a/vdsm/netconf/ifcfg.py
+++ b/vdsm/netconf/ifcfg.py
@@ -267,7 +267,8 @@
             os.makedirs(dirName)
         os.chown(dirName, vdsm_uid, 0)
 
-        open(backup, 'w').write(content)
+        with open(backup, 'w') as backupFile:
+            backupFile.write(content)
         os.chown(backup, vdsm_uid, 0)
         logging.debug("Persistently backed up %s "
                       "(until next 'set safe config')", backup)
@@ -337,14 +338,15 @@
 
     def restoreAtomicBackup(self):
         logging.info("Rolling back configuration (restoring atomic backup)")
-        for confFile, content in self._backups.iteritems():
+        for confFilePath, content in self._backups.iteritems():
             if content is None:
-                utils.rmFile(confFile)
+                utils.rmFile(confFilePath)
                 logging.debug('Removing empty configuration backup %s',
-                              confFile)
+                              confFilePath)
             else:
-                open(confFile, 'w').write(content)
-            logging.info('Restored %s', confFile)
+                with open(confFilePath, 'w') as confFile:
+                    confFile.write(content)
+            logging.info('Restored %s', confFilePath)
 
     def _devType(self, content):
         if re.search('^TYPE=Bridge$', content, re.MULTILINE):
@@ -479,7 +481,8 @@
         self._backup(fileName)
         logging.debug('Writing to file %s configuration:\n%s' % (fileName,
                       configuration))
-        open(fileName, 'w').write(configuration)
+        with open(fileName, 'w') as confFile:
+            confFile.write(configuration)
         os.chmod(fileName, 0664)
         try:
             selinux.restorecon(fileName)
@@ -559,7 +562,8 @@
 
         # create the bonding device to avoid initscripts noise
         if bond.name not in open(netinfo.BONDING_MASTERS).read().split():
-            open(netinfo.BONDING_MASTERS, 'w').write('+%s\n' % bond.name)
+            with open(netinfo.BONDING_MASTERS, 'w') as bondingMasters:
+                bondingMasters.write('+%s\n' % bond.name)
 
     def addNic(self, nic, **opts):
         """ Create ifcfg-* file with proper fields for NIC """
@@ -606,7 +610,8 @@
                        if line.startswith('HWADDR=')]
             l = ['DEVICE=%s\n' % nic, 'ONBOOT=yes\n',
                  'MTU=%s\n' % netinfo.DEFAULT_MTU] + hwlines
-            open(cf, 'w').writelines(l)
+            with open(cf, 'w') as nicFile:
+                nicFile.writelines(l)
         except IOError:
             pass
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6aabfbba44f1250f305ef6ec06f715e4782c4d1b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>


More information about the vdsm-patches mailing list