From patchwork Fri Jul 15 14:21:26 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Boca X-Patchwork-Id: 648847 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 3rrZZ80fWGz9s8d for ; Sat, 16 Jul 2016 00:24:00 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 6043B22C3AE; Fri, 15 Jul 2016 07:23:54 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id 529AD22C39E for ; Fri, 15 Jul 2016 07:23:52 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id DFA0D1626A5 for ; Fri, 15 Jul 2016 08:23:51 -0600 (MDT) X-ASG-Debug-ID: 1468592627-0b32373da60b850001-byXFYA Received: from mx3-pf3.cudamail.com ([192.168.14.3]) by bar6.cudamail.com with ESMTP id Ez6q40zR1aPmyNPz (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 15 Jul 2016 08:23:47 -0600 (MDT) X-Barracuda-Envelope-From: pboca@cloudbasesolutions.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.3 Received: from unknown (HELO cbssmtp1.cloudbase.local) (91.232.152.5) by mx3-pf3.cudamail.com with SMTP; 15 Jul 2016 14:23:46 -0000 Received-SPF: pass (mx3-pf3.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 70471405A1 for ; Fri, 15 Jul 2016 17:23:45 +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 V4Ic23hUVzIU for ; Fri, 15 Jul 2016 17:23:24 +0300 (EEST) Received: from CBSEX1.cloudbase.local (unknown [10.77.78.3]) by cbssmtp1.cloudbase.local (Postfix) with ESMTP id 22F3740551 for ; Fri, 15 Jul 2016 17:21:27 +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, 15 Jul 2016 16:21:26 +0200 X-CudaMail-Envelope-Sender: pboca@cloudbasesolutions.com From: Paul Boca To: "dev@openvswitch.org" X-CudaMail-MID: CM-V3-714014579 X-CudaMail-DTE: 071516 X-CudaMail-Originating-IP: 91.232.152.5 Thread-Topic: [PATCH V7 12/16] python tests: Ported Python daemon to Windows X-ASG-Orig-Subj: [##CM-V3-714014579##][PATCH V7 12/16] python tests: Ported Python daemon to Windows Thread-Index: AQHR3qQrcV/x1MTF9Ema4OXcs4HG8Q== Date: Fri, 15 Jul 2016 14:21:26 +0000 Message-ID: <1468592469-10160-13-git-send-email-pboca@cloudbasesolutions.com> References: <1468592469-10160-1-git-send-email-pboca@cloudbasesolutions.com> In-Reply-To: <1468592469-10160-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.38014 p=-0.45 Source Normal X-MessageSniffer-Rules: 0-0-0-29422-c X-Barracuda-Connect: UNKNOWN[192.168.14.3] X-Barracuda-Start-Time: 1468592627 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.31284 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 V7 12/16] 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 V6: Explicitly close the vlog file in detached daemon. On Windows, even if the daemon is detached, the primary daemon is still holding a handle to the log file, therefore the log cannot be moved/deleted even is the vlog/close command is sent to the detached daemon. V7: Fixed flake8 errors and apply requested changes --- python/ovs/daemon.py | 241 +++++++++++++++++++++++++++++++++++++-------- python/ovs/fatal_signal.py | 13 +++ python/ovs/vlog.py | 12 +++ tests/test-daemon.py | 4 +- 4 files changed, 228 insertions(+), 42 deletions(-) diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py index bd06195..9492b00 100644 --- a/python/ovs/daemon.py +++ b/python/ovs/daemon.py @@ -13,9 +13,7 @@ # limitations under the License. import errno -import fcntl import os -import resource import signal import sys import time @@ -28,6 +26,20 @@ import ovs.timeval import ovs.util import ovs.vlog +if sys.platform == "win32": + import ovs.fcntl_win as fcntl + import threading + import win32file + import win32pipe + import win32api + import win32security + import win32con + import pywintypes + import subprocess +else: + import fcntl + import resource + vlog = ovs.vlog.Vlog("daemon") # --detach: Should we run in the background? @@ -53,6 +65,10 @@ _monitor = False # File descriptor used by daemonize_start() and daemonize_complete(). _daemonize_fd = None +if sys.platform == "win32": + # Running as the child process - Windows only. + _detached = False + RESTART_EXIT_CODE = 5 @@ -110,6 +126,16 @@ def set_monitor(): _monitor = True +if sys.platform == "win32": + 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) @@ -123,8 +149,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 +177,48 @@ 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 @@ -205,6 +243,105 @@ def _waitpid(pid, options): 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 + 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 + + ''' close the log file, on Windows cannot be moved while the parent has + a reference on it.''' + vlog.close_log_file() + + try: + (rfd, wfd) = _windows_create_pipe() + except pywintypes.error 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 pywintypes.error 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: rfd, wfd = os.pipe() @@ -250,7 +387,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() @@ -259,7 +396,14 @@ def _fork_and_wait_for_startup(): return pid -def _fork_notify_startup(fd): +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): if fd is not None: error, bytes_written = ovs.socket_util.write_fully(fd, "0") if error: @@ -268,6 +412,13 @@ def _fork_notify_startup(fd): 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 +447,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 +470,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 +483,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 +501,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 +657,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 +668,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 73e4be6..dfc446e 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/python/ovs/vlog.py b/python/ovs/vlog.py index 48d52ad..2768ce7 100644 --- a/python/ovs/vlog.py +++ b/python/ovs/vlog.py @@ -384,6 +384,17 @@ class Vlog(object): logger.addHandler(Vlog.__file_handler) @staticmethod + def close_log_file(): + """Closes the current log file. (This is useful on Windows, to ensure + that a reference to the file is not kept by the daemon in case of + detach.)""" + + if Vlog.__log_file: + logger = logging.getLogger("file") + logger.removeHandler(Vlog.__file_handler) + Vlog.__file_handler.close() + + @staticmethod def _unixctl_vlog_reopen(conn, unused_argv, unused_aux): if Vlog.__log_file: Vlog.reopen_log_file() @@ -396,6 +407,7 @@ class Vlog(object): if Vlog.__log_file: logger = logging.getLogger("file") logger.removeHandler(Vlog.__file_handler) + Vlog.__file_handler.close() conn.reply(None) @staticmethod 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.")