Nir Soffer has posted comments on this change. Change subject: fileUtils: Add atomic_symlink utility ......................................................................
Patch Set 1: (3 comments) 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 Will add. This is atomic because you may have a link to the target, or not, due to the atomicity of os.rename. 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 sam So do you think we should use ."tmp.random_value"? When should be find and delete such files if we crash in the middle? Maybe during startup? 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 t How about: try: os.rename(tmp_name, name) finally: try: os.unlink(tmp_name) except OSError as e: if e.errno != errno.ENOENT: log.error(...) This should always raise the original error 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 <nsof...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
