Change in vdsm[master]: fileUtils: Add atomic_symlink utility

danken at redhat.com danken at redhat.com
Tue Mar 1 20:06:52 UTC 2016


Dan Kenigsberg has posted comments on this change.

Change subject: fileUtils: Add atomic_symlink utility
......................................................................


Patch Set 1:

(3 comments)

Minor comments, your call if to fix now.

https://gerrit.ovirt.org/#/c/53680/1/vdsm/storage/fileUtils.py
File vdsm/storage/fileUtils.py:

Line 201:     else:
Line 202:         return open(path, mode)
Line 203: 
Line 204: 
Line 205: def atomic_symlink(target, name):
a docstring explaining that this is a crash-resistent kind of atomicity, it is not intended to be a reentrant function
Line 206:     log.info("Linking %r to %r", target, name)
Line 207:     try:
Line 208:         current_target = os.readlink(name)
Line 209:     except OSError as e:


Line 220:         os.symlink(target, tmp_name)
Line 221:     except OSError as e:
Line 222:         if e.errno != errno.EEXIST:
Line 223:             raise
Line 224:         log.debug("Removing stale temporary link %r", tmp_name)
Unhealthy if the directory already held an unrelated .tmp file with the same prefix.

It is not a real worry in our use case, but a general-purpose utility would be better off creating a magic+random subdir to hold the temp link.
Line 225:         os.unlink(tmp_name)
Line 226:         os.symlink(target, tmp_name)
Line 227: 
Line 228:     try:


Line 230:     except:
Line 231:         try:
Line 232:             os.unlink(tmp_name)
Line 233:         except OSError as e:
Line 234:             log.error("Cannot remove temporary link %r: %s", tmp_name, e)
Nit: the most recently-caught exception is raised, which may happen to be the one from unlink.
Line 235:         raise
Line 236: 
Line 237: 
Line 238: class DirectFile(object):


-- 
To view, visit https://gerrit.ovirt.org/53680
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I931eca7a25c7b10344fce78b274d633d73eb082d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stirabos at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list