From patchwork Tue Jan 3 20:10:52 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alin Balutoiu X-Patchwork-Id: 710624 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ttQ9N0cggz9s9Y for ; Wed, 4 Jan 2017 07:12:56 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 29E40A67; Tue, 3 Jan 2017 20:11:44 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 2B2DA904 for ; Tue, 3 Jan 2017 20:11:43 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail.cloudbasesolutions.com (mail.cloudbasesolutions.com [91.232.152.5]) by smtp1.linuxfoundation.org (Postfix) with ESMTP id 72D341A6 for ; Tue, 3 Jan 2017 20:11:41 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by mail.cloudbasesolutions.com (Postfix) with ESMTP id 9955A419BF for ; Tue, 3 Jan 2017 22:11:40 +0200 (EET) X-Virus-Scanned: amavisd-new at cloudbasesolutions.com Received: from mail.cloudbasesolutions.com ([127.0.0.1]) by localhost (mail.cloudbasesolutions.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yjkUlF_4NeBI for ; Tue, 3 Jan 2017 22:11:19 +0200 (EET) Received: from mail.cloudbasesolutions.com (unknown [10.77.78.3]) by mail.cloudbasesolutions.com (Postfix) with ESMTP id 5E6F0419D9 for ; Tue, 3 Jan 2017 22:10:53 +0200 (EET) Received: from CBSEX1.cloudbase.local ([10.77.78.3]) by CBSEX1.cloudbase.local ([10.77.78.3]) with mapi id 14.03.0319.002; Tue, 3 Jan 2017 21:10:53 +0100 From: Alin Balutoiu To: "dev@openvswitch.org" Thread-Topic: [PATCH V4 3/5] Python tests: Daemon ported to Windows Thread-Index: AQHSZf17T+kV2KTcJkyQQfjazkwj6A== Date: Tue, 3 Jan 2017 20:10:52 +0000 Message-ID: <1483474197-8702-4-git-send-email-abalutoiu@cloudbasesolutions.com> References: <1483474197-8702-1-git-send-email-abalutoiu@cloudbasesolutions.com> In-Reply-To: <1483474197-8702-1-git-send-email-abalutoiu@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-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Alin Balutoiu Subject: [ovs-dev] [PATCH V4 3/5] Python tests: Daemon ported to Windows X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: Alin Balutoiu Instead of using os.fork (not supported on Windows), subprocess.Popen is used and os.pipe was replaced with Windows pipes. To be able to identify the child process, an extra parameter was added to daemon process '--pipe-handle'. This parameter contains the parent Windows pipe handle which is used by the child to notify the parent about the startup. The PID file is created directly on Windows, without using a temporary file because the symbolic link does not inherit the file lok set on the temporary file. Signed-off-by: Paul-Daniel Boca Signed-off-by: Alin-Gheorghe Balutoiu Acked-by: Alin Gabriel Serdean Tested-by: Alin Gabriel Serdean --- V2: No changes. V3: Changed Signed-off-by name and added previous Acked-by's, Tested-by's. V4: No changes. --- python/ovs/daemon.py | 194 +++++++++++++++++++++++++++++++++++++-------- python/ovs/fatal_signal.py | 13 +++ python/ovs/vlog.py | 26 +++++- tests/test-daemon.py | 4 +- 4 files changed, 199 insertions(+), 38 deletions(-) diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py index bd06195..06ef92b 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,11 +26,24 @@ import ovs.timeval import ovs.util import ovs.vlog +if sys.platform != 'win32': + import fcntl + import resource +else: + import ovs.winutils as winutils + import ovs.fcntl_win as fcntl + import pywintypes + import subprocess + import win32process + vlog = ovs.vlog.Vlog("daemon") # --detach: Should we run in the background? _detach = False +# Running as the child process - Windows only. +_detached = False + # --pidfile: Name of pidfile (null if none). _pidfile = None @@ -98,6 +109,16 @@ def set_detach(): _detach = 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. Used on Windows only.""" + global _detached + global _daemonize_fd + _detached = True + _daemonize_fd = int(wp) + + def get_detach(): """Will daemonize() really detach?""" return _detach @@ -123,8 +144,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 = "%s.tmp%d" % (_pidfile, pid) + ovs.fatal_signal.add_file_to_unlink(tmpfile) + else: + tmpfile = "%s" % _pidfile + try: # This is global to keep Python from garbage-collecting and # therefore closing our file after this function exits. That would @@ -147,40 +172,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)) + if sys.platform == 'win32': + # Ensure that the pidfile will gets closed and deleted on exit. + ovs.fatal_signal.add_file_to_close_and_unlink(_pidfile, file_handle) else: - while True: + # 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))) + _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) + # 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))) + # 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))) global _pidfile_dev global _pidfile_ino @@ -206,6 +239,9 @@ def _waitpid(pid, options): def _fork_and_wait_for_startup(): + if sys.platform == 'win32': + return _fork_and_wait_for_startup_windows() + try: rfd, wfd = os.pipe() except OSError as e: @@ -259,7 +295,65 @@ def _fork_and_wait_for_startup(): return pid +def _fork_and_wait_for_startup_windows(): + global _detached + if _detached: + # Running in child process + ovs.timeval.postfork() + 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) = winutils.windows_create_pipe() + except pywintypes.error as e: + sys.stderr.write("pipe failed to create: %s\n" % e.strerror) + sys.exit(1) + + try: + creationFlags = win32process.DETACHED_PROCESS + args = ("%s %s --pipe-handle=%ld" % ( + sys.executable, " ".join(sys.argv), int(wfd))) + proc = subprocess.Popen( + args=args, + close_fds=False, + shell=False, + creationflags=creationFlags, + stdout=sys.stdout, + stderr=sys.stderr) + pid = proc.pid + except OSError as e: + sys.stderr.write("CreateProcess failed (%s)\n" % os.strerror(e.errno)) + sys.exit(1) + + # Running in parent process. + winutils.win32file.CloseHandle(wfd) + ovs.fatal_signal.fork() + + error, s = winutils.windows_read_pipe(rfd, 1) + if error: + s = "" + + if len(s) != 1: + retval = proc.wait() + if retval == 0: + sys.stderr.write("fork child failed to signal startup\n") + else: + # Child exited with an error. Convey the same error to + # our parent process as a courtesy. + sys.exit(retval) + winutils.win32file.CloseHandle(rfd) + + return pid + + def _fork_notify_startup(fd): + if sys.platform == 'win32': + _fork_notify_startup_windows(fd) + return + if fd is not None: error, bytes_written = ovs.socket_util.write_fully(fd, "0") if error: @@ -268,9 +362,31 @@ def _fork_notify_startup(fd): os.close(fd) +def _fork_notify_startup_windows(fd): + if fd is not None: + try: + # Python 2 requires a string as second parameter, while + # Python 3 requires a bytes-like object. b"0" fits for both + # python versions. + winutils.win32file.WriteFile(fd, b"0", None) + except winutils.pywintypes.error as e: + sys.stderr.write("could not write to pipe: %s\n" % + os.strerror(e.winerror)) + sys.exit(1) + + def _should_restart(status): global RESTART_EXIT_CODE + if sys.platform == 'win32': + # The exit status is encoded in the high byte of the + # 16-bit number 'status'. + exit_status = status >> 8 + + if exit_status == RESTART_EXIT_CODE: + return True + return False + if os.WIFEXITED(status) and os.WEXITSTATUS(status) == RESTART_EXIT_CODE: return True @@ -296,7 +412,7 @@ def _monitor_daemon(daemon_pid): % (daemon_pid, ovs.process.status_msg(status))) if _should_restart(status): - if os.WCOREDUMP(status): + if sys.platform != 'win32' and os.WCOREDUMP(status): # Disable further core dumps to save disk space. try: resource.setrlimit(resource.RLIMIT_CORE, (0, 0)) @@ -351,8 +467,9 @@ def daemonize_start(): # Running in parent process. sys.exit(0) - # Running in daemon or monitor process. - os.setsid() + if sys.platform != 'win32': + # Running in daemon or monitor process. + os.setsid() if _monitor: saved_daemonize_fd = _daemonize_fd @@ -360,7 +477,8 @@ def daemonize_start(): if daemon_pid > 0: # Running in monitor process. _fork_notify_startup(saved_daemonize_fd) - _close_standard_fds() + if sys.platform != 'win32': + _close_standard_fds() _monitor_daemon(daemon_pid) # Running in daemon process @@ -502,6 +620,10 @@ 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 +632,10 @@ def handle_args(args): parent ArgumentParser should have been prepared by add_args() before calling parse_args().""" + if sys.platform == 'win32': + if 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 a143e24..eb4b663 100644 --- a/python/ovs/fatal_signal.py +++ b/python/ovs/fatal_signal.py @@ -59,6 +59,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.""" @@ -78,6 +89,8 @@ def unlink_file_now(file): def _unlink_files(): for file_ in _files: + if sys.platform == "win32" and _files[file_]: + _files[file_].close() _unlink(file_) diff --git a/python/ovs/vlog.py b/python/ovs/vlog.py index 48d52ad..99bd36b 100644 --- a/python/ovs/vlog.py +++ b/python/ovs/vlog.py @@ -243,7 +243,14 @@ class Vlog(object): Vlog._unixctl_vlog_reopen, None) ovs.unixctl.command_register("vlog/close", "", 0, 0, Vlog._unixctl_vlog_close, None) - ovs.unixctl.command_register("vlog/set", "spec", 1, sys.maxsize, + try: + # Windows limitation on Python 2, sys.maxsize is a long number + # on 64 bits environments, while sys.maxint is the maximum int + # number. Python 3 works as expected. + maxsize_int = sys.maxint + except AttributeError: + maxsize_int = sys.maxsize + ovs.unixctl.command_register("vlog/set", "spec", 1, maxsize_int, Vlog._unixctl_vlog_set, None) ovs.unixctl.command_register("vlog/list", "", 0, 0, Vlog._unixctl_vlog_list, None) @@ -384,6 +391,16 @@ 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() @@ -394,8 +411,11 @@ class Vlog(object): @staticmethod def _unixctl_vlog_close(conn, unused_argv, unused_aux): if Vlog.__log_file: - logger = logging.getLogger("file") - logger.removeHandler(Vlog.__file_handler) + if sys.platform != 'win32': + logger = logging.getLogger("file") + logger.removeHandler(Vlog.__file_handler) + else: + Vlog.close_log_file() conn.reply(None) @staticmethod diff --git a/tests/test-daemon.py b/tests/test-daemon.py index 63c1f70..3f4200d 100644 --- a/tests/test-daemon.py +++ b/tests/test-daemon.py @@ -27,7 +27,9 @@ def handler(signum, _): def main(): - signal.signal(signal.SIGHUP, handler) + if sys.platform != 'win32': + # signal.SIGHUP does not exist on Windows + signal.signal(signal.SIGHUP, handler) parser = argparse.ArgumentParser( description="Open vSwitch daemonization test program for Python.")