Please ignore this patch, I will send another one soon.
Thanks,
Alin Balutoiu.
> -----Original Message-----
> From: Alin Balutoiu
> Sent: Tuesday, January 3, 2017 5:17 PM
> To: d...@openvswitch.org
> Cc: Alin Balutoiu <abalut...@cloudbasesolutions.com>; Paul Boca
> <pb...@cloudbasesolutions.com>
> Subject: [PATCH V2 3/5] Python tests: Daemon ported to Windows
>
> From: Alin Balutoiu <abalut...@cloudbasesolutions.com>
>
> 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 <pb...@cloudbasesolutions.com>
> Signed-off-by: Alin-Gheorghe Balutoiu <abalut...@cloudbasesolutions.com>
> ---
> V2: 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.")
> --
> 2.10.0.windows.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
