Retry some function calls on EINTR, ignore others
From: David Shea dshea@redhat.com
--- blivet/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/blivet/util.py b/blivet/util.py index cf329f9..b39308b 100644 --- a/blivet/util.py +++ b/blivet/util.py @@ -628,11 +628,11 @@ def power_of_two(value): return True
# Copied from python's subprocess.py -def eintr_retry_call(func, *args): +def eintr_retry_call(func, *args, **kwargs): """Retry an interruptible system call if interrupted.""" while True: try: - return func(*args) + return func(*args, **kwargs) except (OSError, IOError) as e: if e.errno == errno.EINTR: continue
From: David Shea dshea@redhat.com
Retrying close in Linux is considered a bad idea, it turns out. --- blivet/devicelibs/edd.py | 2 +- blivet/devices/file.py | 2 +- blivet/devices/optical.py | 2 +- blivet/fcoe.py | 2 +- blivet/iscsi.py | 4 ++-- blivet/util.py | 18 ++++++++++++++++-- 6 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/blivet/devicelibs/edd.py b/blivet/devicelibs/edd.py index 08a713f..7c99bb1 100644 --- a/blivet/devicelibs/edd.py +++ b/blivet/devicelibs/edd.py @@ -165,7 +165,7 @@ def collect_mbrs(devices): # The signature is the unsigned integer at byte 440: os.lseek(fd, 440, 0) mbrsig = struct.unpack('I', util.eintr_retry_call(os.read, fd, 4)) - util.eintr_retry_call(os.close, fd) + util.eintr_ignore(os.close, fd) except OSError as e: log.warning("edd: error reading mbrsig from disk %s: %s", dev.name, str(e)) diff --git a/blivet/devices/file.py b/blivet/devices/file.py index f104568..a2845eb 100644 --- a/blivet/devices/file.py +++ b/blivet/devices/file.py @@ -119,7 +119,7 @@ def _create(self): size_target = zero * rem util.eintr_retry_call(os.write, fd, size_target.encode("utf-8"))
- util.eintr_retry_call(os.close, fd) + util.eintr_ignore(os.close, fd)
def _destroy(self): """ Destroy the device. """ diff --git a/blivet/devices/optical.py b/blivet/devices/optical.py index 3f8fcd2..591fd01 100644 --- a/blivet/devices/optical.py +++ b/blivet/devices/optical.py @@ -63,7 +63,7 @@ def mediaPresent(self): else: return True else: - util.eintr_retry_call(os.close, fd) + util.eintr_ignore(os.close, fd) return True
def eject(self): diff --git a/blivet/fcoe.py b/blivet/fcoe.py index 1723207..a7d7a4c 100644 --- a/blivet/fcoe.py +++ b/blivet/fcoe.py @@ -165,7 +165,7 @@ def write(self, root): else: config += 'AUTO_VLAN="no"\n' util.eintr_retry_call(os.write, fd, config.encode('utf-8')) - util.eintr_retry_call(os.close, fd) + util.eintr_ignore(os.close, fd)
return
diff --git a/blivet/iscsi.py b/blivet/iscsi.py index 160a243..5aa3d4b 100644 --- a/blivet/iscsi.py +++ b/blivet/iscsi.py @@ -272,7 +272,7 @@ def startup(self): fd = util.eintr_retry_call(os.open, INITIATOR_FILE, os.O_RDWR | os.O_CREAT) initiator_name = "InitiatorName=%s\n" % self.initiator util.eintr_retry_call(os.write, fd, initiator_name.encode("utf-8")) - util.eintr_retry_call(os.close, fd) + util.eintr_ignore(os.close, fd) self.initiatorSet = True
for fulldir in (os.path.join("/var/lib/iscsi", d) for d in \ @@ -493,7 +493,7 @@ def write(self, root, storage): fd = util.eintr_retry_call(os.open, root + INITIATOR_FILE, os.O_RDWR | os.O_CREAT) initiator_name = "InitiatorName=%s\n" % self.initiator util.eintr_retry_call(os.write, fd, initiator_name.encode("utf-8")) - util.eintr_retry_call(os.close, fd) + util.eintr_ignore(os.close, fd)
# copy "db" files. *sigh* if os.path.isdir(root + "/var/lib/iscsi"): diff --git a/blivet/util.py b/blivet/util.py index b39308b..30db569 100644 --- a/blivet/util.py +++ b/blivet/util.py @@ -528,7 +528,7 @@ def create_sparse_tempfile(name, size): :returns: the path to the newly created file """ (fd, path) = tempfile.mkstemp(prefix="blivet.", suffix="-%s" % name) - eintr_retry_call(os.close, fd) + eintr_ignore(os.close, fd) create_sparse_file(path, size) return path
@@ -541,7 +541,7 @@ def create_sparse_file(path, size): """ fd = eintr_retry_call(os.open, path, os.O_WRONLY|os.O_CREAT|os.O_TRUNC) eintr_retry_call(os.ftruncate, fd, size) - eintr_retry_call(os.close, fd) + eintr_ignore(os.close, fd)
@contextmanager def sparsetmpfile(name, size): @@ -637,3 +637,17 @@ def eintr_retry_call(func, *args, **kwargs): if e.errno == errno.EINTR: continue raise + +def eintr_ignore(func, *args, **kwargs): + """Call a function and ignore EINTR. + + This is useful for calls to close() and dup2(), which can return EINTR + but which should *not* be retried, since by the time the return the + file descriptor is already closed. + """ + try: + return func(*args, **kwargs) + except (OSError, IOError) as e: + if e.errno == errno.EINTR: + pass + raise
In reply to line 645 of blivet/util.py:
Should be: "by the time they return..."
In reply to line 645 of blivet/util.py:
Should be: "by the time they return..."
Fixed locally, along with the anaconda commit I pasted this into
From: David Shea dshea@redhat.com
Redefine open to a wrapper that retries open, and wrap a call to os.mknod. --- blivet/arch.py | 1 + blivet/blivet.py | 1 + blivet/devicelibs/edd.py | 1 + blivet/devices/disk.py | 1 + blivet/devices/lib.py | 1 + blivet/devices/md.py | 1 + blivet/devices/storage.py | 1 + blivet/devicetree.py | 1 + blivet/fcoe.py | 1 + blivet/flags.py | 2 ++ blivet/formats/fslib.py | 2 ++ blivet/mounts.py | 1 + blivet/osinstall.py | 3 ++- blivet/populator.py | 1 + blivet/udev.py | 1 + blivet/util.py | 5 +++++ blivet/zfcp.py | 1 + 17 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/blivet/arch.py b/blivet/arch.py index ca40832..7759963 100644 --- a/blivet/arch.py +++ b/blivet/arch.py @@ -35,6 +35,7 @@
from .flags import flags from .storage_log import log_exception_info +from .util import open # pylint: disable=redefined-builtin
import logging log = logging.getLogger("blivet") diff --git a/blivet/blivet.py b/blivet/blivet.py index 306a479..cc9607f 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -53,6 +53,7 @@ from . import zfcp from . import devicefactory from . import get_bootloader, getSysroot, shortProductName, __version__ +from .util import open # pylint: disable=redefined-builtin
from .i18n import _
diff --git a/blivet/devicelibs/edd.py b/blivet/devicelibs/edd.py index 7c99bb1..71fb271 100644 --- a/blivet/devicelibs/edd.py +++ b/blivet/devicelibs/edd.py @@ -28,6 +28,7 @@ import struct
from .. import util +from ..util import open # pylint: disable=redefined-builtin
log = logging.getLogger("blivet")
diff --git a/blivet/devices/disk.py b/blivet/devices/disk.py index e2b9dee..7b3bfdd 100644 --- a/blivet/devices/disk.py +++ b/blivet/devices/disk.py @@ -30,6 +30,7 @@ from .. import udev from ..size import Size from ..tasks import availability +from ..util import open # pylint: disable=redefined-builtin
from ..fcoe import fcoe
diff --git a/blivet/devices/lib.py b/blivet/devices/lib.py index 14e9501..5faada5 100644 --- a/blivet/devices/lib.py +++ b/blivet/devices/lib.py @@ -23,6 +23,7 @@ from .. import errors from .. import udev from ..size import Size +from ..util import open # pylint: disable=redefined-builtin
LINUX_SECTOR_SIZE = Size(512)
diff --git a/blivet/devices/md.py b/blivet/devices/md.py index 513d2dd..3291e71 100644 --- a/blivet/devices/md.py +++ b/blivet/devices/md.py @@ -33,6 +33,7 @@ from .. import udev from ..size import Size from ..tasks import availability +from ..util import open # pylint: disable=redefined-builtin
import logging log = logging.getLogger("blivet") diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py index e63c7f9..525db63 100644 --- a/blivet/devices/storage.py +++ b/blivet/devices/storage.py @@ -31,6 +31,7 @@ from .. import udev from ..formats import getFormat from ..size import Size +from ..util import open # pylint: disable=redefined-builtin
import logging log = logging.getLogger("blivet") diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 3349569..6392b65 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -35,6 +35,7 @@ from .devicelibs import edd from . import udev from . import util +from .util import open # pylint: disable=redefined-builtin from .flags import flags from .populator import Populator from .storage_log import log_method_call, log_method_return diff --git a/blivet/fcoe.py b/blivet/fcoe.py index a7d7a4c..861e412 100644 --- a/blivet/fcoe.py +++ b/blivet/fcoe.py @@ -20,6 +20,7 @@ import os from . import udev from . import util +from .util import open # pylint: disable=redefined-builtin import logging import time from .i18n import _ diff --git a/blivet/flags.py b/blivet/flags.py index 0ff1c96..7c0319c 100644 --- a/blivet/flags.py +++ b/blivet/flags.py @@ -22,6 +22,8 @@ import shlex import selinux
+from .util import open # pylint: disable=redefined-builtin + class Flags(object): def __init__(self): # diff --git a/blivet/formats/fslib.py b/blivet/formats/fslib.py index 618c15b..a0e419d 100644 --- a/blivet/formats/fslib.py +++ b/blivet/formats/fslib.py @@ -21,6 +21,8 @@ # David Cantrell dcantrell@redhat.com # Anne Mulhern amulhern@redhat.com
+from ..util import open # pylint: disable=redefined-builtin + kernel_filesystems = [] nodev_filesystems = []
diff --git a/blivet/mounts.py b/blivet/mounts.py index a7763cf..8e32220 100644 --- a/blivet/mounts.py +++ b/blivet/mounts.py @@ -21,6 +21,7 @@ # from collections import defaultdict from . import util +from .util import open # pylint: disable=redefined-builtin from .devicelibs import btrfs
import logging diff --git a/blivet/osinstall.py b/blivet/osinstall.py index 1c63302..7608bf6 100644 --- a/blivet/osinstall.py +++ b/blivet/osinstall.py @@ -28,6 +28,7 @@
from . import util from . import getSysroot, getTargetPhysicalRoot, errorHandler, ERROR_RAISE +from .util import open # pylint: disable=redefined-builtin
from .storage_log import log_exception_info from .devices import FileDevice, NFSDevice, NoDevice, OpticalDevice, NetworkStorageDevice, DirectoryDevice @@ -644,7 +645,7 @@ def mkDevRoot(self): dev = "%s/%s" % (getSysroot(), root.path) if not os.path.exists("%s/dev/root" %(getSysroot(),)) and os.path.exists(dev): rdev = os.stat(dev).st_rdev - os.mknod("%s/dev/root" % (getSysroot(),), stat.S_IFBLK | 0o600, rdev) + util.eintr_retry_call(os.mknod, "%s/dev/root" % (getSysroot(),), stat.S_IFBLK | 0o600, rdev)
@property def swapDevices(self): diff --git a/blivet/populator.py b/blivet/populator.py index d86fa39..3ff6f8e 100644 --- a/blivet/populator.py +++ b/blivet/populator.py @@ -47,6 +47,7 @@ from .devicelibs import raid from . import udev from . import util +from .util import open # pylint: disable=redefined-builtin from .flags import flags from .storage_log import log_exception_info, log_method_call from .i18n import _ diff --git a/blivet/udev.py b/blivet/udev.py index 99a7bc3..1c36cbc 100644 --- a/blivet/udev.py +++ b/blivet/udev.py @@ -25,6 +25,7 @@ import re
from . import util +from .util import open # pylint: disable=redefined-builtin from .size import Size from .flags import flags
diff --git a/blivet/util.py b/blivet/util.py index 30db569..069a266 100644 --- a/blivet/util.py +++ b/blivet/util.py @@ -651,3 +651,8 @@ def eintr_ignore(func, *args, **kwargs): if e.errno == errno.EINTR: pass raise + +_open = open +def open(*args, **kwargs): # pylint: disable=redefined-builtin + """Open a file, and retry on EINTR.""" + return eintr_retry_call(_open, *args, **kwargs) diff --git a/blivet/zfcp.py b/blivet/zfcp.py index 74620bd..b1a1317 100644 --- a/blivet/zfcp.py +++ b/blivet/zfcp.py @@ -22,6 +22,7 @@ import os from . import udev from . import util +from .util import open # pylint: disable=redefined-builtin from .i18n import _ from .util import stringize, unicodeize from gi.repository import BlockDev as blockdev
From: David Shea dshea@redhat.com
None of these scripts are likely to be interrupted by a signal. --- scripts/makebumpver | 2 ++ scripts/makeupdates | 2 ++ scripts/merge-pr | 2 ++ setup.py | 1 + 4 files changed, 7 insertions(+)
diff --git a/scripts/makebumpver b/scripts/makebumpver index 01ff2a8..550d8ce 100755 --- a/scripts/makebumpver +++ b/scripts/makebumpver @@ -20,6 +20,8 @@ # # Author: David Cantrell dcantrell@redhat.com
+# pylint: disable=interruptible-system-call + import logging logging.basicConfig(level=logging.DEBUG) log = logging.getLogger("bugzilla") diff --git a/scripts/makeupdates b/scripts/makeupdates index e3e6a0c..30bff14 100755 --- a/scripts/makeupdates +++ b/scripts/makeupdates @@ -20,6 +20,8 @@ # # Author: David Cantrell dcantrell@redhat.com
+# pylint: disable=interruptible-system-call + import getopt import os import shutil diff --git a/scripts/merge-pr b/scripts/merge-pr index 7b13939..eef796d 100755 --- a/scripts/merge-pr +++ b/scripts/merge-pr @@ -47,6 +47,8 @@ # request against f22-branch but you don't have f22-branch locally, it won't # work.
+# pylint: disable=interruptible-system-call + import argparse import subprocess import sys diff --git a/setup.py b/setup.py index c696535..cd40e80 100644 --- a/setup.py +++ b/setup.py @@ -1,4 +1,5 @@ #!/usr/bin/python2 +# pylint: disable=interruptible-system-call
from distutils.core import setup import subprocess
Looks good to me.
Added label: ACK.
Closed.
Pushed
anaconda-patches@lists.fedorahosted.org
-
dashea