Nir Soffer has uploaded a new change for review.
Change subject: osutils: Start the osutils module
......................................................................
osutils: Start the osutils module
We use to have code calling os.close() using utils.NoIntrCall. If
os.close(fd) was interrupted by signal, we would try to close again.
This may lead to closing other thread file descriptor.
close(2) warns not to retry close() after EINTR:
Note that the return value should be used only for diagnostics. In
particular close() should not be retried after an EINTR since this
may cause a reused descriptor from another thread to be closed.
See also this discussion about close and EINTR on linux:
http://lwn.net/Articles/576478/
The same issue was fixed in cpopen few month ago.
This patch adds osutils.close_fd() - this should be used instead of
os.close() in vdsm.
This should also be the place for os related utilities like NoIntrCall
and friends, to be added later.
Change-Id: Ia995c083f31c3489ced16265d459f30355f854b0
Signed-off-by: Nir Soffer <[email protected]>
---
A lib/vdsm/osutils.py
M tests/Makefile.am
A tests/osutils_test.py
M vdsm.spec.in
4 files changed, 87 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/42/61142/1
diff --git a/lib/vdsm/osutils.py b/lib/vdsm/osutils.py
new file mode 100644
index 0000000..086c8e2
--- /dev/null
+++ b/lib/vdsm/osutils.py
@@ -0,0 +1,43 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from __future__ import absolute_import
+import errno
+import os
+
+
+def close_fd(fd):
+ """
+ Close fd once, ignoring EINTR.
+
+ close(2) warns not to retry close() after EINTR:
+
+ Note that the return value should be used only for diagnostics. In
+ particular close() should not be retried after an EINTR since this
+ may cause a reused descriptor from another thread to be closed.
+
+ See also this discussion about close and EINTR on linux:
+ http://lwn.net/Articles/576478/
+ """
+ try:
+ os.close(fd)
+ except EnvironmentError as e:
+ if e.errno != errno.EINTR:
+ raise
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 467f2d4..0b30dc0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -90,6 +90,7 @@
numaUtilsTests.py \
outOfProcessTests.py \
osinfo_test.py \
+ osutils_test.py \
passwordsTests.py \
periodicTests.py \
permutationTests.py \
diff --git a/tests/osutils_test.py b/tests/osutils_test.py
new file mode 100644
index 0000000..6142e5e
--- /dev/null
+++ b/tests/osutils_test.py
@@ -0,0 +1,42 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from __future__ import absolute_import
+import errno
+import os
+
+from testlib import VdsmTestCase
+from vdsm import osutils
+
+
+class TestCloseFd(VdsmTestCase):
+
+ def test_close(self):
+ fds = os.pipe()
+ for fd in fds:
+ osutils.close_fd(fd)
+ for fd in fds:
+ path = "/proc/self/fd/%d" % fd
+ self.assertFalse(os.path.exists(path))
+
+ def test_propagate_other_errors(self):
+ with self.assertRaises(OSError) as e:
+ osutils.close_fd(-1)
+ self.assertEqual(e.exception.errno, errno.EBADF)
diff --git a/vdsm.spec.in b/vdsm.spec.in
index d1f91cf..f4602eb 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1217,6 +1217,7 @@
%{python_sitelib}/%{vdsm_name}/network/utils.py*
%{python_sitelib}/%{vdsm_name}/numa.py*
%{python_sitelib}/%{vdsm_name}/osinfo.py*
+%{python_sitelib}/%{vdsm_name}/osutils.py*
%{python_sitelib}/%{vdsm_name}/password.py*
%{python_sitelib}/%{vdsm_name}/panic.py*
%{python_sitelib}/%{vdsm_name}/ppc64HardwareInfo.py*
--
To view, visit https://gerrit.ovirt.org/61142
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia995c083f31c3489ced16265d459f30355f854b0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]
