[PATCH 0/5] Enable unix socket support on Windows

[PATCH 0/5] Enable unix socket support on Windows

[Bin Meng]

Support for the unix socket has existed both in BSD and Linux for the
longest time, but not on Windows. Since Windows 10 build 17063 [1],
the native support for the unix socket has came to Windows. Starting
this build, two Win32 processes can use the AF_UNIX address family
over Winsock API to communicate with each other.

Introduce a new build time config option CONFIG_AF_UNIX when the build
host has such a capability, and a run-time check afunix_available() for
Windows host in the QEMU sockets util codes.

[1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/


Bin Meng (5):
  util/qemu-sockets: Replace the call to close a socket with
    closesocket()
  util/oslib-win32: Add a helper to get the Windows version
  qga/commands-win32: Use os_get_win_version()
  util/qemu-sockets: Enable unix socket support on Windows
  chardev/char-socket: Update AF_UNIX for Windows

 meson.build               |  6 +++++
 include/sysemu/os-win32.h |  2 ++
 chardev/char-socket.c     |  8 +++++-
 qga/commands-win32.c      | 27 +-------------------
 util/oslib-win32.c        | 15 +++++++++++
 util/qemu-sockets.c       | 52 ++++++++++++++++++++++++++++++++-------
 6 files changed, 74 insertions(+), 36 deletions(-)

-- 
2.34.1

[PATCH 1/5] util/qemu-sockets: Replace the call to close a socket with closesocket()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

close() is a *nix function. It works on any file descriptor, and
sockets in *nix are an example of a file descriptor.

closesocket() is a Windows-specific function, which works only
specifically with sockets. Sockets on Windows do not use *nix-style
file descriptors, and socket() returns a handle to a kernel object
instead, so it must be closed with closesocket().

In QEMU there is already a logic to handle such platform difference
in os-posix.h and os-win32.h, that:

  * closesocket maps to close on POSIX
  * closesocket maps to a wrapper that calls the real closesocket()
    on Windows

Replace the call to close a socket with closesocket() instead.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 util/qemu-sockets.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 13b5b197f9..0e2298278f 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -487,7 +487,7 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error **errp)
 
         if (ret < 0) {
             error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
-            close(sock);
+            closesocket(sock);
             return -1;
         }
     }
@@ -1050,7 +1050,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
     return sock;
 
  err:
-    close(sock);
+    closesocket(sock);
     return -1;
 }
 
-- 
2.34.1

[PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This adds a helper to get the Windows version via the RtlGetVersion
call, for QEMU codes to determine the Windows version at run-time.

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 include/sysemu/os-win32.h |  2 ++
 util/oslib-win32.c        | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index edc3b38a57..1e324026a4 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
 ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
                            struct sockaddr *addr, socklen_t *addrlen);
 
+void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 5723d3eb4c..6d2387b9ff 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
      */
     return qemu_fdatasync(fd);
 }
+
+void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
+{
+    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
+
+    /* RtlGetVersion is available starting with Windows 2000 */
+    HMODULE module = GetModuleHandle("ntdll");
+    PVOID fun = GetProcAddress(module, "RtlGetVersion");
+    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
+
+    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
+    rtl_get_version(info);
+
+    return;
+}
-- 
2.34.1

[PATCH 3/5] qga/commands-win32: Use os_get_win_version()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Drop its own ga_get_win_version() implementation, and use the one
provided in oslib-win32 instead.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 qga/commands-win32.c | 27 +--------------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 7ed7664715..6186f2e1f2 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2178,26 +2178,6 @@ static ga_win_10_0_t const WIN_10_0_CLIENT_VERSION_MATRIX[3] = {
     {0, 0}
 };
 
