Add support for Windows Nano Server #1768

julien-lebot · 2020-05-25T16:03:59Z

Purpose

The purpose of this PR is to address #693 so that psutil can be used with Windows Nano Server containers.

Additional notes

ConnectTime

I changed the call that obtains the ConnectTime to usewtsapi since WinStationQueryInformationW seems deprecated (see Microsoft documentation).

Dynamic module loading

The API from wtsapi32.lib is loaded dynamically as recommended here

Testing

I wrote some Dockerfile that will build a custom server nano image with python and then copy psutil into that container and ran a few commands:

Nano Server Dockerfile

ARG WINDOWS_VERSION=1809
ARG PYTHON_VERSION=3.8.3
FROM python:$PYTHON_VERSION-windowsservercore-1809 as base
FROM mcr.microsoft.com/windows/nanoserver:$WINDOWS_VERSION

COPY --from=base ["Python", "Python"]

USER ContainerAdministrator
RUN setx /M PATH "%PATH%;c:\Python\;c:\Python\scripts\;"
USER ContainerUser

RUN mkdir c:\\Python\\lib\\site-packages\\psutil
COPY psutil c:\\Python\\lib\\site-packages\\psutil
ENTRYPOINT [ "python" ]

Server Core Dockerfile

ARG WINDOWS_VERSION=1809
ARG PYTHON_VERSION=3.8.3
FROM python:$PYTHON_VERSION-windowsservercore-$WINDOWS_VERSION

RUN mkdir c:\\Python\\lib\\site-packages\\psutil
COPY psutil c:\\Python\\lib\\site-packages\\psutil
ENTRYPOINT [ "python" ]

Test code

In winmake.py

def _test_windows_docker(tag, dockerfile, windows_ver = "1803", python_ver = "3.8.3"):
    docker_build = subprocess.run("docker build --isolation=hyperv --build-arg WINDOWS_VERSION=%s --build-arg PYTHON_VERSION=%s --no-cache -t %s -f %s ." % (windows_ver, python_ver, tag, dockerfile))
    if docker_build.returncode != 0:
        win_colorprint("Failed to build psutil docker image", RED)
        return
    result = subprocess.run("docker run --isolation=hyperv --rm %s -c \"import psutil\nprint(psutil.users())\"" % tag)
    print(result)


def test_docker():
    build()
    nano_server = "Dockerfiles\\windows\\Dockerfile.nanoserver"
    server_core = "Dockerfiles\\windows\\Dockerfile.servercore"
    test_matrix = [
        {
            "tag": "psutil:nanoserver-1809",
            "dockerfile": nano_server,
            "version": '1809'
        },
        {
            "tag": "psutil:servercore-1809",
            "dockerfile": server_core,
            "version": '1809'
        },
        {
            "tag": "psutil:servercore-ltsc2016",
            "dockerfile": server_core,
            "version": 'ltsc2016'
        }
    ]
    for test in test_matrix:
        print(test)
        _test_windows_docker(test["tag"], test["dockerfile"], test["version"])

… winsta.dll with wtsapi. (cherry picked from commit 7728eac066e6576d2971f2c94a4caf55bd12d4c2)

(cherry picked from commit 93a5d2ca55dc548ed8563c3733bfcc77a4084fb2)

…ween wide char and multi-byte functions.

giampaolo · 2020-06-19T19:15:17Z

psutil/arch/windows/ntextapi.h

+VOID(CALLBACK *_WTSFreeMemory)(
+    IN PVOID pMemory
+    );
+


Since these names are available on other Windows systems, I think there should be something like:

#ifdef WINDOWS_NANO_OR_SOMETHING ... #endif

There's no need of the _ prefix. Look at how other declarations are done. E.g.:

PSTR (NTAPI * _RtlIpv6AddressToStringA) ( struct in6_addr *Addr, PSTR P); #define RtlIpv6AddressToStringA _RtlIpv6AddressToStringA

I'm not entirely sure I understand what you mean with the names being available on other Windows systems.

