Yaniv Bronhaim has posted comments on this change. Change subject: osutils: Start the osutils module ......................................................................
Patch Set 1: (3 comments) https://gerrit.ovirt.org/#/c/61142/1/lib/vdsm/osutils.py File lib/vdsm/osutils.py: Line 37: http://lwn.net/Articles/576478/ Line 38: """ Line 39: try: Line 40: os.close(fd) Line 41: except EnvironmentError as e: why not to move NoIntrCall here, and close_fd will use that? Line 42: if e.errno != errno.EINTR: https://gerrit.ovirt.org/#/c/61142/1/tests/osutils_test.py File tests/osutils_test.py: Line 33: for fd in fds: Line 34: osutils.close_fd(fd) Line 35: for fd in fds: Line 36: path = "/proc/self/fd/%d" % fd Line 37: self.assertFalse(os.path.exists(path)) this is racy.. the fd can exist here even if it was closed alright Line 38: Line 39: def test_propagate_other_errors(self): Line 40: with self.assertRaises(OSError) as e: Line 41: osutils.close_fd(-1) https://gerrit.ovirt.org/#/c/61142/1/vdsm.spec.in File vdsm.spec.in: Line 1216: %{python_sitelib}/%{vdsm_name}/network/tc/qdisc.py* Line 1217: %{python_sitelib}/%{vdsm_name}/network/utils.py* Line 1218: %{python_sitelib}/%{vdsm_name}/numa.py* Line 1219: %{python_sitelib}/%{vdsm_name}/osinfo.py* Line 1220: %{python_sitelib}/%{vdsm_name}/osutils.py* I think this module should be placed under vdsm/infra , that way all common utils will be gathered under this package. Line 1221: %{python_sitelib}/%{vdsm_name}/password.py* Line 1222: %{python_sitelib}/%{vdsm_name}/panic.py* Line 1223: %{python_sitelib}/%{vdsm_name}/ppc64HardwareInfo.py* Line 1224: %{python_sitelib}/%{vdsm_name}/profiling/__init__.py* -- To view, visit https://gerrit.ovirt.org/61142 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia995c083f31c3489ced16265d459f30355f854b0 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: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@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
