From: David Shea dshea@redhat.com
Due to the details of how Linux implements closing file descriptors and EINTR, retrying these calls is considered bad. --- anaconda | 2 +- pyanaconda/anaconda.py | 2 +- pyanaconda/bootloader.py | 10 +++++----- pyanaconda/exception.py | 10 +++++----- pyanaconda/iutil.py | 2 +- pyanaconda/kickstart.py | 2 +- pyanaconda/vnc.py | 4 ++-- tests/gui/outside/__init__.py | 13 +------------ 8 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/anaconda b/anaconda index 6c3fb56..9f5447a 100755 --- a/anaconda +++ b/anaconda @@ -969,7 +969,7 @@ if __name__ == "__main__": pidfile_created = True pid_string = "%s\n" % os.getpid() iutil.eintr_retry_call(os.write, pidfile, pid_string.encode("utf-8")) - iutil.eintr_retry_call(os.close, pidfile) + os.close(pidfile) except OSError as e: # If the failure was anything other than EEXIST during the open call, # just re-raise the exception diff --git a/pyanaconda/anaconda.py b/pyanaconda/anaconda.py index 3a91528..278bffa 100644 --- a/pyanaconda/anaconda.py +++ b/pyanaconda/anaconda.py @@ -202,7 +202,7 @@ def dumpState(self): dump_text += threads dump_text = dump_text.encode("utf-8") iutil.eintr_retry_call(os.write, fd, dump_text) - iutil.eintr_retry_call(os.close, fd) + os.close(fd)
# append to a given file with open("/tmp/anaconda-tb-all.log", "a+") as f: diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py index 1f1efac..f6e36f6 100644 --- a/pyanaconda/bootloader.py +++ b/pyanaconda/bootloader.py @@ -61,7 +61,7 @@ def get_boot_block(device, seek_blocks=0): if seek_blocks: os.lseek(fd, seek_blocks * block_size, 0) block = iutil.eintr_retry_call(os.read, fd, 512) - iutil.eintr_retry_call(os.close, fd) + os.close(fd) if not status: try: device.teardown(recursive=True) @@ -1347,11 +1347,11 @@ def install(self, args=None): "stage2dev": self.grub_device_name(stage2dev)}) (pread, pwrite) = os.pipe() iutil.eintr_retry_call(os.write, pwrite, cmd.encode("utf-8")) - iutil.eintr_retry_call(os.close, pwrite) + os.close(pwrite) args = ["--batch", "--no-floppy", "--device-map=%s" % self.device_map_file] rc = iutil.execInSysroot("grub", args, stdin=pread) - iutil.eintr_retry_call(os.close, pread) + os.close(pread) if rc: raise BootLoaderError("boot loader install failed")
@@ -1540,11 +1540,11 @@ def _encrypt_password(self): (pread, pwrite) = os.pipe() passwords = "%s\n%s\n" % (self.password, self.password) iutil.eintr_retry_call(os.write, pwrite, passwords.encode("utf-8")) - iutil.eintr_retry_call(os.close, pwrite) + os.close(pwrite) buf = iutil.execWithCapture("grub2-mkpasswd-pbkdf2", [], stdin=pread, root=iutil.getSysroot()) - iutil.eintr_retry_call(os.close, pread) + os.close(pread) self.encrypted_password = buf.split()[-1].strip() if not self.encrypted_password.startswith("grub.pbkdf2."): raise BootLoaderError("failed to encrypt boot loader password") diff --git a/pyanaconda/exception.py b/pyanaconda/exception.py index 3bdb1a9..2fa3661 100644 --- a/pyanaconda/exception.py +++ b/pyanaconda/exception.py @@ -196,11 +196,11 @@ def runDebug(self, exc_info): iutil.vtActivate(1)
iutil.eintr_retry_call(os.open, "/dev/console", os.O_RDWR) # reclaim stdin - iutil.eintr_retry_call(os.dup2, 0, 1) # reclaim stdout - iutil.eintr_retry_call(os.dup2, 0, 2) # reclaim stderr - # ^ - # | - # +------ dup2 is magic, I tells ya! + os.dup2(0, 1) # reclaim stdout + os.dup2(0, 2) # reclaim stderr + # ^ + # | + # +------ dup2 is magic, I tells ya!
# bring back the echo import termios diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index e8328c1..1ddf342 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -1288,7 +1288,7 @@ def ipmi_report(event): # Event data 2 & 3 - always 0x0 for us event_string = "0x4 0x1F 0x0 0x6f %#x 0x0 0x0\n" % event eintr_retry_call(os.write, fd, event_string.encode("utf-8")) - eintr_retry_call(os.close, fd) + os.close(fd)
execWithCapture("ipmitool", ["sel", "add", path])
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index a490105..788950d 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -104,7 +104,7 @@ def run(self, chroot): (fd, path) = tempfile.mkstemp("", "ks-script-", scriptRoot + "/tmp")
iutil.eintr_retry_call(os.write, fd, self.script.encode("utf-8")) - iutil.eintr_retry_call(os.close, fd) + os.close(fd) iutil.eintr_retry_call(os.chmod, path, 0o700)
# Always log stdout/stderr from scripts. Using --log just lets you diff --git a/pyanaconda/vnc.py b/pyanaconda/vnc.py index 2905268..fad5435 100644 --- a/pyanaconda/vnc.py +++ b/pyanaconda/vnc.py @@ -87,8 +87,8 @@ def setVNCPassword(self): rc = iutil.execWithRedirect("vncpasswd", ["-f"], stdin=r, stdout=pw_file, binary_output=True, log_output=False)
- iutil.eintr_retry_call(os.close, r) - iutil.eintr_retry_call(os.close, w) + os.close(r) + os.close(w)
return rc
diff --git a/tests/gui/outside/__init__.py b/tests/gui/outside/__init__.py index c563a74..e37ecdd 100644 --- a/tests/gui/outside/__init__.py +++ b/tests/gui/outside/__init__.py @@ -29,17 +29,6 @@ import tempfile import errno
-# Copied from python's subprocess.py -def eintr_retry_call(func, *args): - """Retry an interruptible system call if interrupted.""" - while True: - try: - return func(*args) - except (OSError, IOError) as e: - if e.errno == errno.EINTR: - continue - raise - class Creator(object): """A Creator subclass defines all the parameters for making a VM to run a test against as well as handles creating and running that VM, inspecting @@ -111,7 +100,7 @@ def makeDrives(self): """ for (drive, size) in self.drives: (fd, diskimage) = tempfile.mkstemp(dir=self.tempdir) - eintr_retry_call(os.close, fd) + os.close(fd)
# For now we are using qemu-img to create these files but specifying # sizes in blivet Size objects. Unfortunately, qemu-img wants sizes
From: David Shea dshea@redhat.com
The subprocess module in python3 adds a special DEVNULL file, which simplifies things a little for us. --- anaconda | 5 ++--- dracut/driver-updates | 17 ++++++++--------- scripts/merge-pr | 4 +--- tests/gui/outside/__init__.py | 3 +-- tests/gui/outside/test_reclaim.py | 12 ++++++------ 5 files changed, 18 insertions(+), 23 deletions(-)
diff --git a/anaconda b/anaconda index 9f5447a..efc9c1c 100755 --- a/anaconda +++ b/anaconda @@ -119,14 +119,13 @@ def startSpiceVDAgent(): log.info("Started spice-vdagent.")
def startX11(): - # Ignore stdout/stderr, since everything is logged to /tmp/X.log anyway - xfd = open("/dev/null", "wb") + import subprocess
# Start Xorg and wait for it become ready iutil.startX(["Xorg", "-br", "-logfile", "/tmp/X.log", ":%s" % constants.X_DISPLAY_NUMBER, "vt6", "-s", "1440", "-ac", "-nolisten", "tcp", "-dpi", "96", - "-noreset"], output_redirect=xfd) + "-noreset"], output_redirect=subprocess.DEVNULL)
# function to handle X startup special issues for anaconda def doStartupX11Actions(): diff --git a/dracut/driver-updates b/dracut/driver-updates index b5d8ddb..bdcfa0f 100755 --- a/dracut/driver-updates +++ b/dracut/driver-updates @@ -64,15 +64,14 @@ def run_cmd(cmd): :raises: OSError if the cmd doesn't exist, RunCmdError if the rc != 0 """ try: - with open("/dev/null", "w") as fd_null: - log.debug(" ".join(cmd)) - proc = subprocess.Popen(cmd, universal_newlines=True, - stdout=subprocess.PIPE, - stderr=fd_null) - out = proc.communicate()[0] - if out: - for line in out.splitlines(): - log.debug(line) + log.debug(" ".join(cmd)) + proc = subprocess.Popen(cmd, universal_newlines=True, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL) + out = proc.communicate()[0] + if out: + for line in out.splitlines(): + log.debug(line) except OSError as e: log.error("Error running %s: %s", cmd[0], e.strerror) raise diff --git a/scripts/merge-pr b/scripts/merge-pr index c951aea..a13a780 100755 --- a/scripts/merge-pr +++ b/scripts/merge-pr @@ -59,8 +59,6 @@ from tempfile import NamedTemporaryFile import requests import six
-DEVNULL = open(os.devnull, 'w') - def talk_to_github(request): # Send a requests.Request to github, handle 2-factor auth request.headers.update({'User-Agent': 'merge-pr'}) @@ -183,7 +181,7 @@ def main(): # Try checking out the base of the PR. If it isn't available, do a fetch of # the base repo and try again. try: - subprocess.check_call(['git', 'checkout', '-q', pr['base']['sha']], stderr=DEVNULL) + subprocess.check_call(['git', 'checkout', '-q', pr['base']['sha']], stderr=subprocess.DEVNULL) except subprocess.CalledProcessError: try: subprocess.check_call(['git', 'fetch', '-q', pr['base']['repo']['clone_url'], pr['base']['sha']]) diff --git a/tests/gui/outside/__init__.py b/tests/gui/outside/__init__.py index e37ecdd..0ab332b 100644 --- a/tests/gui/outside/__init__.py +++ b/tests/gui/outside/__init__.py @@ -27,7 +27,6 @@ import shutil import subprocess import tempfile -import errno
class Creator(object): """A Creator subclass defines all the parameters for making a VM to run a @@ -65,7 +64,7 @@ def __init__(self): self._reqMemory = 1536
def _call(self, args): - subprocess.call(args, stdout=open("/dev/null", "w"), stderr=open("/dev/null", "w")) + subprocess.call(args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
def archive(self): """Copy all log files and other test results to a subdirectory of the diff --git a/tests/gui/outside/test_reclaim.py b/tests/gui/outside/test_reclaim.py index c4f4dbf..8f7b7d0 100644 --- a/tests/gui/outside/test_reclaim.py +++ b/tests/gui/outside/test_reclaim.py @@ -46,14 +46,14 @@ def makeDrives(self): # force anaconda to display the reclaim dialog. for (drive, size) in self.drives: subprocess.call(["/sbin/parted", "-s", self._drivePaths[drive], "mklabel", "msdos"], - stdout=open("/dev/null", "w"), - stderr=open("/dev/null", "w")) + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL) subprocess.call(["/sbin/parted", "-s", self._drivePaths[drive], "mkpart", "p", "ext2", "0", str(size)], - stdout=open("/dev/null", "w"), - stderr=open("/dev/null", "w")) + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL) subprocess.call(["/sbin/mkfs.ext4", "-F", self._drivePaths[drive]], - stdout=open("/dev/null", "w"), - stderr=open("/dev/null", "w")) + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL)
class BasicReclaimLiveCD_OutsideTest(OutsideMixin, unittest.TestCase): creatorClass = BasicReclaimLiveCDCreator
From: David Shea dshea@redhat.com
--- pyanaconda/iutil.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index 1ddf342..f06d19e 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -1295,11 +1295,11 @@ def ipmi_report(event): os.remove(path)
# 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
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index 1ddf342..f06d19e 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -1295,11 +1295,11 @@ def ipmi_report(event): os.remove(path)
# 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
This should sadly be duplicated in pocketlint, too.
- Chris
From: David Shea dshea@redhat.com
--- anaconda | 20 +++++++------------- pyanaconda/iutil.py | 11 ++++------- 2 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/anaconda b/anaconda index efc9c1c..a7ea57e 100755 --- a/anaconda +++ b/anaconda @@ -44,7 +44,7 @@ if ("debug=1" in proc_cmdline) or ("debug" in proc_cmdline): cov.start()
-import atexit, sys, os, time, signal, stat, errno +import atexit, sys, os, time, signal, stat
def exitHandler(rebootData, storage): # Clear the list of watched PIDs. @@ -928,13 +928,12 @@ if __name__ == "__main__": try: with open(pidfile_path, 'r') as pidfile: pid = int(pidfile.read()) - except IOError as e: + except FileNotFoundError: # Ignore errors due to the file not existing. Other errors mean (most # likely) that we're not running as root, there's a filesystem error, # or someone filled our PID file with garbage, so just let those be # raised. - if e.errno != errno.ENOENT: - raise + pass else: # Is the PID still running? if not os.path.isdir("/proc/%s" % pid): @@ -951,14 +950,14 @@ if __name__ == "__main__": os.unlink(pidfile_path) # If it is anaconda, let the pidfile creation below fail # and print an error - except IOError as e: + except (FileNotFoundError, ProcessLookupError): # Ignore failures due to the file not existing in case the # process ended while we were trying to read about it. Assume # in this case that the process was another anaconda instance, # and the PID file was cleaned up. # If the process ended between open and read, we'll get ESRCH - if e.errno not in (errno.ENOENT, errno.ESRCH): - raise + # (ProcessLookupError) + pass
# Attempt to create the pidfile try: @@ -969,12 +968,7 @@ if __name__ == "__main__": pid_string = "%s\n" % os.getpid() iutil.eintr_retry_call(os.write, pidfile, pid_string.encode("utf-8")) os.close(pidfile) - except OSError as e: - # If the failure was anything other than EEXIST during the open call, - # just re-raise the exception - if e.errno != errno.EEXIST: - raise - + except FileExistsError: log.error("%s already exists, exiting", pidfile_path)
# If we had a $DISPLAY at start and zenity is available, we may be diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index f06d19e..1aa85d0 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -509,9 +509,8 @@ def _sigchld_handler(num=None, frame=None): for child_pid in _forever_pids: try: pid_result, status = eintr_retry_call(os.waitpid, child_pid, os.WNOHANG) - except OSError as e: - if e.errno == errno.ECHILD: - continue + except ChildProcessError: + continue
if pid_result: proc_name = _forever_pids[child_pid][0] @@ -1300,10 +1299,8 @@ def eintr_retry_call(func, *args, **kwargs): while True: try: return func(*args, **kwargs) - except (OSError, IOError) as e: - if e.errno == errno.EINTR: - continue - raise + except InterruptedError: + continue
def parent_dir(directory): """Return the parent's path"""
From: David Shea dshea@redhat.com
The future is now! --- pyanaconda/ntp.py | 2 -- scripts/analog | 2 -- 2 files changed, 4 deletions(-)
diff --git a/pyanaconda/ntp.py b/pyanaconda/ntp.py index ac710c4..81f3027 100644 --- a/pyanaconda/ntp.py +++ b/pyanaconda/ntp.py @@ -23,8 +23,6 @@
"""
-from __future__ import division - import re import os import tempfile diff --git a/scripts/analog b/scripts/analog index 9bf4564..2f2ebf8 100755 --- a/scripts/analog +++ b/scripts/analog @@ -21,8 +21,6 @@ # Author: Ales Kozumplik akozumpl@redhat.com #
-from __future__ import print_function - import getpass import optparse import os
From: David Shea dshea@redhat.com
Python's builtin open will raise an execption if the underlying open system call returns with EINTR, which means that an ill-timed SIGALRM or SIGCHLD or anything happening from another thread could crash anaconda. --- anaconda | 4 +++- pyanaconda/anaconda.py | 1 + pyanaconda/anaconda_argparse.py | 1 + pyanaconda/anaconda_log.py | 6 ++++++ pyanaconda/bootloader.py | 1 + pyanaconda/desktop.py | 1 + pyanaconda/flags.py | 4 ++++ pyanaconda/image.py | 1 + pyanaconda/install.py | 1 + pyanaconda/isys/__init__.py | 1 + pyanaconda/iutil.py | 11 +++++++++++ pyanaconda/keyboard.py | 1 + pyanaconda/kickstart.py | 1 + pyanaconda/localization.py | 1 + pyanaconda/network.py | 1 + pyanaconda/ntp.py | 1 + pyanaconda/packaging/__init__.py | 1 + pyanaconda/packaging/dnfpayload.py | 1 + pyanaconda/packaging/livepayload.py | 1 + pyanaconda/rescue.py | 1 + pyanaconda/simpleconfig.py | 1 + pyanaconda/timezone.py | 1 + pyanaconda/users.py | 1 + pyanaconda/vnc.py | 1 + scripts/anaconda-cleanup | 1 + scripts/analog | 1 + scripts/instperf | 1 + 27 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/anaconda b/anaconda index a7ea57e..65a3d53 100755 --- a/anaconda +++ b/anaconda @@ -33,7 +33,8 @@
coverage = None
-proc_cmdline = open("/proc/cmdline", "r").read() +# If we get a signal immediately after starting that would be pretty messed up +proc_cmdline = open("/proc/cmdline", "r").read() # pylint: disable=interruptible-system-call proc_cmdline = proc_cmdline.split() if ("debug=1" in proc_cmdline) or ("debug" in proc_cmdline): import coverage @@ -862,6 +863,7 @@ if __name__ == "__main__": from pyanaconda import ntp from pyanaconda import keyboard from pyanaconda.iutil import ProxyString, ProxyStringError + from pyanaconda.iutil import open # pylint: disable=redefined-builtin
verdesc = "%s for %s %s" % (getAnacondaVersionString(), product.productName, product.productVersion) diff --git a/pyanaconda/anaconda.py b/pyanaconda/anaconda.py index 278bffa..89c28b2 100644 --- a/pyanaconda/anaconda.py +++ b/pyanaconda/anaconda.py @@ -36,6 +36,7 @@ from pyanaconda.bootloader import get_bootloader from pyanaconda import constants from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin from pyanaconda import addons
import logging diff --git a/pyanaconda/anaconda_argparse.py b/pyanaconda/anaconda_argparse.py index 03f88e4..af5d9f7 100644 --- a/pyanaconda/anaconda_argparse.py +++ b/pyanaconda/anaconda_argparse.py @@ -33,6 +33,7 @@ from argparse import ArgumentParser, ArgumentError, HelpFormatter, Namespace
from pyanaconda.flags import BootArgs +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
import logging log = logging.getLogger("anaconda") diff --git a/pyanaconda/anaconda_log.py b/pyanaconda/anaconda_log.py index 28bd9a8..4008844 100644 --- a/pyanaconda/anaconda_log.py +++ b/pyanaconda/anaconda_log.py @@ -222,6 +222,9 @@ def updateRemote(self, remote_syslog):
Requires updating rsyslogd config and restarting rsyslog """ + # Import here instead of at the module level to avoid an import loop + from pyanaconda.iutil import open # pylint: disable=redefined-builtin + TEMPLATE = "*.* @@%s\n"
self.remote_syslog = remote_syslog @@ -233,6 +236,9 @@ def updateRemote(self, remote_syslog): def setupVirtio(self): """Setup virtio rsyslog logging. """ + # Import here instead of at the module level to avoid an import loop + from pyanaconda.iutil import open # pylint: disable=redefined-builtin + TEMPLATE = "*.* %s;anaconda_syslog\n"
vport = flags.cmdline.get('virtiolog') or self.VIRTIO_PORT diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py index f6e36f6..97c36cb 100644 --- a/pyanaconda/bootloader.py +++ b/pyanaconda/bootloader.py @@ -31,6 +31,7 @@ from itertools import chain
from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin from blivet.devicelibs import raid from pyanaconda.isys import sync from pyanaconda.product import productName diff --git a/pyanaconda/desktop.py b/pyanaconda/desktop.py index 7428a71..f9eaa92 100644 --- a/pyanaconda/desktop.py +++ b/pyanaconda/desktop.py @@ -23,6 +23,7 @@ import os from pyanaconda.constants import RUNLEVELS from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
import logging log = logging.getLogger("anaconda") diff --git a/pyanaconda/flags.py b/pyanaconda/flags.py index 81319dc..fadb4ef 100644 --- a/pyanaconda/flags.py +++ b/pyanaconda/flags.py @@ -112,6 +112,10 @@ def read(self, filenames): Returns a list of successfully read files. filenames can contain *, ?, and character ranges expressed with [] """ + + # Import here instead of at the module level to avoid an import loop + from pyanaconda.iutil import open # pylint: disable=redefined-builtin + readfiles = [] if isinstance(filenames, str): filenames = [filenames] diff --git a/pyanaconda/image.py b/pyanaconda/image.py index fa3ecd8..ed65101 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -20,6 +20,7 @@ from pyanaconda import isys import os, os.path, stat, tempfile from pyanaconda.constants import ISO_DIR +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
from pyanaconda.errors import errorHandler, ERROR_RAISE, InvalidImageSizeError, MediaMountError, MissingImageError
diff --git a/pyanaconda/install.py b/pyanaconda/install.py index fe0511b..2b6b8aa 100644 --- a/pyanaconda/install.py +++ b/pyanaconda/install.py @@ -28,6 +28,7 @@ from pyanaconda.users import createLuserConf, getPassAlgo, Users from pyanaconda import flags from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin from pyanaconda import timezone from pyanaconda import network from pyanaconda.i18n import _ diff --git a/pyanaconda/isys/__init__.py b/pyanaconda/isys/__init__.py index 0afa61a..a571ca4 100644 --- a/pyanaconda/isys/__init__.py +++ b/pyanaconda/isys/__init__.py @@ -33,6 +33,7 @@ import time import datetime import pytz +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
import logging log = logging.getLogger("anaconda") diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index 1aa85d0..ecd089a 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -1312,3 +1312,14 @@ def requests_session(): session.mount("file://", FileAdapter()) session.mount("ftp://", FTPAdapter()) return session + +_open = open +def open(*args, **kwargs): # pylint: disable=redefined-builtin + """Open a file, and retry on EINTR. + + The arguments are the same as those to python's builtin open. + + This is equivalent to eintr_retry_call(open, ...). Some other + high-level languages handle this for you, like C's fopen. + """ + return eintr_retry_call(_open, *args, **kwargs) diff --git a/pyanaconda/keyboard.py b/pyanaconda/keyboard.py index 0de3111..2c648cc 100644 --- a/pyanaconda/keyboard.py +++ b/pyanaconda/keyboard.py @@ -34,6 +34,7 @@ from pyanaconda import safe_dbus from pyanaconda.constants import DEFAULT_VC_FONT, DEFAULT_KEYBOARD from pyanaconda.flags import can_touch_runtime_system +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
from gi.repository import GLib
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index 788950d..8dcad54 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -39,6 +39,7 @@
import glob from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin import os import os.path import tempfile diff --git a/pyanaconda/localization.py b/pyanaconda/localization.py index 1613f48..480293f 100644 --- a/pyanaconda/localization.py +++ b/pyanaconda/localization.py @@ -30,6 +30,7 @@
from pyanaconda import constants from pyanaconda.iutil import upcase_first_letter, setenv +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
import logging log = logging.getLogger("anaconda") diff --git a/pyanaconda/network.py b/pyanaconda/network.py index 51f29d9..471b582 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -26,6 +26,7 @@
import shutil from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin import socket import os import time diff --git a/pyanaconda/ntp.py b/pyanaconda/ntp.py index 81f3027..701fd8d 100644 --- a/pyanaconda/ntp.py +++ b/pyanaconda/ntp.py @@ -32,6 +32,7 @@
from pyanaconda import isys from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin from pyanaconda.threads import threadMgr, AnacondaThread from pyanaconda.constants import THREAD_SYNC_TIME_BASENAME
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py index 090dc11..fd548cf 100644 --- a/pyanaconda/packaging/__init__.py +++ b/pyanaconda/packaging/__init__.py @@ -38,6 +38,7 @@ import functools
from pyanaconda.iutil import requests_session +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
if __name__ == "__main__": from pyanaconda import anaconda_log diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py index 365b5dd..021fda6 100644 --- a/pyanaconda/packaging/dnfpayload.py +++ b/pyanaconda/packaging/dnfpayload.py @@ -43,6 +43,7 @@ import sys import time from pyanaconda.iutil import ProxyString, ProxyStringError +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
log = logging.getLogger("packaging")
diff --git a/pyanaconda/packaging/livepayload.py b/pyanaconda/packaging/livepayload.py index 8b06381..fcccb7c 100644 --- a/pyanaconda/packaging/livepayload.py +++ b/pyanaconda/packaging/livepayload.py @@ -35,6 +35,7 @@ from threading import Lock import requests from pyanaconda.iutil import ProxyString, ProxyStringError, lowerASCII +from pyanaconda.iutil import open # pylint: disable=redefined-builtin import hashlib import glob import functools diff --git a/pyanaconda/rescue.py b/pyanaconda/rescue.py index 2d0e61e..2dda78d 100644 --- a/pyanaconda/rescue.py +++ b/pyanaconda/rescue.py @@ -22,6 +22,7 @@ import sys import os from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin import shutil import time import re diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py index 371e6ac..4becb09 100644 --- a/pyanaconda/simpleconfig.py +++ b/pyanaconda/simpleconfig.py @@ -28,6 +28,7 @@ import string # pylint: disable=deprecated-module import tempfile from pyanaconda.iutil import upperASCII, eintr_retry_call +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
_SAFECHARS = frozenset(string.ascii_letters + string.digits + '@%_-+=:,./')
diff --git a/pyanaconda/timezone.py b/pyanaconda/timezone.py index 9afbfdb..b153449 100644 --- a/pyanaconda/timezone.py +++ b/pyanaconda/timezone.py @@ -30,6 +30,7 @@ from collections import OrderedDict
from pyanaconda import iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin from pyanaconda.constants import THREAD_STORAGE from pyanaconda.flags import flags from pyanaconda.threads import threadMgr diff --git a/pyanaconda/users.py b/pyanaconda/users.py index cfcba03..ce8e718 100644 --- a/pyanaconda/users.py +++ b/pyanaconda/users.py @@ -30,6 +30,7 @@ from pyanaconda import iutil import pwquality from pyanaconda.iutil import strip_accents +from pyanaconda.iutil import open # pylint: disable=redefined-builtin from pyanaconda.constants import PASSWORD_MIN_LEN from pyanaconda.errors import errorHandler, PasswordCryptError, ERROR_RAISE
diff --git a/pyanaconda/vnc.py b/pyanaconda/vnc.py index fad5435..7f1a7d3 100644 --- a/pyanaconda/vnc.py +++ b/pyanaconda/vnc.py @@ -22,6 +22,7 @@ import os, sys import time from pyanaconda import constants, network, product, iutil +from pyanaconda.iutil import open # pylint: disable=redefined-builtin import socket import subprocess import dbus diff --git a/scripts/anaconda-cleanup b/scripts/anaconda-cleanup index 632cee9..eab5a3d 100755 --- a/scripts/anaconda-cleanup +++ b/scripts/anaconda-cleanup @@ -16,6 +16,7 @@ """ import os import sys +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
live_install = "--liveinst" in sys.argv image_install = False diff --git a/scripts/analog b/scripts/analog index 2f2ebf8..4b498cc 100755 --- a/scripts/analog +++ b/scripts/analog @@ -26,6 +26,7 @@ import optparse import os import os.path import sys +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
DEFAULT_PORT = 6080 DEFAULT_ANALOG_DIR = '.analog' diff --git a/scripts/instperf b/scripts/instperf index b183352..bded964 100755 --- a/scripts/instperf +++ b/scripts/instperf @@ -3,6 +3,7 @@ import logging from subprocess import Popen, PIPE import time +from pyanaconda.iutil import open # pylint: disable=redefined-builtin
# grab the top five memory consuming processes, returning them in a string of # of the format:
From: David Shea dshea@redhat.com
--- pyanaconda/ui/lib/entropy.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/pyanaconda/ui/lib/entropy.py b/pyanaconda/ui/lib/entropy.py index c108480..59cf9da 100644 --- a/pyanaconda/ui/lib/entropy.py +++ b/pyanaconda/ui/lib/entropy.py @@ -33,6 +33,7 @@
from pyanaconda.progress import progress_message from pyanaconda.constants import MAX_ENTROPY_WAIT +from pyanaconda.iutil import eintr_retry_call from pykickstart.constants import DISPLAY_MODE_GRAPHICAL from blivet.util import get_current_entropy
@@ -117,7 +118,7 @@ def _tui_wait(msg, desired_entropy): # and then just read everything from the input buffer and revert the # termios state data = "have something" - while sys.stdin in select.select([sys.stdin], [], [], 0)[0] and data: + while sys.stdin in eintr_retry_call(select.select, [sys.stdin], [], [], 0)[0] and data: # just read from stdin and scratch the read data data = sys.stdin.read(1)
From: David Shea dshea@redhat.com
These will run before pyanaconda is available --- dracut/driver-updates | 9 +++++++++ dracut/parse-kickstart | 10 ++++++++++ 2 files changed, 19 insertions(+)
diff --git a/dracut/driver-updates b/dracut/driver-updates index bdcfa0f..e16fe1d 100755 --- a/dracut/driver-updates +++ b/dracut/driver-updates @@ -47,6 +47,15 @@ import readline # pylint:disable=unused-import
log = logging.getLogger("DD")
+import functools +def eintr_retry_call(func, *args, **kwargs): + """Retry an interruptible system call if interrupted.""" + while True: + try: + return func(*args, **kwargs) + except InterruptedError: + continue +open = functools.partial(eintr_retry_call, open) # pylint: disable=redefined-builtin
class RunCmdError(Exception): """ Raised when run_cmd gets a non-zero returncode diff --git a/dracut/parse-kickstart b/dracut/parse-kickstart index 5612064..fc4d406 100755 --- a/dracut/parse-kickstart +++ b/dracut/parse-kickstart @@ -44,6 +44,16 @@ log = logging.getLogger('parse-kickstart').addHandler(logging.NullHandler())
TMPDIR = "/tmp"
+import functools +def eintr_retry_call(func, *args, **kwargs): + """Retry an interruptible system call if interrupted.""" + while True: + try: + return func(*args, **kwargs) + except InterruptedError: + continue +open = functools.partial(eintr_retry_call, open) # pylint: disable=redefined-builtin + # Helper function for reading simple files in /sys def readsysfile(f): '''Return the contents of f, or "" if missing.'''
From: David Shea dshea@redhat.com
Don't bother handling EINTR in test scripts and single-threaded scripts run outside of the anaconda environment --- docs/conf.py | 3 +++ scripts/githooks/pre-commit | 4 ++++ scripts/makebumpver | 3 +++ scripts/makeupdates | 3 +++ tests/dracut_tests/parse-kickstart_test.py | 5 ++++- tests/gettext/gettext_potfiles.py | 3 +++ tests/glade/accelerators/check_accelerators.py | 3 +++ tests/glade/format_string/check_format_string.py | 3 +++ tests/glade/icons/check_icons.py | 3 +++ tests/glade/markup/check_markup.py | 3 +++ tests/glade/pw_visibility/check_invisible_char.py | 3 +++ tests/glade/pw_visibility/check_pw_visibility.py | 3 +++ tests/glade/viewport/check_viewport.py | 3 +++ tests/gui/outside/__init__.py | 3 +++ tests/lib/translatepo.py | 3 +++ tests/pyanaconda_tests/iutil_test.py | 3 +++ tests/pyanaconda_tests/ks_version_test.py | 4 ++++ tests/pyanaconda_tests/simpleconfig_test.py | 3 +++ 18 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/docs/conf.py b/docs/conf.py index ebc16ef..eb6971c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -11,6 +11,9 @@ # All configuration values have a default; values that are commented out # serve to show the default.
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import sys, os
# If extensions (or modules to document with autodoc) are in another directory, diff --git a/scripts/githooks/pre-commit b/scripts/githooks/pre-commit index 40ae626..cd4b0b6 100755 --- a/scripts/githooks/pre-commit +++ b/scripts/githooks/pre-commit @@ -5,6 +5,10 @@
""" + +# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import os import sys from subprocess import check_output, CalledProcessError diff --git a/scripts/makebumpver b/scripts/makebumpver index 4042742..f17ad29 100755 --- a/scripts/makebumpver +++ b/scripts/makebumpver @@ -20,6 +20,9 @@ # # Author: David Cantrell dcantrell@redhat.com
+# Ignore any interruptible calls +# 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 3fecebb..b288bc0 100755 --- a/scripts/makeupdates +++ b/scripts/makeupdates @@ -21,6 +21,9 @@ # # Author: David Cantrell dcantrell@redhat.com
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import os import shutil import subprocess diff --git a/tests/dracut_tests/parse-kickstart_test.py b/tests/dracut_tests/parse-kickstart_test.py index 5d19206..5aff119 100644 --- a/tests/dracut_tests/parse-kickstart_test.py +++ b/tests/dracut_tests/parse-kickstart_test.py @@ -16,7 +16,10 @@ # with the express permission of Red Hat, Inc. # # Author(s): Brian C. Lane bcl@redhat.com -# + +# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import os import unittest import tempfile diff --git a/tests/gettext/gettext_potfiles.py b/tests/gettext/gettext_potfiles.py index 480abbc..6dd9674 100755 --- a/tests/gettext/gettext_potfiles.py +++ b/tests/gettext/gettext_potfiles.py @@ -1,5 +1,8 @@ #!/usr/bin/python3
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import os, sys, subprocess
# The xpath in this file is simple enough that Python's built-in diff --git a/tests/glade/accelerators/check_accelerators.py b/tests/glade/accelerators/check_accelerators.py index 26b4159..ab87ec1 100755 --- a/tests/glade/accelerators/check_accelerators.py +++ b/tests/glade/accelerators/check_accelerators.py @@ -17,6 +17,9 @@ # # Author: David Shea dshea@redhat.com
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import sys import argparse import re diff --git a/tests/glade/format_string/check_format_string.py b/tests/glade/format_string/check_format_string.py index b8833b5..b730a94 100755 --- a/tests/glade/format_string/check_format_string.py +++ b/tests/glade/format_string/check_format_string.py @@ -29,6 +29,9 @@ strings are allowable within glade. """
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import sys import argparse import re diff --git a/tests/glade/icons/check_icons.py b/tests/glade/icons/check_icons.py index e73c9ad..bfeac6d 100755 --- a/tests/glade/icons/check_icons.py +++ b/tests/glade/icons/check_icons.py @@ -21,6 +21,9 @@ Check that all icons referenced from glade files are valid in the gnome icon theme. """
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import argparse import sys
diff --git a/tests/glade/markup/check_markup.py b/tests/glade/markup/check_markup.py index d437384..ba639da 100755 --- a/tests/glade/markup/check_markup.py +++ b/tests/glade/markup/check_markup.py @@ -23,6 +23,9 @@ valid markup. """
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import sys import argparse
diff --git a/tests/glade/pw_visibility/check_invisible_char.py b/tests/glade/pw_visibility/check_invisible_char.py index 2eaaa96..72d8486 100755 --- a/tests/glade/pw_visibility/check_invisible_char.py +++ b/tests/glade/pw_visibility/check_invisible_char.py @@ -24,6 +24,9 @@ The invisible char is often non-ASCII and sometimes that gets clobbered. """
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import argparse import sys
diff --git a/tests/glade/pw_visibility/check_pw_visibility.py b/tests/glade/pw_visibility/check_pw_visibility.py index 6c5f115..f48f875 100755 --- a/tests/glade/pw_visibility/check_pw_visibility.py +++ b/tests/glade/pw_visibility/check_pw_visibility.py @@ -24,6 +24,9 @@
"""
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import argparse import sys
diff --git a/tests/glade/viewport/check_viewport.py b/tests/glade/viewport/check_viewport.py index 16480d3..bd9d771 100755 --- a/tests/glade/viewport/check_viewport.py +++ b/tests/glade/viewport/check_viewport.py @@ -24,6 +24,9 @@ to add an extra layer. """
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import argparse import sys
diff --git a/tests/gui/outside/__init__.py b/tests/gui/outside/__init__.py index 0ab332b..3eba8d9 100644 --- a/tests/gui/outside/__init__.py +++ b/tests/gui/outside/__init__.py @@ -17,6 +17,9 @@ # # Author: Chris Lumens clumens@redhat.com
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + __all__ = ["Creator", "OutsideMixin"]
from blivet.size import MiB diff --git a/tests/lib/translatepo.py b/tests/lib/translatepo.py index f431618..56b6f01 100644 --- a/tests/lib/translatepo.py +++ b/tests/lib/translatepo.py @@ -22,6 +22,9 @@ # in the po directory, so no gettext compilation or LC_MESSAGES directories are # necessary.
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + import os import locale import re diff --git a/tests/pyanaconda_tests/iutil_test.py b/tests/pyanaconda_tests/iutil_test.py index cee03ab..ad0bc82 100644 --- a/tests/pyanaconda_tests/iutil_test.py +++ b/tests/pyanaconda_tests/iutil_test.py @@ -19,6 +19,9 @@ # Red Hat Author(s): Vratislav Podzimek vpodzime@redhat.com # Martin Kolman mkolman@redhat.com
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + from pyanaconda import iutil import unittest import os diff --git a/tests/pyanaconda_tests/ks_version_test.py b/tests/pyanaconda_tests/ks_version_test.py index 4041d6b..d0b640e 100644 --- a/tests/pyanaconda_tests/ks_version_test.py +++ b/tests/pyanaconda_tests/ks_version_test.py @@ -16,6 +16,10 @@ # along with this program. If not, see http://www.gnu.org/licenses/. # # Author: Chris Lumens clumens@redhat.com + +# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + from mock import Mock import unittest import os diff --git a/tests/pyanaconda_tests/simpleconfig_test.py b/tests/pyanaconda_tests/simpleconfig_test.py index 7da31db..c40b69e 100644 --- a/tests/pyanaconda_tests/simpleconfig_test.py +++ b/tests/pyanaconda_tests/simpleconfig_test.py @@ -18,6 +18,9 @@ # # Red Hat Author(s): Brian C. Lane bcl@redhat.com
+# Ignore any interruptible calls +# pylint: disable=interruptible-system-call + from pyanaconda.simpleconfig import SimpleConfigFile from pyanaconda import simpleconfig import unittest
Added label: ACK.
Holy crap. Ack. Thanks for tracking all these down.
Holy crap. Ack. Thanks for tracking all these down.
Same here, thanks and ACK!
Pushed
Closed.
anaconda-patches@lists.fedorahosted.org