I think the function names are still available when compiling for Windows Nano, but the library is not present on the target system (i.e. Windows Nano is not a separate compilation target with a separate SDK).

If we're worried about name conflicts, what about using a distinctive name for the functions, like so:

BOOL(CALLBACK *psutil_WTSQuerySessionInformation) ( HANDLE hServer, DWORD SessionId, WTS_INFO_CLASS WTSInfoClass, LPTSTR* ppBuffer, DWORD* pBytesReturned ); BOOL(CALLBACK *psutil_WTSEnumerateSessions)( HANDLE hServer, DWORD Reserved, DWORD Version, PWTS_SESSION_INFO* ppSessionInfo, DWORD* pCount ); VOID(CALLBACK *psutil_WTSFreeMemory)( IN PVOID pMemory ); ... psutil_WTSEnumerateSessions = psutil_GetProcAddressFromLib( "wtsapi32.dll", "WTSEnumerateSessionsW"); psutil_WTSQuerySessionInformation = psutil_GetProcAddressFromLib( "wtsapi32.dll", "WTSQuerySessionInformationW"); psutil_WTSFreeMemory = psutil_GetProcAddressFromLib( "wtsapi32.dll", "WTSFreeMemory");

This way we also removes the need for the _ prefix.

I'm not entirely sure I understand what you mean

It's OK, let me try to rephrase it differently.
What I am aiming at is using the original API names, without the _ or psutil_ prefix (the original version of this file did exactly that, then I changed it).
In order to do that you first have to "declare" the API name, like this:

psutil/psutil/arch/windows/ntextapi.h

Lines 501 to 507 in 96cc7ea

NTSTATUS (NTAPI *_NtQuerySystemInformation) (

ULONG SystemInformationClass,

PVOID SystemInformation,

ULONG SystemInformationLength,

PULONG ReturnLength);

#define NtQuerySystemInformation _NtQuerySystemInformation

Then you'll have to "load" the API, in here, like this:

psutil/psutil/_psutil_common.c

Lines 217 to 220 in 96cc7ea

NtQuerySystemInformation = psutil_GetProcAddressFromLib(

"ntdll.dll", "NtQuerySystemInformation");

if (! NtQuerySystemInformation)

return 1;

Is it more clear?

Not tested, but it should be something like this:

// ntextapi.h BOOL (CALLBACK *_WTSQuerySessionInformation) ( HANDLE hServer, DWORD SessionId, WTS_INFO_CLASS WTSInfoClass, LPTSTR* ppBuffer, DWORD* pBytesReturned ); #define WTSQuerySessionInformation _WTSQuerySessionInformation

// _psutil_common.c WTSEnumerateSessions = psutil_GetProcAddressFromLib( "wtsapi32.dll", "WTSEnumerateSessions"); if (! WTSEnumerateSessions) return 1;

Also, MS doc says:

The wtsapi32.h header defines WTSEnumerateSessions as an alias which automatically selects the ANSI or Unicode version of this function based on the definition of the UNICODE preprocessor constant.

...so I think you should keep using WTSEnumerateSessions instead of WTSEnumerateSessionsW.

Thanks for the explanation, I think I see what you're aiming for.
I included the header Wtsapi32.h in ntextapi.h to use the existing structures definition, but the point of ntextapi.h is to be independent of such header, right ?
So I should remove that #include and copy over all the definitions required to make it look like we are using the original header, correct ?

If you can fix the issue just by adding that new header (or from setup.py) there is no need to redefine anything in ntextapi.h.

Just define them explicitly in ntextapi.h only if you can’t solve it otherwise, in which case you should put the definition inside an “#ifdef nanoserver” because I think it will break other windows versions (but the CI tests should break)

AFAIK there is no way to distinguish Windows Nano via preprocessor definitions - I think it is still using the same _WIN32_WINNT and WINVER as a regular Windows.
If we introduce a new prepocessor flag for Windows Nano, then we would have to maintain 2 versions of psutil, one linked with WtsApi32.lib and one that loads dynamically the DLL at runtime.
Why not just keep the dynamically loaded version for all version of Windows and avoid that issue altogether ?
You mention it will break other Windows version, do you have something specific in mind ?
To me it seems the dynamic loading will also work on other Windows version (I ran my tests on many versions of Windows actually - I should probably update the PR description).