-static void ga_get_win_version(RTL_OSVERSIONINFOEXW *info, Error **errp)
-{
-    typedef NTSTATUS(WINAPI *rtl_get_version_t)(
-        RTL_OSVERSIONINFOEXW *os_version_info_ex);
-
-    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
-
-    HMODULE module = GetModuleHandle("ntdll");
-    PVOID fun = GetProcAddress(module, "RtlGetVersion");
-    if (fun == NULL) {
-        error_setg(errp, QERR_QGA_COMMAND_FAILED,
-            "Failed to get address of RtlGetVersion");
-        return;
-    }
-
-    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
-    rtl_get_version(info);
-    return;
-}
-
 static char *ga_get_win_name(OSVERSIONINFOEXW const *os_version, bool id)
 {
     DWORD major = os_version->dwMajorVersion;
@@ -2312,17 +2292,12 @@ static char *ga_get_current_arch(void)
 
 GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
 {
-    Error *local_err = NULL;
     OSVERSIONINFOEXW os_version = {0};
     bool server;
     char *product_name;
     GuestOSInfo *info;
 
-    ga_get_win_version(&os_version, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return NULL;
-    }
+    os_get_win_version(&os_version);
 
     server = os_version.wProductType != VER_NT_WORKSTATION;
     product_name = ga_get_win_product_name(errp);
-- 
2.34.1

[PATCH 4/5] util/qemu-sockets: Enable unix socket support on Windows

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Support for the unix socket has existed both in BSD and Linux for the
longest time, but not on Windows. Since Windows 10 build 17063 [1],
the native support for the unix socket has came to Windows. Starting
this build, two Win32 processes can use the AF_UNIX address family
over Winsock API to communicate with each other.

Introduce a new build time config option CONFIG_AF_UNIX when the build
host has such a capability, and a run-time check afunix_available() for
Windows host in the QEMU sockets util codes.

[1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/

Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 meson.build         |  6 ++++++
 util/qemu-sockets.c | 48 ++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/meson.build b/meson.build
index 75aaca8462..73e5de5957 100644
--- a/meson.build
+++ b/meson.build
@@ -2327,6 +2327,12 @@ have_afalg = get_option('crypto_afalg') \
   '''), error_message: 'AF_ALG requested but could not be detected').allowed()
 config_host_data.set('CONFIG_AF_ALG', have_afalg)
 
+if targetos != 'windows'
+  config_host_data.set('CONFIG_AF_UNIX', true)
+else
+  config_host_data.set('CONFIG_AF_UNIX', cc.has_header('afunix.h'))
+endif
+
 config_host_data.set('CONFIG_AF_VSOCK', cc.has_header_symbol(
   'linux/vm_sockets.h', 'AF_VSOCK',
   prefix: '#include <sys/socket.h>',
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 0e2298278f..d85f3ea3ee 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -17,6 +17,15 @@
  */
 #include "qemu/osdep.h"
 
+#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
+# include <afunix.h>
+/*
+ * AF_UNIX support is available since Windows 10 build 17063
+ * See https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
+ */
+# define WIN_BUILD_AF_UNIX 17063
+#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
+
 #ifdef CONFIG_AF_VSOCK
 #include <linux/vm_sockets.h>
 #endif /* CONFIG_AF_VSOCK */
@@ -880,7 +889,7 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
 }
 #endif /* CONFIG_AF_VSOCK */
 
-#ifndef _WIN32
+#ifdef CONFIG_AF_UNIX
 
 static bool saddr_is_abstract(UnixSocketAddress *saddr)
 {
@@ -900,6 +909,17 @@ static bool saddr_is_tight(UnixSocketAddress *saddr)
 #endif
 }
 
+#ifdef CONFIG_WIN32
+static bool afunix_available(void)
+{
+    OSVERSIONINFOEXW os_version = { 0 };
+
+    os_get_win_version(&os_version);
+
+    return os_version.dwBuildNumber >= WIN_BUILD_AF_UNIX;
+}
+#endif
+
 static int unix_listen_saddr(UnixSocketAddress *saddr,
                              int num,
                              Error **errp)
@@ -912,6 +932,13 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
     size_t pathlen;
     size_t addrlen;
 
+#ifdef CONFIG_WIN32
+    if (!afunix_available()) {
+        error_setg(errp, "AF_UNIX is not available on your Windows");
+        return -1;
+    }
+#endif
+
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
         error_setg_errno(errp, errno, "Failed to create Unix socket");
@@ -1004,6 +1031,13 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
         return -1;
     }
 
+#ifdef CONFIG_WIN32
+    if (!afunix_available()) {
+        error_setg(errp, "AF_UNIX is not available on your Windows");
+        return -1;
+    }
+#endif
+
     sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
     if (sock < 0) {
         error_setg_errno(errp, errno, "Failed to create socket");
@@ -1060,14 +1094,14 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
                              int num,
                              Error **errp)
 {
-    error_setg(errp, "unix sockets are not available on windows");
+    error_setg(errp, "unix sockets are not available on your host");
     errno = ENOTSUP;
     return -1;
 }
 
 static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
 {
-    error_setg(errp, "unix sockets are not available on windows");
+    error_setg(errp, "unix sockets are not available on your host");
     errno = ENOTSUP;
     return -1;
 }
@@ -1335,7 +1369,7 @@ socket_sockaddr_to_address_inet(struct sockaddr_storage *sa,
 }
 
 
-#ifndef WIN32
+#ifdef CONFIG_AF_UNIX
 static SocketAddress *
 socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
                                 socklen_t salen,
@@ -1362,7 +1396,7 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
     addr->u.q_unix.path = g_strndup(su->sun_path, salen);
     return addr;
 }
-#endif /* WIN32 */
+#endif /* CONFIG_AF_UNIX */
 
 #ifdef CONFIG_AF_VSOCK
 static SocketAddress *
@@ -1394,10 +1428,10 @@ socket_sockaddr_to_address(struct sockaddr_storage *sa,
     case AF_INET6:
         return socket_sockaddr_to_address_inet(sa, salen, errp);
 
-#ifndef WIN32
+#ifdef CONFIG_AF_UNIX
     case AF_UNIX:
         return socket_sockaddr_to_address_unix(sa, salen, errp);
-#endif /* WIN32 */
+#endif
 
 #ifdef CONFIG_AF_VSOCK
     case AF_VSOCK:
-- 
2.34.1

[PATCH 5/5] chardev/char-socket: Update AF_UNIX for Windows

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Now that AF_UNIX has come to Windows, update the existing logic in
qemu_chr_compute_filename() and qmp_chardev_open_socket() for Windows.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 chardev/char-socket.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index dc4e218eeb..5a1fa0db33 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -37,6 +37,10 @@
 #include "chardev/char-io.h"
 #include "chardev/char-socket.h"
 
+#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
+# include <afunix.h>
+#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
+
 static gboolean socket_reconnect_timeout(gpointer opaque);
 static void tcp_chr_telnet_init(Chardev *chr);
 
@@ -557,7 +561,7 @@ static char *qemu_chr_compute_filename(SocketChardev *s)
     const char *left = "", *right = "";
 
     switch (ss->ss_family) {
-#ifndef _WIN32
+#ifdef CONFIG_AF_UNIX
     case AF_UNIX:
         return g_strdup_printf("unix:%s%s",
                                ((struct sockaddr_un *)(ss))->sun_path,
@@ -1372,10 +1376,12 @@ static void qmp_chardev_open_socket(Chardev *chr,
     }
 
     qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_RECONNECTABLE);
+#ifndef _WIN32
     /* TODO SOCKET_ADDRESS_FD where fd has AF_UNIX */
     if (addr->type == SOCKET_ADDRESS_TYPE_UNIX) {
         qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
     }
+#endif
 
     /*
      * In the chardev-change special-case, we shouldn't register a new yank
-- 
2.34.1

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Yan Vugenfirer]

On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This adds a helper to get the Windows version via the RtlGetVersion
> call, for QEMU codes to determine the Windows version at run-time.
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  include/sysemu/os-win32.h |  2 ++
>  util/oslib-win32.c        | 15 +++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index edc3b38a57..1e324026a4 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
>  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
>                             struct sockaddr *addr, socklen_t *addrlen);
>
> +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 5723d3eb4c..6d2387b9ff 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
>       */
>      return qemu_fdatasync(fd);
>  }
> +
> +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> +{
> +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> +
> +    /* RtlGetVersion is available starting with Windows 2000 */
> +    HMODULE module = GetModuleHandle("ntdll");
> +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> +
> +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> +    rtl_get_version(info);
The original function, when it was present in qemu-ga, tested that
getting the function address succeeded.
I think this test should be kept.
See below:
-    PVOID fun = GetProcAddress(module, "RtlGetVersion");
-    if (fun == NULL) {
-        error_setg(errp, QERR_QGA_COMMAND_FAILED,
-            "Failed to get address of RtlGetVersion");
-        return;
-    }

Best regards,
Yan.
> +
> +    return;
> +}
> --
> 2.34.1
>
>

Re: [PATCH 4/5] util/qemu-sockets: Enable unix socket support on Windows

[Yan Vugenfirer]

On Wed, Jul 27, 2022 at 10:46 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> Support for the unix socket has existed both in BSD and Linux for the
> longest time, but not on Windows. Since Windows 10 build 17063 [1],
> the native support for the unix socket has came to Windows. Starting
> this build, two Win32 processes can use the AF_UNIX address family
> over Winsock API to communicate with each other.
>
> Introduce a new build time config option CONFIG_AF_UNIX when the build
> host has such a capability, and a run-time check afunix_available() for
> Windows host in the QEMU sockets util codes.
>
> [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  meson.build         |  6 ++++++
>  util/qemu-sockets.c | 48 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 75aaca8462..73e5de5957 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2327,6 +2327,12 @@ have_afalg = get_option('crypto_afalg') \
>    '''), error_message: 'AF_ALG requested but could not be detected').allowed()
>  config_host_data.set('CONFIG_AF_ALG', have_afalg)
>
> +if targetos != 'windows'
> +  config_host_data.set('CONFIG_AF_UNIX', true)
> +else
> +  config_host_data.set('CONFIG_AF_UNIX', cc.has_header('afunix.h'))
> +endif
> +
>  config_host_data.set('CONFIG_AF_VSOCK', cc.has_header_symbol(
>    'linux/vm_sockets.h', 'AF_VSOCK',
>    prefix: '#include <sys/socket.h>',
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 0e2298278f..d85f3ea3ee 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -17,6 +17,15 @@
>   */
>  #include "qemu/osdep.h"
>
> +#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
> +# include <afunix.h>
> +/*
> + * AF_UNIX support is available since Windows 10 build 17063
> + * See https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> + */
> +# define WIN_BUILD_AF_UNIX 17063
> +#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
> +
>  #ifdef CONFIG_AF_VSOCK
>  #include <linux/vm_sockets.h>
>  #endif /* CONFIG_AF_VSOCK */
> @@ -880,7 +889,7 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
>  }
>  #endif /* CONFIG_AF_VSOCK */
>
> -#ifndef _WIN32
> +#ifdef CONFIG_AF_UNIX
>
>  static bool saddr_is_abstract(UnixSocketAddress *saddr)
>  {
> @@ -900,6 +909,17 @@ static bool saddr_is_tight(UnixSocketAddress *saddr)
>  #endif
>  }
>
> +#ifdef CONFIG_WIN32
> +static bool afunix_available(void)
> +{
> +    OSVERSIONINFOEXW os_version = { 0 };
> +
> +    os_get_win_version(&os_version);
> +
> +    return os_version.dwBuildNumber >= WIN_BUILD_AF_UNIX;
It can be that CONFIG_WIN32 is defined,but CONFIG_AF_UNIX is not. In
this case WIN_BUILD_AF_UNIX will be undefined.
Also, WIN_BUILD_AF_UNIX is just a build constant, why not define it
always under CONFIG_WIN32?

Best regards,
Yan.


> +}
> +#endif
> +
>  static int unix_listen_saddr(UnixSocketAddress *saddr,
>                               int num,
>                               Error **errp)
> @@ -912,6 +932,13 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>      size_t pathlen;
>      size_t addrlen;
>
> +#ifdef CONFIG_WIN32
> +    if (!afunix_available()) {
> +        error_setg(errp, "AF_UNIX is not available on your Windows");
> +        return -1;
> +    }
> +#endif
> +
>      sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
>      if (sock < 0) {
>          error_setg_errno(errp, errno, "Failed to create Unix socket");
> @@ -1004,6 +1031,13 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
>          return -1;
>      }
>
> +#ifdef CONFIG_WIN32
> +    if (!afunix_available()) {
> +        error_setg(errp, "AF_UNIX is not available on your Windows");
> +        return -1;
> +    }
> +#endif
> +
>      sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
>      if (sock < 0) {
>          error_setg_errno(errp, errno, "Failed to create socket");
> @@ -1060,14 +1094,14 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>                               int num,
>                               Error **errp)
>  {
> -    error_setg(errp, "unix sockets are not available on windows");
> +    error_setg(errp, "unix sockets are not available on your host");
>      errno = ENOTSUP;
>      return -1;
>  }
>
>  static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
>  {
> -    error_setg(errp, "unix sockets are not available on windows");
> +    error_setg(errp, "unix sockets are not available on your host");
>      errno = ENOTSUP;
>      return -1;
>  }
> @@ -1335,7 +1369,7 @@ socket_sockaddr_to_address_inet(struct sockaddr_storage *sa,
>  }
>
>
> -#ifndef WIN32
> +#ifdef CONFIG_AF_UNIX
>  static SocketAddress *
>  socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
>                                  socklen_t salen,
> @@ -1362,7 +1396,7 @@ socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
>      addr->u.q_unix.path = g_strndup(su->sun_path, salen);
>      return addr;
>  }
> -#endif /* WIN32 */
> +#endif /* CONFIG_AF_UNIX */
>
>  #ifdef CONFIG_AF_VSOCK
>  static SocketAddress *
> @@ -1394,10 +1428,10 @@ socket_sockaddr_to_address(struct sockaddr_storage *sa,
>      case AF_INET6:
>          return socket_sockaddr_to_address_inet(sa, salen, errp);
>
> -#ifndef WIN32
> +#ifdef CONFIG_AF_UNIX
>      case AF_UNIX:
>          return socket_sockaddr_to_address_unix(sa, salen, errp);
> -#endif /* WIN32 */
> +#endif
>
>  #ifdef CONFIG_AF_VSOCK
>      case AF_VSOCK:
> --
> 2.34.1
>
>

Re: [PATCH 4/5] util/qemu-sockets: Enable unix socket support on Windows

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 5680 bytes --]

On Wed, Jul 27, 2022 at 10:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:

> From: Bin Meng <bin.meng@windriver.com>
>
> Support for the unix socket has existed both in BSD and Linux for the
> longest time, but not on Windows. Since Windows 10 build 17063 [1],
> the native support for the unix socket has came to Windows. Starting
> this build, two Win32 processes can use the AF_UNIX address family
> over Winsock API to communicate with each other.
>
> Introduce a new build time config option CONFIG_AF_UNIX when the build
> host has such a capability, and a run-time check afunix_available() for
> Windows host in the QEMU sockets util codes.
>
> [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
>
> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  meson.build         |  6 ++++++
>  util/qemu-sockets.c | 48 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 47 insertions(+), 7 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 75aaca8462..73e5de5957 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2327,6 +2327,12 @@ have_afalg = get_option('crypto_afalg') \
>    '''), error_message: 'AF_ALG requested but could not be
> detected').allowed()
>  config_host_data.set('CONFIG_AF_ALG', have_afalg)
>
> +if targetos != 'windows'
> +  config_host_data.set('CONFIG_AF_UNIX', true)
> +else
> +  config_host_data.set('CONFIG_AF_UNIX', cc.has_header('afunix.h'))
> +endif
> +
>  config_host_data.set('CONFIG_AF_VSOCK', cc.has_header_symbol(
>    'linux/vm_sockets.h', 'AF_VSOCK',
>    prefix: '#include <sys/socket.h>',
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 0e2298278f..d85f3ea3ee 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -17,6 +17,15 @@
>   */
>  #include "qemu/osdep.h"
>
> +#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
> +# include <afunix.h>
> +/*
> + * AF_UNIX support is available since Windows 10 build 17063
> + * See
> https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> + */
> +# define WIN_BUILD_AF_UNIX 17063
> +#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
> +
>  #ifdef CONFIG_AF_VSOCK
>  #include <linux/vm_sockets.h>
>  #endif /* CONFIG_AF_VSOCK */
> @@ -880,7 +889,7 @@ static int vsock_parse(VsockSocketAddress *addr, const
> char *str,
>  }
>  #endif /* CONFIG_AF_VSOCK */
>
> -#ifndef _WIN32
> +#ifdef CONFIG_AF_UNIX
>
>  static bool saddr_is_abstract(UnixSocketAddress *saddr)
>  {
> @@ -900,6 +909,17 @@ static bool saddr_is_tight(UnixSocketAddress *saddr)
>  #endif
>  }
>
> +#ifdef CONFIG_WIN32
> +static bool afunix_available(void)
> +{
> +    OSVERSIONINFOEXW os_version = { 0 };
> +
> +    os_get_win_version(&os_version);
> +
> +    return os_version.dwBuildNumber >= WIN_BUILD_AF_UNIX;
>

I think this is a bad variant to check feature support by checking
Windows build. From my point, you should try to create an AF_UNIX
socket and if it fails then fall back to the old behavior.


> +}
> +#endif
> +
>  static int unix_listen_saddr(UnixSocketAddress *saddr,
>                               int num,
>                               Error **errp)
> @@ -912,6 +932,13 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
>      size_t pathlen;
>      size_t addrlen;
>
> +#ifdef CONFIG_WIN32
> +    if (!afunix_available()) {
> +        error_setg(errp, "AF_UNIX is not available on your Windows");
> +        return -1;
> +    }
>
+#endif
> +
>      sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
>      if (sock < 0) {
>          error_setg_errno(errp, errno, "Failed to create Unix socket");
> @@ -1004,6 +1031,13 @@ static int unix_connect_saddr(UnixSocketAddress
> *saddr, Error **errp)
>          return -1;
>      }
>
> +#ifdef CONFIG_WIN32
> +    if (!afunix_available()) {
> +        error_setg(errp, "AF_UNIX is not available on your Windows");
> +        return -1;
> +    }
> +#endif
> +
>      sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
>      if (sock < 0) {
>          error_setg_errno(errp, errno, "Failed to create socket");
> @@ -1060,14 +1094,14 @@ static int unix_listen_saddr(UnixSocketAddress
> *saddr,
>                               int num,
>                               Error **errp)
>  {
> -    error_setg(errp, "unix sockets are not available on windows");
> +    error_setg(errp, "unix sockets are not available on your host");
>      errno = ENOTSUP;
>      return -1;
>  }
>
>  static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
>  {
> -    error_setg(errp, "unix sockets are not available on windows");
> +    error_setg(errp, "unix sockets are not available on your host");
>      errno = ENOTSUP;
>      return -1;
>  }
> @@ -1335,7 +1369,7 @@ socket_sockaddr_to_address_inet(struct
> sockaddr_storage *sa,
>  }
>
>
> -#ifndef WIN32
> +#ifdef CONFIG_AF_UNIX
>  static SocketAddress *
>  socket_sockaddr_to_address_unix(struct sockaddr_storage *sa,
>                                  socklen_t salen,
> @@ -1362,7 +1396,7 @@ socket_sockaddr_to_address_unix(struct
> sockaddr_storage *sa,
>      addr->u.q_unix.path = g_strndup(su->sun_path, salen);
>      return addr;
>  }
> -#endif /* WIN32 */
> +#endif /* CONFIG_AF_UNIX */
>
>  #ifdef CONFIG_AF_VSOCK
>  static SocketAddress *
> @@ -1394,10 +1428,10 @@ socket_sockaddr_to_address(struct sockaddr_storage
> *sa,
>      case AF_INET6:
>          return socket_sockaddr_to_address_inet(sa, salen, errp);
>
> -#ifndef WIN32
> +#ifdef CONFIG_AF_UNIX
>      case AF_UNIX:
>          return socket_sockaddr_to_address_unix(sa, salen, errp);
> -#endif /* WIN32 */
> +#endif
>
>  #ifdef CONFIG_AF_VSOCK
>      case AF_VSOCK:
> --
> 2.34.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 7856 bytes --]

Re: [PATCH 3/5] qga/commands-win32: Use os_get_win_version()

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 2208 bytes --]

On Wed, Jul 27, 2022 at 10:36 AM Bin Meng <bmeng.cn@gmail.com> wrote:

> From: Bin Meng <bin.meng@windriver.com>
>
> Drop its own ga_get_win_version() implementation, and use the one
> provided in oslib-win32 instead.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  qga/commands-win32.c | 27 +--------------------------
>  1 file changed, 1 insertion(+), 26 deletions(-)
>
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 7ed7664715..6186f2e1f2 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -2178,26 +2178,6 @@ static ga_win_10_0_t const
> WIN_10_0_CLIENT_VERSION_MATRIX[3] = {
>      {0, 0}
>  };
>
> -static void ga_get_win_version(RTL_OSVERSIONINFOEXW *info, Error **errp)
> -{
> -    typedef NTSTATUS(WINAPI *rtl_get_version_t)(
> -        RTL_OSVERSIONINFOEXW *os_version_info_ex);
> -
> -    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> -
> -    HMODULE module = GetModuleHandle("ntdll");
> -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> -    if (fun == NULL) {
> -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> -            "Failed to get address of RtlGetVersion");
> -        return;
> -    }
> -
> -    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> -    rtl_get_version(info);
> -    return;
> -}
> -
>  static char *ga_get_win_name(OSVERSIONINFOEXW const *os_version, bool id)
>  {
>      DWORD major = os_version->dwMajorVersion;
> @@ -2312,17 +2292,12 @@ static char *ga_get_current_arch(void)
>
>  GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
>  {
> -    Error *local_err = NULL;
>      OSVERSIONINFOEXW os_version = {0};
>      bool server;
>      char *product_name;
>      GuestOSInfo *info;
>
> -    ga_get_win_version(&os_version, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return NULL;
> -    }
> +    os_get_win_version(&os_version);
>

GA should not fail if it can't detect the Windows version but theoretically
os_get_win_version can crash application if GetProcAddress will return NULL


>
>      server = os_version.wProductType != VER_NT_WORKSTATION;
>      product_name = ga_get_win_product_name(errp);
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 3161 bytes --]

Re: [PATCH 0/5] Enable unix socket support on Windows

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 03:35:37PM +0800, Bin Meng wrote:
> Support for the unix socket has existed both in BSD and Linux for the
> longest time, but not on Windows. Since Windows 10 build 17063 [1],
> the native support for the unix socket has came to Windows. Starting
> this build, two Win32 processes can use the AF_UNIX address family
> over Winsock API to communicate with each other.
> 
> Introduce a new build time config option CONFIG_AF_UNIX when the build
> host has such a capability, and a run-time check afunix_available() for
> Windows host in the QEMU sockets util codes.
> 
> [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> 
> 
> Bin Meng (5):
>   util/qemu-sockets: Replace the call to close a socket with
>     closesocket()
>   util/oslib-win32: Add a helper to get the Windows version
>   qga/commands-win32: Use os_get_win_version()
>   util/qemu-sockets: Enable unix socket support on Windows
>   chardev/char-socket: Update AF_UNIX for Windows
> 
>  meson.build               |  6 +++++
>  include/sysemu/os-win32.h |  2 ++
>  chardev/char-socket.c     |  8 +++++-
>  qga/commands-win32.c      | 27 +-------------------
>  util/oslib-win32.c        | 15 +++++++++++
>  util/qemu-sockets.c       | 52 ++++++++++++++++++++++++++++++++-------
>  6 files changed, 74 insertions(+), 36 deletions(-)

What about net/socket.c ?

Also there are many tests using AF_UNIX and this doesn't appear to
have enablede any of them.  I'd at least exepct to see  the sockets
tests-io-channel-socket.c test enabled to prove this functionality
is working.

There are a few other AF_UNIX references in teh code, though many
seems to be Linux specific.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Bin Meng]

On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
>
> On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This adds a helper to get the Windows version via the RtlGetVersion
> > call, for QEMU codes to determine the Windows version at run-time.
> >
> > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  include/sysemu/os-win32.h |  2 ++
> >  util/oslib-win32.c        | 15 +++++++++++++++
> >  2 files changed, 17 insertions(+)
> >
> > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > index edc3b38a57..1e324026a4 100644
> > --- a/include/sysemu/os-win32.h
> > +++ b/include/sysemu/os-win32.h
> > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
> >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
> >                             struct sockaddr *addr, socklen_t *addrlen);
> >
> > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> > +
> >  #ifdef __cplusplus
> >  }
> >  #endif
> > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > index 5723d3eb4c..6d2387b9ff 100644
> > --- a/util/oslib-win32.c
> > +++ b/util/oslib-win32.c
> > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
> >       */
> >      return qemu_fdatasync(fd);
> >  }
> > +
> > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> > +{
> > +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> > +
> > +    /* RtlGetVersion is available starting with Windows 2000 */
> > +    HMODULE module = GetModuleHandle("ntdll");
> > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> > +
> > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> > +    rtl_get_version(info);
> The original function, when it was present in qemu-ga, tested that
> getting the function address succeeded.
> I think this test should be kept.
> See below:
> -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> -    if (fun == NULL) {
> -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> -            "Failed to get address of RtlGetVersion");
> -        return;
> -    }
>

Please see the comment:

/* RtlGetVersion is available starting with Windows 2000 */

I don't think we need that check.

Regards,
Bin

Re: [PATCH 4/5] util/qemu-sockets: Enable unix socket support on Windows

[Bin Meng]

On Wed, Jul 27, 2022 at 4:51 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
>
> On Wed, Jul 27, 2022 at 10:46 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Support for the unix socket has existed both in BSD and Linux for the
> > longest time, but not on Windows. Since Windows 10 build 17063 [1],
> > the native support for the unix socket has came to Windows. Starting
> > this build, two Win32 processes can use the AF_UNIX address family
> > over Winsock API to communicate with each other.
> >
> > Introduce a new build time config option CONFIG_AF_UNIX when the build
> > host has such a capability, and a run-time check afunix_available() for
> > Windows host in the QEMU sockets util codes.
> >
> > [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> >
> > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  meson.build         |  6 ++++++
> >  util/qemu-sockets.c | 48 ++++++++++++++++++++++++++++++++++++++-------
> >  2 files changed, 47 insertions(+), 7 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index 75aaca8462..73e5de5957 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -2327,6 +2327,12 @@ have_afalg = get_option('crypto_afalg') \
> >    '''), error_message: 'AF_ALG requested but could not be detected').allowed()
> >  config_host_data.set('CONFIG_AF_ALG', have_afalg)
> >
> > +if targetos != 'windows'
> > +  config_host_data.set('CONFIG_AF_UNIX', true)
> > +else
> > +  config_host_data.set('CONFIG_AF_UNIX', cc.has_header('afunix.h'))
> > +endif
> > +
> >  config_host_data.set('CONFIG_AF_VSOCK', cc.has_header_symbol(
> >    'linux/vm_sockets.h', 'AF_VSOCK',
> >    prefix: '#include <sys/socket.h>',
> > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > index 0e2298278f..d85f3ea3ee 100644
> > --- a/util/qemu-sockets.c
> > +++ b/util/qemu-sockets.c
> > @@ -17,6 +17,15 @@
> >   */
> >  #include "qemu/osdep.h"
> >
> > +#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
> > +# include <afunix.h>
> > +/*
> > + * AF_UNIX support is available since Windows 10 build 17063
> > + * See https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> > + */
> > +# define WIN_BUILD_AF_UNIX 17063
> > +#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
> > +
> >  #ifdef CONFIG_AF_VSOCK
> >  #include <linux/vm_sockets.h>
> >  #endif /* CONFIG_AF_VSOCK */
> > @@ -880,7 +889,7 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
> >  }
> >  #endif /* CONFIG_AF_VSOCK */
> >
> > -#ifndef _WIN32
> > +#ifdef CONFIG_AF_UNIX
> >
> >  static bool saddr_is_abstract(UnixSocketAddress *saddr)
> >  {
> > @@ -900,6 +909,17 @@ static bool saddr_is_tight(UnixSocketAddress *saddr)
> >  #endif
> >  }
> >
> > +#ifdef CONFIG_WIN32
> > +static bool afunix_available(void)
> > +{
> > +    OSVERSIONINFOEXW os_version = { 0 };
> > +
> > +    os_get_win_version(&os_version);
> > +
> > +    return os_version.dwBuildNumber >= WIN_BUILD_AF_UNIX;
> It can be that CONFIG_WIN32 is defined,but CONFIG_AF_UNIX is not. In
> this case WIN_BUILD_AF_UNIX will be undefined.
> Also, WIN_BUILD_AF_UNIX is just a build constant, why not define it
> always under CONFIG_WIN32?
>

Thanks for the review.

Will put WIN_BUILD_AF_UNIX under CONFIG_WIN32 only.

Regards,
Bin

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 05:38:27PM +0800, Bin Meng wrote:
> On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
> >
> > On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > This adds a helper to get the Windows version via the RtlGetVersion
> > > call, for QEMU codes to determine the Windows version at run-time.
> > >
> > > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > ---
> > >
> > >  include/sysemu/os-win32.h |  2 ++
> > >  util/oslib-win32.c        | 15 +++++++++++++++
> > >  2 files changed, 17 insertions(+)
> > >
> > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > > index edc3b38a57..1e324026a4 100644
> > > --- a/include/sysemu/os-win32.h
> > > +++ b/include/sysemu/os-win32.h
> > > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
> > >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
> > >                             struct sockaddr *addr, socklen_t *addrlen);
> > >
> > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> > > +
> > >  #ifdef __cplusplus
> > >  }
> > >  #endif
> > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > > index 5723d3eb4c..6d2387b9ff 100644
> > > --- a/util/oslib-win32.c
> > > +++ b/util/oslib-win32.c
> > > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
> > >       */
> > >      return qemu_fdatasync(fd);
> > >  }
> > > +
> > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> > > +{
> > > +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> > > +
> > > +    /* RtlGetVersion is available starting with Windows 2000 */
> > > +    HMODULE module = GetModuleHandle("ntdll");
> > > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> > > +
> > > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> > > +    rtl_get_version(info);
> > The original function, when it was present in qemu-ga, tested that
> > getting the function address succeeded.
> > I think this test should be kept.
> > See below:
> > -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > -    if (fun == NULL) {
> > -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> > -            "Failed to get address of RtlGetVersion");
> > -        return;
> > -    }
> >
> 
> Please see the comment:
> 
> /* RtlGetVersion is available starting with Windows 2000 */
> 
> I don't think we need that check.

