[PATCH 0/2] introduce qemu_socketpiar()

[PATCH 0/2] introduce qemu_socketpiar()

[tugy]

From: Guoyi Tu <tugy@chinatelecom.cn>

introduce qemu_socketpair() to create socket pair fd, and 
set the close-on-exec flag at default as with the other type
of socket does.

besides, the live update feature is developing, so it's necessary
to do that.

Guoyi Tu (2):
  osdeps: Introduce qemu_socketpair()
  vhost-user: Call qemu_socketpair() instead of socketpair()

 hw/display/vhost-user-gpu.c |  3 ++-
 hw/virtio/vhost-user.c      |  2 +-
 include/qemu/sockets.h      |  3 +++
 util/osdep.c                | 24 ++++++++++++++++++++++++
 4 files changed, 30 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 1/2] osdeps: Introduce qemu_socketpair()

[tugy]

From: Guoyi Tu <tugy@chinatelecom.cn>

qemu_socketpair() will create a pair of connected sockets
with FD_CLOEXEC set

Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
---
 include/qemu/sockets.h |  3 +++
 util/osdep.c           | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 038faa157f..52cf2855df 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -14,6 +14,9 @@ int inet_aton(const char *cp, struct in_addr *ia);
 /* misc helpers */
 bool fd_is_socket(int fd);
 int qemu_socket(int domain, int type, int protocol);
+#ifndef WIN32
+int qemu_socketpair(int domain, int type, int protocol, int sv[2]);
+#endif
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
diff --git a/util/osdep.c b/util/osdep.c
index 60fcbbaebe..4b1ab623c7 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -481,6 +481,30 @@ int qemu_socket(int domain, int type, int protocol)
     return ret;
 }
 
+#ifndef _WIN32
+/*
+ * Create a pair of connected sockets with FD_CLOEXEC set
+ */
+int qemu_socketpair(int domain, int type, int protocol, int sv[2])
+{
+    int ret;
+
+#ifdef SOCK_CLOEXEC
+    ret = socketpair(domain, type | SOCK_CLOEXEC, protocol, sv);
+    if (ret != -1 || errno != EINVAL) {
+        return ret;
+    }
+#endif
+    ret = socketpair(domain, type, protocol, sv);;
+    if (ret == 0) {
+        qemu_set_cloexec(sv[0]);
+        qemu_set_cloexec(sv[1]);
+    }
+
+    return ret;
+}
+#endif
+
 /*
  * Accept a connection and set FD_CLOEXEC
  */
-- 
2.25.1

[PATCH 2/2] vhost-user: Call qemu_socketpair() instead of socketpair()

[tugy]

From: Guoyi Tu <tugy@chinatelecom.cn>

set close-on-exec flag on the new opened file descriptors at default

Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
---
 hw/display/vhost-user-gpu.c | 3 ++-
 hw/virtio/vhost-user.c      | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
index 3340ef9e5f..19c0e20103 100644
--- a/hw/display/vhost-user-gpu.c
+++ b/hw/display/vhost-user-gpu.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/sockets.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio-gpu.h"
 #include "chardev/char-fe.h"
@@ -375,7 +376,7 @@ vhost_user_gpu_do_set_socket(VhostUserGPU *g, Error **errp)
     Chardev *chr;
     int sv[2];
 
-    if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
+    if (qemu_socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
         error_setg_errno(errp, errno, "socketpair() failed");
         return false;
     }
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 75b8df21a4..4d2b227028 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1726,7 +1726,7 @@ static int vhost_setup_slave_channel(struct vhost_dev *dev)
         return 0;
     }
 