giampaolo · 2020-06-19T19:18:32Z

psutil/_psutil_common.c

@@ -320,6 +316,20 @@ psutil_loadlibs() {
    // minumum requirement: Win 7
    GetLogicalProcessorInformationEx = psutil_GetProcAddressFromLib(
        "kernel32", "GetLogicalProcessorInformationEx");
+    // minimum requirements: Windows Server Core
+#ifdef UNICODE


Mmm. Is this necessary? Is it possible for a system to not have unicode support?
Regardless from that, we already use other unicode "W" functions in many other places. If this is broken then they are all broken (in which case it should be a separate fix).

I wasn't sure if psutil was ever compiled with UNICODE unset, so I provided both "A" and "W". I will simplify this to just use the unicode methods.

Only provide UNICODE variants of WTSQuerySessionInformation and WTSEnumerateSessions. Copy definitions from WtsApi32.h in ntextapi.h to prevent conflicting name definitions. Removed _ prefix for WTS* api calls.

giampaolo · 2020-06-22T17:42:12Z

psutil/_psutil_windows.c

+        WTSFreeMemory == NULL) {
+        PyErr_SetString(PyExc_RuntimeError, "wtsapi32.dll cannot be found.");
+        goto error;
+    }


I'm not sure I understand. Are these names available on Windows Nano? Does this function (users()) work at all?

No, those names are not available on Windows Nano, and consequently users does not work. If you try to run psutil.users in Windows Nano, that is what you get:

OSError: [WinError 120] This function is not supported on this system: '(originated from WTSEnumerateSessions)'

But by loading those symbols dynamically, at least psutil can be used on Windows Nano. Previously it would simply try to load WtsApi32.dll and fail even if you never called users().

If Windows Nano does not support users at all then I think it makes more sense to just return an empty list. It's less disruptive than an error.

I think return Py_BuildValue("[]"); should do it.

giampaolo · 2020-06-25T02:32:24Z

psutil/_psutil_windows.c


        // username
        bytes = 0;
