You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2018/05/02 01:38:05 UTC
[05/31] mesos git commit: Windows: Replaced `_wopen()` with
`CreateFileW()` in `os::open()`.
Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.
Instead of using the CRT implementation of `_wopen()` for the
`os::open()` API, we now use the Windows API `CreateFileW()`, mapping
each of the Linux `open()` flags to their semantic equivalents. This
will make implementing overlapped I/O possible, and is a step toward
removing the use of integer file descriptors on Windows.
Note that instead of redefining the C flags like `O_RDONLY`, we just
use them directly in our mapping logic, and set the used but
unsupported flags to zero.
This change uncovered several bugs such as incorrect access flags, and
used-but-not-included headers.
We currently ignore creation permissions as they will be handled in a
broader project to map permissions to Windows correctly.
Review: https://reviews.apache.org/r/66424
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/86bb9641
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/86bb9641
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/86bb9641
Branch: refs/heads/master
Commit: 86bb96413cba3b2f349d21e08b30cac78d9f7f5a
Parents: 8b7798f
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Fri Mar 16 11:38:17 2018 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue May 1 18:36:04 2018 -0700
----------------------------------------------------------------------
3rdparty/stout/include/stout/net.hpp | 1 +
.../stout/include/stout/os/windows/fcntl.hpp | 10 --
.../stout/include/stout/os/windows/mktemp.hpp | 3 +-
.../stout/include/stout/os/windows/open.hpp | 114 ++++++++++++++++---
3rdparty/stout/include/stout/windows.hpp | 5 -
5 files changed, 100 insertions(+), 33 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/net.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/net.hpp b/3rdparty/stout/include/stout/net.hpp
index d2992c0..52fa09b 100644
--- a/3rdparty/stout/include/stout/net.hpp
+++ b/3rdparty/stout/include/stout/net.hpp
@@ -56,6 +56,7 @@
#include <stout/try.hpp>
#include <stout/os/int_fd.hpp>
+#include <stout/os/close.hpp>
#include <stout/os/open.hpp>
#ifdef __WINDOWS__
http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/os/windows/fcntl.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/fcntl.hpp b/3rdparty/stout/include/stout/os/windows/fcntl.hpp
index bf8c38a..0e8fa8d 100644
--- a/3rdparty/stout/include/stout/os/windows/fcntl.hpp
+++ b/3rdparty/stout/include/stout/os/windows/fcntl.hpp
@@ -22,16 +22,6 @@
#include <stout/os/socket.hpp>
#include <stout/os/windows/fd.hpp>
-#define O_RDONLY _O_RDONLY
-#define O_WRONLY _O_WRONLY
-#define O_RDWR _O_RDWR
-#define O_CREAT _O_CREAT
-#define O_TRUNC _O_TRUNC
-#define O_APPEND _O_APPEND
-// NOTE: Windows does not support the semantics of close-on-exec. Instead, by
-// default we set all handles to be non-inheritable.
-#define O_CLOEXEC 0
-
namespace os {
inline Try<Nothing> cloexec(const WindowsFD& fd)
http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/os/windows/mktemp.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/mktemp.hpp b/3rdparty/stout/include/stout/os/windows/mktemp.hpp
index 5c775c4..b4f5279 100644
--- a/3rdparty/stout/include/stout/os/windows/mktemp.hpp
+++ b/3rdparty/stout/include/stout/os/windows/mktemp.hpp
@@ -61,7 +61,8 @@ inline Try<std::string> mktemp(
// attempt to match POSIX's specification of `mkstemp`. We use `_S_IREAD` and
// `_S_IWRITE` here instead of the POSIX equivalents. On Windows the file is
// is not present, we use `_O_CREAT` option when opening the file.
- Try<int_fd> fd = os::open(temp_file, _O_CREAT, _S_IREAD | _S_IWRITE);
+ Try<int_fd> fd =
+ os::open(temp_file, O_RDWR | O_CREAT | O_EXCL, _S_IREAD | _S_IWRITE);
if (fd.isError()) {
return Error(fd.error());
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/os/windows/open.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/windows/open.hpp b/3rdparty/stout/include/stout/os/windows/open.hpp
index 9598fdd..7bfaa28 100644
--- a/3rdparty/stout/include/stout/os/windows/open.hpp
+++ b/3rdparty/stout/include/stout/os/windows/open.hpp
@@ -16,38 +16,118 @@
#include <string>
#include <stout/error.hpp>
-#include <stout/nothing.hpp>
#include <stout/try.hpp>
-#include <stout/windows.hpp> // For `mode_t`.
+#include <stout/windows.hpp> // For `mode_t`.
-#include <stout/os/close.hpp>
-#include <stout/os/fcntl.hpp> // For `oflag` values.
#include <stout/os/int_fd.hpp>
#include <stout/internal/windows/longpath.hpp>
-#ifndef O_CLOEXEC
-#error "missing O_CLOEXEC support on this platform"
-// NOTE: On Windows, `fnctl.hpp` defines `O_CLOEXEC` to a no-op.
-#endif
+// TODO(andschwa): Windows does not support the Linux extension
+// O_NONBLOCK, as asynchronous I/O is done through other mechanisms.
+// Overlapped I/O will be implemented later.
+constexpr int O_NONBLOCK = 0;
+
+// Windows does not support the Linux extension O_SYNC, as buffering
+// is done differently.
+// TODO(andschwa): This could be equivalent to
+// `FILE_FLAG_WRITE_THROUGH`, but we don't seem to need it.
+constexpr int O_SYNC = 0;
+
+// Windows does not support the Linux extension O_CLOEXEC. Instead, by
+// default we set all handles to be non-inheritable.
+constexpr int O_CLOEXEC = 0;
namespace os {
+// TODO(andschwa): Handle specified creation permissions in `mode_t mode`. See
+// MESOS-3176.
inline Try<int_fd> open(const std::string& path, int oflag, mode_t mode = 0)
{
std::wstring longpath = ::internal::windows::longpath(path);
- // By default, Windows will perform "text translation" meaning that it will
- // automatically write CR/LF instead of LF line feeds. To prevent this, and
- // use the POSIX semantics, we open with `O_BINARY`.
+
+ // Map the POSIX `oflag` access flags.
+
+ // O_APPEND: Write only appends.
//
- // Also by default, we will mimic the Windows (non-CRT) APIs and make all
- // opened handles non-inheritable.
- int_fd fd = ::_wopen(longpath.data(), oflag | O_BINARY | O_NOINHERIT, mode);
- if (fd < 0) {
- return ErrnoError();
+ // NOTE: We choose a `write` flag here because emulating `O_APPEND`
+ // requires granting the `FILE_APPEND_DATA` access right, but not
+ // the `FILE_WRITE_DATA` access right, which `GENERIC_WRITE` would
+ // otherwise grant.
+ const DWORD write = (oflag & O_APPEND) ? FILE_APPEND_DATA : GENERIC_WRITE;
+
+ DWORD access;
+ switch (oflag & (O_RDONLY | O_WRONLY | O_RDWR)) {
+ case O_RDONLY: {
+ access = GENERIC_READ;
+ break;
+ }
+ case O_WRONLY: {
+ access = write;
+ break;
+ }
+ case O_RDWR: {
+ access = GENERIC_READ | write;
+ break;
+ }
+ default: {
+ return Error("Access mode not specified.");
+ }
+ }
+
+ // Map the POSIX `oflag` creation flags.
+ DWORD create;
+ switch (oflag & (O_CREAT | O_EXCL | O_TRUNC)) {
+ case O_CREAT: {
+ // Create a new file or open an existing file.
+ create = OPEN_ALWAYS;
+ break;
+ }
+ case O_CREAT | O_EXCL:
+ case O_CREAT | O_EXCL | O_TRUNC: {
+ // Create a new file, but fail if it already exists.
+ // Ignore `O_TRUNC` with `O_CREAT | O_EXCL`
+ create = CREATE_NEW;
+ break;
+ }
+ case O_CREAT | O_TRUNC: {
+ // Truncate file if it already exists.
+ create = CREATE_ALWAYS;
+ break;
+ }
+ case O_EXCL:
+ case O_EXCL | O_TRUNC: {
+ return Error("`O_EXCL` is undefined without `O_CREAT`.");
+ }
+ case O_TRUNC: {
+ // Truncate file if it exists, otherwise fail.
+ create = TRUNCATE_EXISTING;
+ break;
+ }
+ default: {
+ // Open file if it exists, otherwise fail.
+ create = OPEN_EXISTING;
+ break;
+ }
+ }
+
+ const HANDLE handle = ::CreateFileW(
+ longpath.data(),
+ access,
+ // Share all access so we don't lock the file.
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
+ // Disable inheritance by default.
+ nullptr,
+ create,
+ FILE_ATTRIBUTE_NORMAL,
+ // No template file.
+ nullptr);
+
+ if (handle == INVALID_HANDLE_VALUE) {
+ return WindowsError();
}
- return fd;
+ return handle;
}
} // namespace os {
http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/windows.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/windows.hpp b/3rdparty/stout/include/stout/windows.hpp
index 1bfcdf4..8056ad8 100644
--- a/3rdparty/stout/include/stout/windows.hpp
+++ b/3rdparty/stout/include/stout/windows.hpp
@@ -335,11 +335,6 @@ const mode_t S_ISUID = 0x08000000; // No-op.
const mode_t S_ISGID = 0x04000000; // No-op.
const mode_t S_ISVTX = 0x02000000; // No-op.
-
-// Flags not supported by Windows.
-const mode_t O_SYNC = 0x00000000; // No-op.
-const mode_t O_NONBLOCK = 0x00000000; // No-op.
-
// Even though SIGKILL doesn't exist on Windows, we define
// it here, because Docker defines it. So, the docker
// executor needs this signal value to properly kill containers.