In include/qemu/osdep.h we have 

/* as defined in sdkddkver.h */
#ifndef _WIN32_WINNT
#define _WIN32_WINNT 0x0601 /* Windows 7 API (should be in sync with glib) */
#endif

so do we even need to have the GetProcAddress calls at all ?

Surely we can just  '#include <ddk/ntddk.h>' and call
RtlGetVersion directly at compile time ?


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 4/5] util/qemu-sockets: Enable unix socket support on Windows

[Bin Meng]

On Wed, Jul 27, 2022 at 4:53 PM Konstantin Kostiuk <kkostiuk@redhat.com> wrote:
>
>
>
>
>
> On Wed, Jul 27, 2022 at 10:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> Support for the unix socket has existed both in BSD and Linux for the
>> longest time, but not on Windows. Since Windows 10 build 17063 [1],
>> the native support for the unix socket has came to Windows. Starting
>> this build, two Win32 processes can use the AF_UNIX address family
>> over Winsock API to communicate with each other.
>>
>> Introduce a new build time config option CONFIG_AF_UNIX when the build
>> host has such a capability, and a run-time check afunix_available() for
>> Windows host in the QEMU sockets util codes.
>>
>> [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
>>
>> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> ---
>>
>>  meson.build         |  6 ++++++
>>  util/qemu-sockets.c | 48 ++++++++++++++++++++++++++++++++++++++-------
>>  2 files changed, 47 insertions(+), 7 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index 75aaca8462..73e5de5957 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -2327,6 +2327,12 @@ have_afalg = get_option('crypto_afalg') \
>>    '''), error_message: 'AF_ALG requested but could not be detected').allowed()
>>  config_host_data.set('CONFIG_AF_ALG', have_afalg)
>>
>> +if targetos != 'windows'
>> +  config_host_data.set('CONFIG_AF_UNIX', true)
>> +else
>> +  config_host_data.set('CONFIG_AF_UNIX', cc.has_header('afunix.h'))
>> +endif
>> +
>>  config_host_data.set('CONFIG_AF_VSOCK', cc.has_header_symbol(
>>    'linux/vm_sockets.h', 'AF_VSOCK',
>>    prefix: '#include <sys/socket.h>',
>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
>> index 0e2298278f..d85f3ea3ee 100644
>> --- a/util/qemu-sockets.c
>> +++ b/util/qemu-sockets.c
>> @@ -17,6 +17,15 @@
>>   */
>>  #include "qemu/osdep.h"
>>
>> +#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
>> +# include <afunix.h>
>> +/*
>> + * AF_UNIX support is available since Windows 10 build 17063
>> + * See https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
>> + */
>> +# define WIN_BUILD_AF_UNIX 17063
>> +#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
>> +
>>  #ifdef CONFIG_AF_VSOCK
>>  #include <linux/vm_sockets.h>
>>  #endif /* CONFIG_AF_VSOCK */
>> @@ -880,7 +889,7 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
>>  }
>>  #endif /* CONFIG_AF_VSOCK */
>>
>> -#ifndef _WIN32
>> +#ifdef CONFIG_AF_UNIX
>>
>>  static bool saddr_is_abstract(UnixSocketAddress *saddr)
>>  {
>> @@ -900,6 +909,17 @@ static bool saddr_is_tight(UnixSocketAddress *saddr)
>>  #endif
>>  }
>>
>> +#ifdef CONFIG_WIN32
>> +static bool afunix_available(void)
>> +{
>> +    OSVERSIONINFOEXW os_version = { 0 };
>> +
>> +    os_get_win_version(&os_version);
>> +
>> +    return os_version.dwBuildNumber >= WIN_BUILD_AF_UNIX;
>
>
> I think this is a bad variant to check feature support by checking
> Windows build. From my point, you should try to create an AF_UNIX
> socket and if it fails then fall back to the old behavior.
>

The caller intends to create an AF_UNIX socket, and if Windows does
not have the capability, it fails, and we return -1 to the caller.
I am not sure what old behavior we should fall back to.

Regards,
Bin

Re: [PATCH 0/5] Enable unix socket support on Windows

[Bin Meng]

On Wed, Jul 27, 2022 at 5:06 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jul 27, 2022 at 03:35:37PM +0800, Bin Meng wrote:
> > Support for the unix socket has existed both in BSD and Linux for the
> > longest time, but not on Windows. Since Windows 10 build 17063 [1],
> > the native support for the unix socket has came to Windows. Starting
> > this build, two Win32 processes can use the AF_UNIX address family
> > over Winsock API to communicate with each other.
> >
> > Introduce a new build time config option CONFIG_AF_UNIX when the build
> > host has such a capability, and a run-time check afunix_available() for
> > Windows host in the QEMU sockets util codes.
> >
> > [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> >
> >
> > Bin Meng (5):
> >   util/qemu-sockets: Replace the call to close a socket with
> >     closesocket()
> >   util/oslib-win32: Add a helper to get the Windows version
> >   qga/commands-win32: Use os_get_win_version()
> >   util/qemu-sockets: Enable unix socket support on Windows
> >   chardev/char-socket: Update AF_UNIX for Windows
> >
> >  meson.build               |  6 +++++
> >  include/sysemu/os-win32.h |  2 ++
> >  chardev/char-socket.c     |  8 +++++-
> >  qga/commands-win32.c      | 27 +-------------------
> >  util/oslib-win32.c        | 15 +++++++++++
> >  util/qemu-sockets.c       | 52 ++++++++++++++++++++++++++++++++-------
> >  6 files changed, 74 insertions(+), 36 deletions(-)
>
> What about net/socket.c ?

It looks net/socket.c does not need to adapt.

> Also there are many tests using AF_UNIX and this doesn't appear to
> have enablede any of them.  I'd at least exepct to see  the sockets
> tests-io-channel-socket.c test enabled to prove this functionality
> is working.
>

Enabling qtest to run on Windows is underway but that's a separate
topic. The qtest itself is using unix socket so as long as we can run
qtest on Windows, we should be fine.
I feel this series is independent enough of being a standalone one.

> There are a few other AF_UNIX references in teh code, though many
> seems to be Linux specific.

Regards,
Bin

Re: [PATCH 0/5] Enable unix socket support on Windows

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 06:15:50PM +0800, Bin Meng wrote:
> On Wed, Jul 27, 2022 at 5:06 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Jul 27, 2022 at 03:35:37PM +0800, Bin Meng wrote:
> > > Support for the unix socket has existed both in BSD and Linux for the
> > > longest time, but not on Windows. Since Windows 10 build 17063 [1],
> > > the native support for the unix socket has came to Windows. Starting
> > > this build, two Win32 processes can use the AF_UNIX address family
> > > over Winsock API to communicate with each other.
> > >
> > > Introduce a new build time config option CONFIG_AF_UNIX when the build
> > > host has such a capability, and a run-time check afunix_available() for
> > > Windows host in the QEMU sockets util codes.
> > >
> > > [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> > >
> > >
> > > Bin Meng (5):
> > >   util/qemu-sockets: Replace the call to close a socket with
> > >     closesocket()
> > >   util/oslib-win32: Add a helper to get the Windows version
> > >   qga/commands-win32: Use os_get_win_version()
> > >   util/qemu-sockets: Enable unix socket support on Windows
> > >   chardev/char-socket: Update AF_UNIX for Windows
> > >
> > >  meson.build               |  6 +++++
> > >  include/sysemu/os-win32.h |  2 ++
> > >  chardev/char-socket.c     |  8 +++++-
> > >  qga/commands-win32.c      | 27 +-------------------
> > >  util/oslib-win32.c        | 15 +++++++++++
> > >  util/qemu-sockets.c       | 52 ++++++++++++++++++++++++++++++++-------
> > >  6 files changed, 74 insertions(+), 36 deletions(-)
> >
> > What about net/socket.c ?
> 
> It looks net/socket.c does not need to adapt.
> 
> > Also there are many tests using AF_UNIX and this doesn't appear to
> > have enablede any of them.  I'd at least exepct to see  the sockets
> > tests-io-channel-socket.c test enabled to prove this functionality
> > is working.
> >
> 
> Enabling qtest to run on Windows is underway but that's a separate
> topic. The qtest itself is using unix socket so as long as we can run
> qtest on Windows, we should be fine.
> I feel this series is independent enough of being a standalone one.

That isn't qtest, that is basic unit tests. I would expect those to
be able to work with this series 


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Yan Vugenfirer]

On Wed, Jul 27, 2022 at 1:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jul 27, 2022 at 05:38:27PM +0800, Bin Meng wrote:
> > On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
> > >
> > > On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > From: Bin Meng <bin.meng@windriver.com>
> > > >
> > > > This adds a helper to get the Windows version via the RtlGetVersion
> > > > call, for QEMU codes to determine the Windows version at run-time.
> > > >
> > > > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > ---
> > > >
> > > >  include/sysemu/os-win32.h |  2 ++
> > > >  util/oslib-win32.c        | 15 +++++++++++++++
> > > >  2 files changed, 17 insertions(+)
> > > >
> > > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > > > index edc3b38a57..1e324026a4 100644
> > > > --- a/include/sysemu/os-win32.h
> > > > +++ b/include/sysemu/os-win32.h
> > > > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
> > > >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
> > > >                             struct sockaddr *addr, socklen_t *addrlen);
> > > >
> > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> > > > +
> > > >  #ifdef __cplusplus
> > > >  }
> > > >  #endif
> > > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > > > index 5723d3eb4c..6d2387b9ff 100644
> > > > --- a/util/oslib-win32.c
> > > > +++ b/util/oslib-win32.c
> > > > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
> > > >       */
> > > >      return qemu_fdatasync(fd);
> > > >  }
> > > > +
> > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> > > > +{
> > > > +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> > > > +
> > > > +    /* RtlGetVersion is available starting with Windows 2000 */
> > > > +    HMODULE module = GetModuleHandle("ntdll");
> > > > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> > > > +
> > > > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> > > > +    rtl_get_version(info);
> > > The original function, when it was present in qemu-ga, tested that
> > > getting the function address succeeded.
> > > I think this test should be kept.
> > > See below:
> > > -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > -    if (fun == NULL) {
> > > -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> > > -            "Failed to get address of RtlGetVersion");
> > > -        return;
> > > -    }
> > >
> >
> > Please see the comment:
> >
> > /* RtlGetVersion is available starting with Windows 2000 */
> >
> > I don't think we need that check.
>
> In include/qemu/osdep.h we have
>
> /* as defined in sdkddkver.h */
> #ifndef _WIN32_WINNT
> #define _WIN32_WINNT 0x0601 /* Windows 7 API (should be in sync with glib) */
> #endif
>
> so do we even need to have the GetProcAddress calls at all ?
>
> Surely we can just  '#include <ddk/ntddk.h>' and call
> RtlGetVersion directly at compile time ?
Yes.
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlgetversion
- RtlGetVersion is available from Windows 2000.

Best regards,
Yan.

>
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>

Re: [PATCH 0/5] Enable unix socket support on Windows

[Bin Meng]

On Wed, Jul 27, 2022 at 6:24 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jul 27, 2022 at 06:15:50PM +0800, Bin Meng wrote:
> > On Wed, Jul 27, 2022 at 5:06 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Wed, Jul 27, 2022 at 03:35:37PM +0800, Bin Meng wrote:
> > > > Support for the unix socket has existed both in BSD and Linux for the
> > > > longest time, but not on Windows. Since Windows 10 build 17063 [1],
> > > > the native support for the unix socket has came to Windows. Starting
> > > > this build, two Win32 processes can use the AF_UNIX address family
> > > > over Winsock API to communicate with each other.
> > > >
> > > > Introduce a new build time config option CONFIG_AF_UNIX when the build
> > > > host has such a capability, and a run-time check afunix_available() for
> > > > Windows host in the QEMU sockets util codes.
> > > >
> > > > [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> > > >
> > > >
> > > > Bin Meng (5):
> > > >   util/qemu-sockets: Replace the call to close a socket with
> > > >     closesocket()
> > > >   util/oslib-win32: Add a helper to get the Windows version
> > > >   qga/commands-win32: Use os_get_win_version()
> > > >   util/qemu-sockets: Enable unix socket support on Windows
> > > >   chardev/char-socket: Update AF_UNIX for Windows
> > > >
> > > >  meson.build               |  6 +++++
> > > >  include/sysemu/os-win32.h |  2 ++
> > > >  chardev/char-socket.c     |  8 +++++-
> > > >  qga/commands-win32.c      | 27 +-------------------
> > > >  util/oslib-win32.c        | 15 +++++++++++
> > > >  util/qemu-sockets.c       | 52 ++++++++++++++++++++++++++++++++-------
> > > >  6 files changed, 74 insertions(+), 36 deletions(-)
> > >
> > > What about net/socket.c ?
> >
> > It looks net/socket.c does not need to adapt.
> >
> > > Also there are many tests using AF_UNIX and this doesn't appear to
> > > have enablede any of them.  I'd at least exepct to see  the sockets
> > > tests-io-channel-socket.c test enabled to prove this functionality
> > > is working.
> > >
> >
> > Enabling qtest to run on Windows is underway but that's a separate
> > topic. The qtest itself is using unix socket so as long as we can run
> > qtest on Windows, we should be fine.
> > I feel this series is independent enough of being a standalone one.
>
> That isn't qtest, that is basic unit tests. I would expect those to
> be able to work with this series
>

Ah, I see. Agreed, will do in v2.

Regards,
Bin

Re: [PATCH 0/5] Enable unix socket support on Windows

[Stefan Weil via]

Am 27.07.22 um 13:37 schrieb Bin Meng:

> On Wed, Jul 27, 2022 at 6:24 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>> That isn't qtest, that is basic unit tests. I would expect those to
>> be able to work with this series
> Ah, I see. Agreed, will do in v2.
>
> Regards,
> Bin


In v2 you might also call RtlGetVersion directly instead of getting the 
address (that is only needed if we want to support old Windows versions 
which don't provide that function).

Regards, Stefan

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Bin Meng]

On Wed, Jul 27, 2022 at 6:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jul 27, 2022 at 05:38:27PM +0800, Bin Meng wrote:
> > On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
> > >
> > > On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > From: Bin Meng <bin.meng@windriver.com>
> > > >
> > > > This adds a helper to get the Windows version via the RtlGetVersion
> > > > call, for QEMU codes to determine the Windows version at run-time.
> > > >
> > > > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > ---
> > > >
> > > >  include/sysemu/os-win32.h |  2 ++
> > > >  util/oslib-win32.c        | 15 +++++++++++++++
> > > >  2 files changed, 17 insertions(+)
> > > >
> > > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > > > index edc3b38a57..1e324026a4 100644
> > > > --- a/include/sysemu/os-win32.h
> > > > +++ b/include/sysemu/os-win32.h
> > > > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
> > > >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
> > > >                             struct sockaddr *addr, socklen_t *addrlen);
> > > >
> > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> > > > +
> > > >  #ifdef __cplusplus
> > > >  }
> > > >  #endif
> > > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > > > index 5723d3eb4c..6d2387b9ff 100644
> > > > --- a/util/oslib-win32.c
> > > > +++ b/util/oslib-win32.c
> > > > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
> > > >       */
> > > >      return qemu_fdatasync(fd);
> > > >  }
> > > > +
> > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> > > > +{
> > > > +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> > > > +
> > > > +    /* RtlGetVersion is available starting with Windows 2000 */
> > > > +    HMODULE module = GetModuleHandle("ntdll");
> > > > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> > > > +
> > > > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> > > > +    rtl_get_version(info);
> > > The original function, when it was present in qemu-ga, tested that
> > > getting the function address succeeded.
> > > I think this test should be kept.
> > > See below:
> > > -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > -    if (fun == NULL) {
> > > -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> > > -            "Failed to get address of RtlGetVersion");
> > > -        return;
> > > -    }
> > >
> >
> > Please see the comment:
> >
> > /* RtlGetVersion is available starting with Windows 2000 */
> >
> > I don't think we need that check.
>
> In include/qemu/osdep.h we have
>
> /* as defined in sdkddkver.h */
> #ifndef _WIN32_WINNT
> #define _WIN32_WINNT 0x0601 /* Windows 7 API (should be in sync with glib) */
> #endif
>
> so do we even need to have the GetProcAddress calls at all ?
>
> Surely we can just  '#include <ddk/ntddk.h>' and call
> RtlGetVersion directly at compile time ?
>

I believe #include <ddk/ntddk.h> is used in the kernel mode driver
programming environment.
In the user space we will have to use the ntdll exported symbol.

I cannot locate a Microsoft doc that tells us to call RtlGetVersion
directly in the user space.

Regards,
Bin

Re: [PATCH 0/5] Enable unix socket support on Windows

[Bin Meng]

On Wed, Jul 27, 2022 at 7:45 PM Stefan Weil <sw@weilnetz.de> wrote:
>
> Am 27.07.22 um 13:37 schrieb Bin Meng:
>
> > On Wed, Jul 27, 2022 at 6:24 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> That isn't qtest, that is basic unit tests. I would expect those to
> >> be able to work with this series
> > Ah, I see. Agreed, will do in v2.
> >
> > Regards,
> > Bin
>
>
> In v2 you might also call RtlGetVersion directly instead of getting the
> address (that is only needed if we want to support old Windows versions
> which don't provide that function).

I can't find a way to directly call RtlGetVersion. This API is
exported by ntdll so we have to do the indirect call via
GetProcAddress.

Regards,
Bin

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 07:55:40PM +0800, Bin Meng wrote:
> On Wed, Jul 27, 2022 at 6:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Jul 27, 2022 at 05:38:27PM +0800, Bin Meng wrote:
> > > On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
> > > >
> > > > On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > >
> > > > > This adds a helper to get the Windows version via the RtlGetVersion
> > > > > call, for QEMU codes to determine the Windows version at run-time.
> > > > >
> > > > > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > ---
> > > > >
> > > > >  include/sysemu/os-win32.h |  2 ++
> > > > >  util/oslib-win32.c        | 15 +++++++++++++++
> > > > >  2 files changed, 17 insertions(+)
> > > > >
> > > > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > > > > index edc3b38a57..1e324026a4 100644
> > > > > --- a/include/sysemu/os-win32.h
> > > > > +++ b/include/sysemu/os-win32.h
> > > > > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
> > > > >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
> > > > >                             struct sockaddr *addr, socklen_t *addrlen);
> > > > >
> > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> > > > > +
> > > > >  #ifdef __cplusplus
> > > > >  }
> > > > >  #endif
> > > > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > > > > index 5723d3eb4c..6d2387b9ff 100644
> > > > > --- a/util/oslib-win32.c
> > > > > +++ b/util/oslib-win32.c
> > > > > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
> > > > >       */
> > > > >      return qemu_fdatasync(fd);
> > > > >  }
> > > > > +
> > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> > > > > +{
> > > > > +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> > > > > +
> > > > > +    /* RtlGetVersion is available starting with Windows 2000 */
> > > > > +    HMODULE module = GetModuleHandle("ntdll");
> > > > > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> > > > > +
> > > > > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> > > > > +    rtl_get_version(info);
> > > > The original function, when it was present in qemu-ga, tested that
> > > > getting the function address succeeded.
> > > > I think this test should be kept.
> > > > See below:
> > > > -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > -    if (fun == NULL) {
> > > > -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> > > > -            "Failed to get address of RtlGetVersion");
> > > > -        return;
> > > > -    }
> > > >
> > >
> > > Please see the comment:
> > >
> > > /* RtlGetVersion is available starting with Windows 2000 */
> > >
> > > I don't think we need that check.
> >
> > In include/qemu/osdep.h we have
> >
> > /* as defined in sdkddkver.h */
> > #ifndef _WIN32_WINNT
> > #define _WIN32_WINNT 0x0601 /* Windows 7 API (should be in sync with glib) */
> > #endif
> >
> > so do we even need to have the GetProcAddress calls at all ?
> >
> > Surely we can just  '#include <ddk/ntddk.h>' and call
> > RtlGetVersion directly at compile time ?
> >
> 
> I believe #include <ddk/ntddk.h> is used in the kernel mode driver
> programming environment.
> In the user space we will have to use the ntdll exported symbol.
> 
> I cannot locate a Microsoft doc that tells us to call RtlGetVersion
> directly in the user space.

I wonder if we actually need to add this helper API to QEMU at all.

Would it be possible to use GLib 's  g_win32_check_windows_version API
instead ?

https://developer-old.gnome.org/glib/unstable/glib-Windows-Compatibility-Functions.html#g-win32-check-windows-version

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Bin Meng]

On Wed, Jul 27, 2022 at 8:53 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jul 27, 2022 at 07:55:40PM +0800, Bin Meng wrote:
> > On Wed, Jul 27, 2022 at 6:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Wed, Jul 27, 2022 at 05:38:27PM +0800, Bin Meng wrote:
> > > > On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
> > > > >
> > > > > On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > > >
> > > > > > This adds a helper to get the Windows version via the RtlGetVersion
> > > > > > call, for QEMU codes to determine the Windows version at run-time.
> > > > > >
> > > > > > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > > ---
> > > > > >
> > > > > >  include/sysemu/os-win32.h |  2 ++
> > > > > >  util/oslib-win32.c        | 15 +++++++++++++++
> > > > > >  2 files changed, 17 insertions(+)
> > > > > >
> > > > > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > > > > > index edc3b38a57..1e324026a4 100644
> > > > > > --- a/include/sysemu/os-win32.h
> > > > > > +++ b/include/sysemu/os-win32.h
> > > > > > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
> > > > > >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
> > > > > >                             struct sockaddr *addr, socklen_t *addrlen);
> > > > > >
> > > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> > > > > > +
> > > > > >  #ifdef __cplusplus
> > > > > >  }
> > > > > >  #endif
> > > > > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > > > > > index 5723d3eb4c..6d2387b9ff 100644
> > > > > > --- a/util/oslib-win32.c
> > > > > > +++ b/util/oslib-win32.c
> > > > > > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
> > > > > >       */
> > > > > >      return qemu_fdatasync(fd);
> > > > > >  }
> > > > > > +
> > > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> > > > > > +{
> > > > > > +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> > > > > > +
> > > > > > +    /* RtlGetVersion is available starting with Windows 2000 */
> > > > > > +    HMODULE module = GetModuleHandle("ntdll");
> > > > > > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > > > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> > > > > > +
> > > > > > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> > > > > > +    rtl_get_version(info);
> > > > > The original function, when it was present in qemu-ga, tested that
> > > > > getting the function address succeeded.
> > > > > I think this test should be kept.
> > > > > See below:
> > > > > -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > > -    if (fun == NULL) {
> > > > > -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> > > > > -            "Failed to get address of RtlGetVersion");
> > > > > -        return;
> > > > > -    }
> > > > >
> > > >
> > > > Please see the comment:
> > > >
> > > > /* RtlGetVersion is available starting with Windows 2000 */
> > > >
> > > > I don't think we need that check.
> > >
> > > In include/qemu/osdep.h we have
> > >
> > > /* as defined in sdkddkver.h */
> > > #ifndef _WIN32_WINNT
> > > #define _WIN32_WINNT 0x0601 /* Windows 7 API (should be in sync with glib) */
> > > #endif
> > >
> > > so do we even need to have the GetProcAddress calls at all ?
> > >
> > > Surely we can just  '#include <ddk/ntddk.h>' and call
> > > RtlGetVersion directly at compile time ?
> > >
> >
> > I believe #include <ddk/ntddk.h> is used in the kernel mode driver
> > programming environment.
> > In the user space we will have to use the ntdll exported symbol.
> >
> > I cannot locate a Microsoft doc that tells us to call RtlGetVersion
> > directly in the user space.
>
> I wonder if we actually need to add this helper API to QEMU at all.
>
> Would it be possible to use GLib 's  g_win32_check_windows_version API
> instead ?
>
> https://developer-old.gnome.org/glib/unstable/glib-Windows-Compatibility-Functions.html#g-win32-check-windows-version
>

I am afraid not. g_win32_check_windows_version API does not provide
detailed build number like RtlGetVersion does.

Regards,
Bin

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 4029 bytes --]

On Wed, Jul 27, 2022 at 2:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> On Wed, Jul 27, 2022 at 6:00 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> >
> > On Wed, Jul 27, 2022 at 05:38:27PM +0800, Bin Meng wrote:
> > > On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com>
> wrote:
> > > >
> > > > On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com>
> wrote:
> > > > >
> > > > > From: Bin Meng <bin.meng@windriver.com>
> > > > >
> > > > > This adds a helper to get the Windows version via the RtlGetVersion
> > > > > call, for QEMU codes to determine the Windows version at run-time.
> > > > >
> > > > > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > > > > ---
> > > > >
> > > > >  include/sysemu/os-win32.h |  2 ++
> > > > >  util/oslib-win32.c        | 15 +++++++++++++++
> > > > >  2 files changed, 17 insertions(+)
> > > > >
> > > > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> > > > > index edc3b38a57..1e324026a4 100644
> > > > > --- a/include/sysemu/os-win32.h
> > > > > +++ b/include/sysemu/os-win32.h
> > > > > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf,
> size_t len, int flags);
> > > > >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int
> flags,
> > > > >                             struct sockaddr *addr, socklen_t
> *addrlen);
> > > > >
> > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
> > > > > +
> > > > >  #ifdef __cplusplus
> > > > >  }
> > > > >  #endif
> > > > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > > > > index 5723d3eb4c..6d2387b9ff 100644
> > > > > --- a/util/oslib-win32.c
> > > > > +++ b/util/oslib-win32.c
> > > > > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int
> fd)
> > > > >       */
> > > > >      return qemu_fdatasync(fd);
> > > > >  }
> > > > > +
> > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
> > > > > +{
> > > > > +    typedef LONG (WINAPI
> *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
> > > > > +
> > > > > +    /* RtlGetVersion is available starting with Windows 2000 */
> > > > > +    HMODULE module = GetModuleHandle("ntdll");
> > > > > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
> > > > > +
> > > > > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
> > > > > +    rtl_get_version(info);
> > > > The original function, when it was present in qemu-ga, tested that
> > > > getting the function address succeeded.
> > > > I think this test should be kept.
> > > > See below:
> > > > -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
> > > > -    if (fun == NULL) {
> > > > -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
> > > > -            "Failed to get address of RtlGetVersion");
> > > > -        return;
> > > > -    }
> > > >
> > >
> > > Please see the comment:
> > >
> > > /* RtlGetVersion is available starting with Windows 2000 */
> > >
> > > I don't think we need that check.
> >
> > In include/qemu/osdep.h we have
> >
> > /* as defined in sdkddkver.h */
> > #ifndef _WIN32_WINNT
> > #define _WIN32_WINNT 0x0601 /* Windows 7 API (should be in sync with
> glib) */
> > #endif
> >
> > so do we even need to have the GetProcAddress calls at all ?
> >
> > Surely we can just  '#include <ddk/ntddk.h>' and call
> > RtlGetVersion directly at compile time ?
> >
>
> I believe #include <ddk/ntddk.h> is used in the kernel mode driver
> programming environment.
> In the user space we will have to use the ntdll exported symbol.
>
> I cannot locate a Microsoft doc that tells us to call RtlGetVersion
> directly in the user space.
>

From MS docs: RtlGetVersion is the kernel-mode equivalent of the user-mode
GetVersionEx function in the Windows SDK.
So you can use GetVersionEx instread.


>
> Regards,
> Bin
>
>

[-- Attachment #2: Type: text/html, Size: 5968 bytes --]

Re: [PATCH 2/5] util/oslib-win32: Add a helper to get the Windows version

[Bin Meng]

On Wed, Jul 27, 2022 at 9:18 PM Konstantin Kostiuk <kkostiuk@redhat.com> wrote:
>
>
>
> On Wed, Jul 27, 2022 at 2:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> On Wed, Jul 27, 2022 at 6:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>> >
>> > On Wed, Jul 27, 2022 at 05:38:27PM +0800, Bin Meng wrote:
>> > > On Wed, Jul 27, 2022 at 4:50 PM Yan Vugenfirer <yvugenfi@redhat.com> wrote:
>> > > >
>> > > > On Wed, Jul 27, 2022 at 10:43 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>> > > > >
>> > > > > From: Bin Meng <bin.meng@windriver.com>
>> > > > >
>> > > > > This adds a helper to get the Windows version via the RtlGetVersion
>> > > > > call, for QEMU codes to determine the Windows version at run-time.
>> > > > >
>> > > > > Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>> > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> > > > > ---
>> > > > >
>> > > > >  include/sysemu/os-win32.h |  2 ++
>> > > > >  util/oslib-win32.c        | 15 +++++++++++++++
>> > > > >  2 files changed, 17 insertions(+)
>> > > > >
>> > > > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
>> > > > > index edc3b38a57..1e324026a4 100644
>> > > > > --- a/include/sysemu/os-win32.h
>> > > > > +++ b/include/sysemu/os-win32.h
>> > > > > @@ -204,6 +204,8 @@ ssize_t qemu_recv_wrap(int sockfd, void *buf, size_t len, int flags);
>> > > > >  ssize_t qemu_recvfrom_wrap(int sockfd, void *buf, size_t len, int flags,
>> > > > >                             struct sockaddr *addr, socklen_t *addrlen);
>> > > > >
>> > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info);
>> > > > > +
>> > > > >  #ifdef __cplusplus
>> > > > >  }
>> > > > >  #endif
>> > > > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>> > > > > index 5723d3eb4c..6d2387b9ff 100644
>> > > > > --- a/util/oslib-win32.c
>> > > > > +++ b/util/oslib-win32.c
>> > > > > @@ -547,3 +547,18 @@ int qemu_msync(void *addr, size_t length, int fd)
>> > > > >       */
>> > > > >      return qemu_fdatasync(fd);
>> > > > >  }
>> > > > > +
>> > > > > +void os_get_win_version(RTL_OSVERSIONINFOEXW *info)
>> > > > > +{
>> > > > > +    typedef LONG (WINAPI *rtl_get_version_t)(PRTL_OSVERSIONINFOEXW);
>> > > > > +
>> > > > > +    /* RtlGetVersion is available starting with Windows 2000 */
>> > > > > +    HMODULE module = GetModuleHandle("ntdll");
>> > > > > +    PVOID fun = GetProcAddress(module, "RtlGetVersion");
>> > > > > +    rtl_get_version_t rtl_get_version = (rtl_get_version_t)fun;
>> > > > > +
>> > > > > +    info->dwOSVersionInfoSize = sizeof(RTL_OSVERSIONINFOEXW);
>> > > > > +    rtl_get_version(info);
>> > > > The original function, when it was present in qemu-ga, tested that
>> > > > getting the function address succeeded.
>> > > > I think this test should be kept.
>> > > > See below:
>> > > > -    PVOID fun = GetProcAddress(module, "RtlGetVersion");
>> > > > -    if (fun == NULL) {
>> > > > -        error_setg(errp, QERR_QGA_COMMAND_FAILED,
>> > > > -            "Failed to get address of RtlGetVersion");
>> > > > -        return;
>> > > > -    }
>> > > >
>> > >
>> > > Please see the comment:
>> > >
>> > > /* RtlGetVersion is available starting with Windows 2000 */
>> > >
>> > > I don't think we need that check.
>> >
>> > In include/qemu/osdep.h we have
>> >
>> > /* as defined in sdkddkver.h */
>> > #ifndef _WIN32_WINNT
>> > #define _WIN32_WINNT 0x0601 /* Windows 7 API (should be in sync with glib) */
>> > #endif
>> >
>> > so do we even need to have the GetProcAddress calls at all ?
>> >
>> > Surely we can just  '#include <ddk/ntddk.h>' and call
>> > RtlGetVersion directly at compile time ?
>> >
>>
>> I believe #include <ddk/ntddk.h> is used in the kernel mode driver
>> programming environment.
>> In the user space we will have to use the ntdll exported symbol.
>>
>> I cannot locate a Microsoft doc that tells us to call RtlGetVersion
>> directly in the user space.
>
>
> From MS docs: RtlGetVersion is the kernel-mode equivalent of the user-mode GetVersionEx function in the Windows SDK.
> So you can use GetVersionEx instread.
>

Unfortunately this does not work. GetVersionEx is deprecated since Windows 8.1

Regards,
Bin

Re: [PATCH 4/5] util/qemu-sockets: Enable unix socket support on Windows

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 4005 bytes --]

Hi

On Wed, Jul 27, 2022 at 2:05 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> On Wed, Jul 27, 2022 at 4:53 PM Konstantin Kostiuk <kkostiuk@redhat.com>
> wrote:
> >
> >
> >
> >
> >
> > On Wed, Jul 27, 2022 at 10:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >>
> >> From: Bin Meng <bin.meng@windriver.com>
> >>
> >> Support for the unix socket has existed both in BSD and Linux for the
> >> longest time, but not on Windows. Since Windows 10 build 17063 [1],
> >> the native support for the unix socket has came to Windows. Starting
> >> this build, two Win32 processes can use the AF_UNIX address family
> >> over Winsock API to communicate with each other.
> >>
> >> Introduce a new build time config option CONFIG_AF_UNIX when the build
> >> host has such a capability, and a run-time check afunix_available() for
> >> Windows host in the QEMU sockets util codes.
> >>
> >> [1]
> https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> >>
> >> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
> >> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >> ---
> >>
> >>  meson.build         |  6 ++++++
> >>  util/qemu-sockets.c | 48 ++++++++++++++++++++++++++++++++++++++-------
> >>  2 files changed, 47 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/meson.build b/meson.build
> >> index 75aaca8462..73e5de5957 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -2327,6 +2327,12 @@ have_afalg = get_option('crypto_afalg') \
> >>    '''), error_message: 'AF_ALG requested but could not be
> detected').allowed()
> >>  config_host_data.set('CONFIG_AF_ALG', have_afalg)
> >>
> >> +if targetos != 'windows'
> >> +  config_host_data.set('CONFIG_AF_UNIX', true)
> >> +else
> >> +  config_host_data.set('CONFIG_AF_UNIX', cc.has_header('afunix.h'))
> >> +endif
> >> +
> >>  config_host_data.set('CONFIG_AF_VSOCK', cc.has_header_symbol(
> >>    'linux/vm_sockets.h', 'AF_VSOCK',
> >>    prefix: '#include <sys/socket.h>',
> >> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> >> index 0e2298278f..d85f3ea3ee 100644
> >> --- a/util/qemu-sockets.c
> >> +++ b/util/qemu-sockets.c
> >> @@ -17,6 +17,15 @@
> >>   */
> >>  #include "qemu/osdep.h"
> >>
> >> +#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
> >> +# include <afunix.h>
> >> +/*
> >> + * AF_UNIX support is available since Windows 10 build 17063
> >> + * See
> https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
> >> + */
> >> +# define WIN_BUILD_AF_UNIX 17063
> >> +#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
> >> +
> >>  #ifdef CONFIG_AF_VSOCK
> >>  #include <linux/vm_sockets.h>
> >>  #endif /* CONFIG_AF_VSOCK */
> >> @@ -880,7 +889,7 @@ static int vsock_parse(VsockSocketAddress *addr,
> const char *str,
> >>  }
> >>  #endif /* CONFIG_AF_VSOCK */
> >>
> >> -#ifndef _WIN32
> >> +#ifdef CONFIG_AF_UNIX
> >>
> >>  static bool saddr_is_abstract(UnixSocketAddress *saddr)
> >>  {
> >> @@ -900,6 +909,17 @@ static bool saddr_is_tight(UnixSocketAddress
> *saddr)
> >>  #endif
> >>  }
> >>
> >> +#ifdef CONFIG_WIN32
> >> +static bool afunix_available(void)
> >> +{
> >> +    OSVERSIONINFOEXW os_version = { 0 };
> >> +
> >> +    os_get_win_version(&os_version);
> >> +
> >> +    return os_version.dwBuildNumber >= WIN_BUILD_AF_UNIX;
> >
> >
> > I think this is a bad variant to check feature support by checking
> > Windows build. From my point, you should try to create an AF_UNIX
> > socket and if it fails then fall back to the old behavior.
> >
>
> The caller intends to create an AF_UNIX socket, and if Windows does
> not have the capability, it fails, and we return -1 to the caller.
> I am not sure what old behavior we should fall back to.
>
>
I agree with Konstantin, we shouldn't check the Windows version, but assume
the socket creation can work, and just report a regular error if not.

(you can drop some of the preliminary patch then)

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 5863 bytes --]

Re: [PATCH 4/5] util/qemu-sockets: Enable unix socket support on Windows

[Bin Meng]

On Thu, Jul 28, 2022 at 9:11 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Wed, Jul 27, 2022 at 2:05 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> On Wed, Jul 27, 2022 at 4:53 PM Konstantin Kostiuk <kkostiuk@redhat.com> wrote:
>> >
>> >
>> >
>> >
>> >
>> > On Wed, Jul 27, 2022 at 10:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>> >>
>> >> From: Bin Meng <bin.meng@windriver.com>
>> >>
>> >> Support for the unix socket has existed both in BSD and Linux for the
>> >> longest time, but not on Windows. Since Windows 10 build 17063 [1],
>> >> the native support for the unix socket has came to Windows. Starting
>> >> this build, two Win32 processes can use the AF_UNIX address family
>> >> over Winsock API to communicate with each other.
>> >>
>> >> Introduce a new build time config option CONFIG_AF_UNIX when the build
>> >> host has such a capability, and a run-time check afunix_available() for
>> >> Windows host in the QEMU sockets util codes.
>> >>
>> >> [1] https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
>> >>
>> >> Signed-off-by: Xuzhou Cheng <xuzhou.cheng@windriver.com>
>> >> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> >> ---
>> >>
>> >>  meson.build         |  6 ++++++
>> >>  util/qemu-sockets.c | 48 ++++++++++++++++++++++++++++++++++++++-------
>> >>  2 files changed, 47 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/meson.build b/meson.build
>> >> index 75aaca8462..73e5de5957 100644
>> >> --- a/meson.build
>> >> +++ b/meson.build
>> >> @@ -2327,6 +2327,12 @@ have_afalg = get_option('crypto_afalg') \
>> >>    '''), error_message: 'AF_ALG requested but could not be detected').allowed()
>> >>  config_host_data.set('CONFIG_AF_ALG', have_afalg)
>> >>
>> >> +if targetos != 'windows'
>> >> +  config_host_data.set('CONFIG_AF_UNIX', true)
>> >> +else
>> >> +  config_host_data.set('CONFIG_AF_UNIX', cc.has_header('afunix.h'))
>> >> +endif
>> >> +
>> >>  config_host_data.set('CONFIG_AF_VSOCK', cc.has_header_symbol(
>> >>    'linux/vm_sockets.h', 'AF_VSOCK',
>> >>    prefix: '#include <sys/socket.h>',
>> >> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
>> >> index 0e2298278f..d85f3ea3ee 100644
>> >> --- a/util/qemu-sockets.c
>> >> +++ b/util/qemu-sockets.c
>> >> @@ -17,6 +17,15 @@
>> >>   */
>> >>  #include "qemu/osdep.h"
>> >>
>> >> +#if defined(CONFIG_WIN32) && defined(CONFIG_AF_UNIX)
>> >> +# include <afunix.h>
>> >> +/*
>> >> + * AF_UNIX support is available since Windows 10 build 17063
>> >> + * See https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
>> >> + */
>> >> +# define WIN_BUILD_AF_UNIX 17063
>> >> +#endif /* CONFIG_WIN32 && CONFIG_AF_UNIX */
>> >> +
>> >>  #ifdef CONFIG_AF_VSOCK
>> >>  #include <linux/vm_sockets.h>
>> >>  #endif /* CONFIG_AF_VSOCK */
>> >> @@ -880,7 +889,7 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
>> >>  }
>> >>  #endif /* CONFIG_AF_VSOCK */
>> >>
>> >> -#ifndef _WIN32
>> >> +#ifdef CONFIG_AF_UNIX
>> >>
>> >>  static bool saddr_is_abstract(UnixSocketAddress *saddr)
>> >>  {
>> >> @@ -900,6 +909,17 @@ static bool saddr_is_tight(UnixSocketAddress *saddr)
>> >>  #endif
>> >>  }
>> >>
>> >> +#ifdef CONFIG_WIN32
>> >> +static bool afunix_available(void)
>> >> +{
>> >> +    OSVERSIONINFOEXW os_version = { 0 };
>> >> +
>> >> +    os_get_win_version(&os_version);
>> >> +
>> >> +    return os_version.dwBuildNumber >= WIN_BUILD_AF_UNIX;
>> >
>> >
>> > I think this is a bad variant to check feature support by checking
>> > Windows build. From my point, you should try to create an AF_UNIX
>> > socket and if it fails then fall back to the old behavior.
>> >
>>
>> The caller intends to create an AF_UNIX socket, and if Windows does
>> not have the capability, it fails, and we return -1 to the caller.
>> I am not sure what old behavior we should fall back to.
>>
>
> I agree with Konstantin, we shouldn't check the Windows version, but assume the socket creation can work, and just report a regular error if not.
>
> (you can drop some of the preliminary patch then)
>

Sure, will do in v3.

Regards,
Bin