From patchwork Fri Jul 1 16:27:03 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Boca X-Patchwork-Id: 643146 X-Patchwork-Delegate: guru@ovn.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3rh2H76kYZz9sCY for ; Sat, 2 Jul 2016 02:41:23 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 3458B22C39C; Fri, 1 Jul 2016 09:41:23 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 870D322C397 for ; Fri, 1 Jul 2016 09:41:21 -0700 (PDT) Received: from bar5.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id 120C74200E0 for ; Fri, 1 Jul 2016 10:41:21 -0600 (MDT) X-ASG-Debug-ID: 1467391280-09eadd3a0604300001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar5.cudamail.com with ESMTP id 7ri04iiJx1E10Zy3 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 01 Jul 2016 10:41:20 -0600 (MDT) X-Barracuda-Envelope-From: pboca@cloudbasesolutions.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO cbssmtp1.cloudbase.local) (91.232.152.5) by mx1-pf2.cudamail.com with SMTP; 1 Jul 2016 16:41:19 -0000 Received-SPF: pass (mx1-pf2.cudamail.com: SPF record at cloudbasesolutions.com designates 91.232.152.5 as permitted sender) X-Barracuda-Apparent-Source-IP: 91.232.152.5 X-Barracuda-RBL-IP: 91.232.152.5 Received: from localhost (localhost [127.0.0.1]) by cbssmtp1.cloudbase.local (Postfix) with ESMTP id BE03F4182F for ; Fri, 1 Jul 2016 19:41:18 +0300 (EEST) X-Virus-Scanned: amavisd-new at cloudbasesolutions.com Received: from cbssmtp1.cloudbase.local ([127.0.0.1]) by localhost (cbssmtp1.cloudbase.local [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PRJxM2JUreqV for ; Fri, 1 Jul 2016 19:40:57 +0300 (EEST) Received: from CBSEX1.cloudbase.local (unknown [10.77.78.3]) by cbssmtp1.cloudbase.local (Postfix) with ESMTP id E45E641850 for ; Fri, 1 Jul 2016 19:27:03 +0300 (EEST) Received: from CBSEX1.cloudbase.local ([10.77.78.3]) by CBSEX1.cloudbase.local ([10.77.78.3]) with mapi id 14.03.0301.000; Fri, 1 Jul 2016 18:27:03 +0200 X-CudaMail-Envelope-Sender: pboca@cloudbasesolutions.com From: Paul Boca To: "dev@openvswitch.org" X-CudaMail-MID: CM-E2-630037593 X-CudaMail-DTE: 070116 X-CudaMail-Originating-IP: 91.232.152.5 Thread-Topic: [PATCH V5 12/17] python tests: Ported Python daemon to Windows X-ASG-Orig-Subj: [##CM-E2-630037593##][PATCH V5 12/17] python tests: Ported Python daemon to Windows Thread-Index: AQHR07Vmk1hoP6QPg0+FPm+Frj5rrg== Date: Fri, 1 Jul 2016 16:27:03 +0000 Message-ID: <1467390322-12852-13-git-send-email-pboca@cloudbasesolutions.com> References: <1467390322-12852-1-git-send-email-pboca@cloudbasesolutions.com> In-Reply-To: <1467390322-12852-1-git-send-email-pboca@cloudbasesolutions.com> Accept-Language: en-US, it-IT Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.77.78.1] MIME-Version: 1.0 X-GBUdb-Analysis: 0, 91.232.152.5, Ugly c=0.372106 p=-0.351351 Source Normal X-MessageSniffer-Rules: 0-0-0-26764-c X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1467391280 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.10 X-Barracuda-Spam-Status: No, SCORE=0.10 using global scores of TAG_LEVEL=3.5 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=4.0 tests=RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.30926 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS Subject: [ovs-dev] [PATCH V5 12/17] python tests: Ported Python daemon to Windows X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" Used subprocess.Popen instead os.fork (not implemented on windows) and repaced of os.pipe with Windows pipes. To be able to identify the child process I added an extra parameter to daemon process '--pipe-handle', this parameter also contains the parent Windows pipe handle, used by the child to signal the start. The PID file is created directly on Windows, without using a temporary file because the symbolic link doesn't inheriths the file lock set on temporary file. Signed-off-by: Paul-Daniel Boca --- V2: Fix lockf on Linux, small error on os.link and missing pipe_handle parameter. V3: Import modules at the start of the code V4: Close file before trying to delete it in signal hooks. On Windows the PID file cannot be deleted while it's handle is opened for write. V5: No changes --- python/ovs/daemon.py | 218 ++++++++++++++++++++++++++++++++++++--------- python/ovs/fatal_signal.py | 13 +++ tests/test-daemon.py | 4 +- 3 files changed, 192 insertions(+), 43 deletions(-) diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py index bd06195..d327629 100644 --- a/python/ovs/daemon.py +++ b/python/ovs/daemon.py @@ -13,12 +13,11 @@ # limitations under the License. import errno -import fcntl import os -import resource import signal import sys import time +import threading import ovs.dirs import ovs.fatal_signal @@ -28,6 +27,15 @@ import ovs.timeval import ovs.util import ovs.vlog +if sys.platform == "win32": + import ovs.fcntl_win as fcntl + import win32file, win32pipe, win32api, win32security, win32con, pywintypes + import msvcrt + import subprocess +else: + import fcntl + import resource + vlog = ovs.vlog.Vlog("daemon") # --detach: Should we run in the background? @@ -53,6 +61,9 @@ _monitor = False # File descriptor used by daemonize_start() and daemonize_complete(). _daemonize_fd = None +# Running as the child process - Windows only. +_detached = False + RESTART_EXIT_CODE = 5 @@ -109,13 +120,20 @@ def set_monitor(): global _monitor _monitor = True +def set_detached(wp): + """Sets up a following call to daemonize() to fork a supervisory process to + monitor the daemon and restart it if it dies due to an error signal.""" + global _detached + global _daemonize_fd + _detached = True + _daemonize_fd = int(wp) + def _fatal(msg): vlog.err(msg) sys.stderr.write("%s\n" % msg) sys.exit(1) - def _make_pidfile(): """If a pidfile has been configured, creates it and stores the running process's pid in it. Ensures that the pidfile will be deleted when the @@ -123,8 +141,12 @@ def _make_pidfile(): pid = os.getpid() # Create a temporary pidfile. - tmpfile = "%s.tmp%d" % (_pidfile, pid) - ovs.fatal_signal.add_file_to_unlink(tmpfile) + if sys.platform == "win32": + tmpfile = _pidfile + else: + tmpfile = "%s.tmp%d" % (_pidfile, pid) + ovs.fatal_signal.add_file_to_unlink(tmpfile) + try: # This is global to keep Python from garbage-collecting and # therefore closing our file after this function exits. That would @@ -147,40 +169,47 @@ def _make_pidfile(): _fatal("%s: write failed: %s" % (tmpfile, e.strerror)) try: - fcntl.lockf(file_handle, fcntl.LOCK_EX | fcntl.LOCK_NB) + if sys.platform != "win32": + fcntl.lockf(file_handle, fcntl.LOCK_EX | fcntl.LOCK_NB) + else: + fcntl.lockf(file_handle, fcntl.LOCK_SH | fcntl.LOCK_NB) except IOError as e: _fatal("%s: fcntl failed: %s" % (tmpfile, e.strerror)) - # Rename or link it to the correct name. - if _overwrite_pidfile: - try: - os.rename(tmpfile, _pidfile) - except OSError as e: - _fatal("failed to rename \"%s\" to \"%s\" (%s)" - % (tmpfile, _pidfile, e.strerror)) - else: - while True: + if sys.platform != "win32": + # Rename or link it to the correct name. + if _overwrite_pidfile: try: - os.link(tmpfile, _pidfile) - error = 0 + os.rename(tmpfile, _pidfile) except OSError as e: - error = e.errno - if error == errno.EEXIST: - _check_already_running() - elif error != errno.EINTR: - break - if error: - _fatal("failed to link \"%s\" as \"%s\" (%s)" - % (tmpfile, _pidfile, os.strerror(error))) - - # Ensure that the pidfile will get deleted on exit. - ovs.fatal_signal.add_file_to_unlink(_pidfile) - - # Delete the temporary pidfile if it still exists. - if not _overwrite_pidfile: - error = ovs.fatal_signal.unlink_file_now(tmpfile) - if error: - _fatal("%s: unlink failed (%s)" % (tmpfile, os.strerror(error))) + _fatal("failed to rename \"%s\" to \"%s\" (%s)" + % (tmpfile, _pidfile, e.strerror)) + else: + while True: + try: + os.link(tmpfile, _pidfile) + error = 0 + except OSError as e: + error = e.errno + if error == errno.EEXIST: + _check_already_running() + elif error != errno.EINTR: + break + if error: + _fatal("failed to link \"%s\" as \"%s\" (%s)" + % (tmpfile, _pidfile, os.strerror(error))) + + # Ensure that the pidfile will get deleted on exit. + ovs.fatal_signal.add_file_to_unlink(_pidfile) + + # Delete the temporary pidfile if it still exists. + if not _overwrite_pidfile: + error = ovs.fatal_signal.unlink_file_now(tmpfile) + if error: + _fatal("%s: unlink failed (%s)" % (tmpfile, os.strerror(error))) + else: + # Ensure that the pidfile will gets closed and deleted on exit. + ovs.fatal_signal.add_file_to_close_and_unlink(_pidfile, file_handle) global _pidfile_dev global _pidfile_ino @@ -204,6 +233,93 @@ def _waitpid(pid, options): pass return -e.errno, 0 +def _windows_create_pipe(): + sAttrs = win32security.SECURITY_ATTRIBUTES() + sAttrs.bInheritHandle = 1 + + (read_pipe, write_pipe) = win32pipe.CreatePipe(sAttrs, 0) + pid = win32api.GetCurrentProcess() + write_pipe2 = win32api.DuplicateHandle(pid, write_pipe, pid, 0, 1, win32con.DUPLICATE_SAME_ACCESS) + win32file.CloseHandle(write_pipe) + + return (read_pipe, write_pipe2) + +def __windows_fork_notify_startup(fd): + if fd is not None: + try: + win32file.WriteFile(fd, "0", None) + except: + win32file.WriteFile(fd, bytes("0", 'UTF-8'), None) + +def _windows_read_pipe(fd): + if fd is not None: + sAttrs = win32security.SECURITY_ATTRIBUTES() + sAttrs.bInheritHandle = 1 + overlapped = pywintypes.OVERLAPPED() + try: + (ret, data) = win32file.ReadFile(fd, 1, None) + return data + except pywintypes.error as e: + raise OSError(errno.EIO, "", "") + +def _windows_detach(proc, wfd): + """ If the child process closes and it was detached + then close the communication pipe so the parent process + can terminate """ + proc.wait() + win32file.CloseHandle(wfd) + +def _windows_fork_and_wait_for_startup(): + if _detached: + return 0 + + try: + (rfd, wfd) = _windows_create_pipe() + except OSError as e: + sys.stderr.write("pipe failed: %s\n" % os.strerror(e.errno)) + sys.exit(1) + + try: + proc = subprocess.Popen("%s %s --pipe-handle=%ld" % (sys.executable, " ".join(sys.argv), int(wfd)), close_fds=False, shell=False) + pid = proc.pid + #start a thread and wait the subprocess exit code + thread = threading.Thread(target=_windows_detach, args=(proc, wfd)) + thread.daemon = True + thread.start() + except OSError as e: + sys.stderr.write("could not fork: %s\n" % os.strerror(e.errno)) + sys.exit(1) + + if pid > 0: + # Running in parent process. + ovs.fatal_signal.fork() + while True: + try: + s = _windows_read_pipe(rfd) + error = 0 + except OSError as e: + s = "" + error = e.errno + if error != errno.EINTR: + break + if len(s) != 1: + retval, status = _waitpid(pid, 0) + if retval == pid: + if os.WIFEXITED(status) and os.WEXITSTATUS(status): + # Child exited with an error. Convey the same error to + # our parent process as a courtesy. + sys.exit(os.WEXITSTATUS(status)) + else: + sys.stderr.write("fork child failed to signal " + "startup (%s)\n" + % ovs.process.status_msg(status)) + else: + assert retval < 0 + sys.stderr.write("waitpid failed (%s)\n" + % os.strerror(-retval)) + sys.exit(1) + + return pid def _fork_and_wait_for_startup(): try: @@ -250,7 +366,7 @@ def _fork_and_wait_for_startup(): os.close(rfd) else: - # Running in parent process. + # Running in child process. os.close(rfd) ovs.timeval.postfork() @@ -258,8 +374,13 @@ def _fork_and_wait_for_startup(): _daemonize_fd = wfd return pid +def fork_and_wait_for_startup(): + if sys.platform == "win32": + return _windows_fork_and_wait_for_startup() + else: + return _fork_and_wait_for_startup() -def _fork_notify_startup(fd): +def __fork_notify_startup(fd): if fd is not None: error, bytes_written = ovs.socket_util.write_fully(fd, "0") if error: @@ -267,6 +388,11 @@ def _fork_notify_startup(fd): sys.exit(1) os.close(fd) +def _fork_notify_startup(fd): + if sys.platform == "win32": + return __windows_fork_notify_startup(fd) + else: + return __fork_notify_startup(fd) def _should_restart(status): global RESTART_EXIT_CODE @@ -296,7 +422,7 @@ def _monitor_daemon(daemon_pid): % (daemon_pid, ovs.process.status_msg(status))) if _should_restart(status): - if os.WCOREDUMP(status): + if os.WCOREDUMP(status) and sys.platform != "win32": # Disable further core dumps to save disk space. try: resource.setrlimit(resource.RLIMIT_CORE, (0, 0)) @@ -319,7 +445,7 @@ def _monitor_daemon(daemon_pid): last_restart = ovs.timeval.msec() vlog.err("%s, restarting" % status_msg) - daemon_pid = _fork_and_wait_for_startup() + daemon_pid = fork_and_wait_for_startup() if not daemon_pid: break else: @@ -332,6 +458,9 @@ def _monitor_daemon(daemon_pid): def _close_standard_fds(): """Close stdin, stdout, stderr. If we're started from e.g. an SSH session, then this keeps us from holding that session open artificially.""" + if sys.platform == "win32": + return + null_fd = ovs.socket_util.get_null_fd() if null_fd >= 0: os.dup2(null_fd, 0) @@ -347,16 +476,17 @@ def daemonize_start(): nonzero exit code).""" if _detach: - if _fork_and_wait_for_startup() > 0: + if fork_and_wait_for_startup() > 0: # Running in parent process. sys.exit(0) # Running in daemon or monitor process. - os.setsid() + if sys.platform != "win32": + os.setsid() if _monitor: saved_daemonize_fd = _daemonize_fd - daemon_pid = _fork_and_wait_for_startup() + daemon_pid = fork_and_wait_for_startup() if daemon_pid > 0: # Running in monitor process. _fork_notify_startup(saved_daemonize_fd) @@ -502,6 +632,9 @@ def add_args(parser): help="Create pidfile (default %s)." % pidfile) group.add_argument("--overwrite-pidfile", action="store_true", help="With --pidfile, start even if already running.") + if sys.platform == "win32": + group.add_argument("--pipe-handle", + help="With --pidfile, start even if already running.") def handle_args(args): @@ -510,6 +643,9 @@ def handle_args(args): parent ArgumentParser should have been prepared by add_args() before calling parse_args().""" + if sys.platform == "win32" and args.pipe_handle: + set_detached(args.pipe_handle) + if args.detach: set_detach() diff --git a/python/ovs/fatal_signal.py b/python/ovs/fatal_signal.py index 5b90559..c16d704 100644 --- a/python/ovs/fatal_signal.py +++ b/python/ovs/fatal_signal.py @@ -58,6 +58,17 @@ def add_file_to_unlink(file): _files[file] = None +def add_file_to_close_and_unlink(file, fd = None): + """Registers 'file' to be unlinked when the program terminates via + sys.exit() or a fatal signal and the 'fd' to be closed. On Windows a file + cannot be removed while it is open for writing.""" + global _added_hook + if not _added_hook: + _added_hook = True + add_hook(_unlink_files, _cancel_files, True) + _files[file] = fd + + def remove_file_to_unlink(file): """Unregisters 'file' from being unlinked when the program terminates via sys.exit() or a fatal signal.""" @@ -77,6 +88,8 @@ def unlink_file_now(file): def _unlink_files(): for file_ in _files: + if _files[file_]: + _files[file_].close() _unlink(file_) diff --git a/tests/test-daemon.py b/tests/test-daemon.py index 63c1f70..a3b5751 100644 --- a/tests/test-daemon.py +++ b/tests/test-daemon.py @@ -26,8 +26,8 @@ def handler(signum, _): def main(): - - signal.signal(signal.SIGHUP, handler) + if sys.platform != "win32": + signal.signal(signal.SIGHUP, handler) parser = argparse.ArgumentParser( description="Open vSwitch daemonization test program for Python.")