-    if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
+    if (qemu_socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
         int saved_errno = errno;
         error_report("socketpair() failed");
         return -saved_errno;
-- 
2.25.1

Re: [PATCH 1/2] osdeps: Introduce qemu_socketpair()

[Peter Maydell]

On Fri, 12 Aug 2022 at 12:44, <tugy@chinatelecom.cn> wrote:
>
> From: Guoyi Tu <tugy@chinatelecom.cn>
>
> qemu_socketpair() will create a pair of connected sockets
> with FD_CLOEXEC set
>
> Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
> ---
>  include/qemu/sockets.h |  3 +++
>  util/osdep.c           | 24 ++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 038faa157f..52cf2855df 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -14,6 +14,9 @@ int inet_aton(const char *cp, struct in_addr *ia);
>  /* misc helpers */
>  bool fd_is_socket(int fd);
>  int qemu_socket(int domain, int type, int protocol);
> +#ifndef WIN32
> +int qemu_socketpair(int domain, int type, int protocol, int sv[2]);

Any new function declaration in a header file needs a
doc-comment documenting what it does, please.

> +#endif
>  int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
>  int socket_set_cork(int fd, int v);
>  int socket_set_nodelay(int fd);
> diff --git a/util/osdep.c b/util/osdep.c
> index 60fcbbaebe..4b1ab623c7 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -481,6 +481,30 @@ int qemu_socket(int domain, int type, int protocol)
>      return ret;
>  }
>
> +#ifndef _WIN32

If this function only exists and is usable on posix
hosts, put it in util/oslib-posix.c rather than having
it here with a win32 ifdef.

thanks
-- PMM

Re: [PATCH 2/2] vhost-user: Call qemu_socketpair() instead of socketpair()

[Peter Maydell]

On Fri, 12 Aug 2022 at 12:44, <tugy@chinatelecom.cn> wrote:
>
> From: Guoyi Tu <tugy@chinatelecom.cn>
>
> set close-on-exec flag on the new opened file descriptors at default

What goes wrong if we don't do this? The commit message
is a good place to explain what bug the commit is fixing,
and its consequences.

thanks
-- PMM

Re: [PATCH 1/2] osdeps: Introduce qemu_socketpair()

[Guoyi Tu]

On 8/12/22 19:49, Peter Maydell wrote:
> On Fri, 12 Aug 2022 at 12:44, <tugy@chinatelecom.cn> wrote:
>>
>> From: Guoyi Tu <tugy@chinatelecom.cn>
>>
>> qemu_socketpair() will create a pair of connected sockets
>> with FD_CLOEXEC set
>>
>> Signed-off-by: Guoyi Tu <tugy@chinatelecom.cn>
>> ---
>>   include/qemu/sockets.h |  3 +++
>>   util/osdep.c           | 24 ++++++++++++++++++++++++
>>   2 files changed, 27 insertions(+)
>>
>> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
>> index 038faa157f..52cf2855df 100644
>> --- a/include/qemu/sockets.h
>> +++ b/include/qemu/sockets.h
>> @@ -14,6 +14,9 @@ int inet_aton(const char *cp, struct in_addr *ia);
>>   /* misc helpers */
>>   bool fd_is_socket(int fd);
>>   int qemu_socket(int domain, int type, int protocol);
>> +#ifndef WIN32
>> +int qemu_socketpair(int domain, int type, int protocol, int sv[2]);
> 
> Any new function declaration in a header file needs a
> doc-comment documenting what it does, please.

OK, I'll add some comments

>> +#endif
>>   int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
>>   int socket_set_cork(int fd, int v);
>>   int socket_set_nodelay(int fd);
>> diff --git a/util/osdep.c b/util/osdep.c
>> index 60fcbbaebe..4b1ab623c7 100644
>> --- a/util/osdep.c
>> +++ b/util/osdep.c
>> @@ -481,6 +481,30 @@ int qemu_socket(int domain, int type, int protocol)
>>       return ret;
>>   }
>>
>> +#ifndef _WIN32
> 
> If this function only exists and is usable on posix
> hosts, put it in util/oslib-posix.c rather than having
> it here with a win32 ifdef.
> 
will do

Thanks.

> thanks
> -- PMM
> 

Re: [PATCH 2/2] vhost-user: Call qemu_socketpair() instead of socketpair()

[Guoyi Tu]

On 8/12/22 19:50, Peter Maydell wrote:
> On Fri, 12 Aug 2022 at 12:44, <tugy@chinatelecom.cn> wrote:
>>
>> From: Guoyi Tu <tugy@chinatelecom.cn>
>>
>> set close-on-exec flag on the new opened file descriptors at default
> 
> What goes wrong if we don't do this? The commit message
> is a good place to explain what bug the commit is fixing,
> and its consequences.
> 
> thanks
> -- PMM
> 
OK, will do and send another version

Thanks.