-        if (WTSQuerySessionInformationW(hServer, sessionId, WTSUserName,
+        if (WTSQuerySessionInformation(hServer, sessionId, WTSUserName,


I think it makes more sense to keep the old unicode name.

giampaolo · 2020-06-25T02:34:36Z

psutil/_psutil_windows.c

-        {
-            PyErr_SetFromOSErrnoWithSyscall("WinStationQueryInformationW");
+        bytes = 0;
+        if (WTSQuerySessionInformation(hServer, sessionId, WTSSessionInfo,


same here, keep the old name

(cherry picked from commit 1c82c6a1d64c47c94e85e59b54d3737228d245af)

(cherry picked from commit 6013def71d72455994e6949156d8f16687ac96b4)

(cherry picked from commit e6a7d4ca81e6983a05916bb4ae0df7ed25db674f) (cherry picked from commit 11108cce06ddf28f656b809d64aa7f9c6a32cc96)

(cherry picked from commit c7569a85893e83801473cd6e12bacf08c97d5160)

(cherry picked from commit cf330cb9579ef250c57d22ec91cd5557df89896e)

giampaolo · 2020-07-05T02:02:26Z

Merged. Glad to see a new platform being supported.

julien-lebot added 5 commits May 25, 2020 17:43

Make calls to wtsapi dynamic to support windows nano. Replace call to…

650af83

… winsta.dll with wtsapi. (cherry picked from commit 7728eac066e6576d2971f2c94a4caf55bd12d4c2)

Make linter happy

09f97e7

(cherry picked from commit 93a5d2ca55dc548ed8563c3733bfcc77a4084fb2)

Fixed a memory leak, simplified callback declaration, fixed mixup bet…

5b53c38

…ween wide char and multi-byte functions.

Fix issue with WTSClientAddress

0c8b10a

Merge branch 'master' into windows_nano

5e03404

giampaolo requested changes Jun 19, 2020

View reviewed changes

Addressed review comments.

46c393b

Only provide UNICODE variants of WTSQuerySessionInformation and WTSEnumerateSessions. Copy definitions from WtsApi32.h in ntextapi.h to prevent conflicting name definitions. Removed _ prefix for WTS* api calls.

giampaolo reviewed Jun 22, 2020

View reviewed changes

giampaolo reviewed Jun 25, 2020

View reviewed changes

albertvaka mentioned this pull request Jun 25, 2020

Pin k8s.io/klog@release-1.x DataDog/datadog-agent#5580

Merged

julien-lebot added 7 commits June 30, 2020 14:19

Use original unicode name

e049b8b

(cherry picked from commit 1c82c6a1d64c47c94e85e59b54d3737228d245af)

Check last error code instead of checking if API == null

edb50f1

(cherry picked from commit 6013def71d72455994e6949156d8f16687ac96b4)

Use ony Unicode objects

4d7deaf

(cherry picked from commit e6a7d4ca81e6983a05916bb4ae0df7ed25db674f) (cherry picked from commit 11108cce06ddf28f656b809d64aa7f9c6a32cc96)

Re-add null api check

1f1191c

(cherry picked from commit c7569a85893e83801473cd6e12bacf08c97d5160)

Ensure we use only UNICODE objects

1c5b069

(cherry picked from commit cf330cb9579ef250c57d22ec91cd5557df89896e)

Improve documentation.

22dfd24

Set syscall name in error description to UNICODE name

12982a6

julien-lebot force-pushed the windows_nano branch from dcc970f to 12982a6 Compare July 1, 2020 15:33

giampaolo merged commit 54de882 into giampaolo:master Jul 5, 2020

giampaolo added a commit that referenced this pull request Jul 5, 2020

give CREDITS for #1768 and #1781

57775dd

julien-lebot mentioned this pull request Jul 6, 2020

Revert "Revert "Change Windows Containers to Windows Server Nano"" DataDog/datadog-agent#5898

Merged

giampaolo added windows enhancement labels Nov 15, 2020

giampaolo mentioned this pull request Dec 29, 2020

psutil compatibility on Windows Nano Server #693

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Windows Nano Server #1768

Add support for Windows Nano Server #1768

julien-lebot commented May 25, 2020 •

edited

giampaolo Jun 19, 2020

julien-lebot Jun 22, 2020

giampaolo Jun 22, 2020

giampaolo Jun 22, 2020

giampaolo Jun 22, 2020

julien-lebot Jun 22, 2020

giampaolo Jun 22, 2020

julien-lebot Jun 22, 2020

giampaolo Jun 19, 2020

julien-lebot Jun 22, 2020

giampaolo Jun 22, 2020

julien-lebot Jun 23, 2020

julien-lebot Jun 23, 2020

giampaolo Jun 25, 2020

giampaolo Jun 25, 2020 •

edited

giampaolo Jun 25, 2020

giampaolo Jun 25, 2020

giampaolo commented Jul 5, 2020

	NTSTATUS (NTAPI *_NtQuerySystemInformation) (
	ULONG SystemInformationClass,
	PVOID SystemInformation,
	ULONG SystemInformationLength,
	PULONG ReturnLength);

	#define NtQuerySystemInformation _NtQuerySystemInformation

	NtQuerySystemInformation = psutil_GetProcAddressFromLib(
	"ntdll.dll", "NtQuerySystemInformation");
	if (! NtQuerySystemInformation)
	return 1;

Add support for Windows Nano Server #1768

Add support for Windows Nano Server #1768

Conversation

julien-lebot commented May 25, 2020 • edited

Purpose

Additional notes

ConnectTime

Dynamic module loading

Testing

Nano Server Dockerfile

Server Core Dockerfile

Test code

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giampaolo Jun 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giampaolo commented Jul 5, 2020

julien-lebot commented May 25, 2020 •

edited

giampaolo Jun 25, 2020 •

edited