Nir Soffer has posted comments on this change. Change subject: clusterlock: Fix double close on EINTR ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/60539/1/lib/vdsm/storage/clusterlock.py File lib/vdsm/storage/clusterlock.py: Line 463: t, v, tb = sys.exec_info() Line 464: try: Line 465: os.close(lockFile) Line 466: except OSError as ce: Line 467: if ce.errno != errno.EINTR: > Perhaps a comment on what we expect to happen in this case? We're swallowi The commit message explain about this - do you think we need to document this for each close() call? Line 468: raise Line 469: if e.errno in (errno.EACCES, errno.EAGAIN): Line 470: raise se.AcquireLockFailure( Line 471: self._sdUUID, e.errno, "Cannot acquire local lock", Line 496: try: Line 497: os.close(lockFile) Line 498: except OSError as e: Line 499: if e.errno != errno.EINTR: Line 500: raise > Do we need a helper for this close and ignore -EINTR logic? generic helper for closing fds? Line 501: self._globalLockMap[self._sdUUID] = (hostId, None) Line 502: Line 503: self.log.debug("Local lock for domain %s successfully released", -- To view, visit https://gerrit.ovirt.org/60539 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cae714af9646e268f40ff1ff3cdb3f6b048e6e9 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: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@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/admin/lists/vdsm-patches@lists.fedorahosted.org
