mbox series

[v1,0/2] introduce qemu_socketpiar()

Message ID cover.1661240709.git.tugy@chinatelecom.cn
Headers show
Series introduce qemu_socketpiar() | expand

Message

Guoyi Tu Aug. 23, 2022, 7:50 a.m. UTC
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):
  oslib-posix: 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      | 18 ++++++++++++++++++
 util/oslib-posix.c          | 19 +++++++++++++++++++
 4 files changed, 40 insertions(+), 2 deletions(-)

Comments

Guoyi Tu Sept. 5, 2022, 9:53 a.m. UTC | #1
Ping...

Any advises are welcome

On 8/23/22 15:50, tugy@chinatelecom.cn wrote:
> 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):
>    oslib-posix: 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      | 18 ++++++++++++++++++
>   util/oslib-posix.c          | 19 +++++++++++++++++++
>   4 files changed, 40 insertions(+), 2 deletions(-)
>
Marc-André Lureau Sept. 5, 2022, 11:19 a.m. UTC | #2
Hi

On Tue, Aug 23, 2022 at 12:00 PM <tugy@chinatelecom.cn> wrote:

> 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):
>   oslib-posix: Introduce qemu_socketpair()
>   vhost-user: Call qemu_socketpair() instead of socketpair()
>

Looks like a good idea to me. We will eventually extend the support for
win32 (as discussed in "[PATCH 19/51] tests/qtest: Build
test-filter-{mirror, redirector} cases for posix only").

There are other places where you can replace existing socketpair() calls in
the code base. Why not do it?

Current patches lgtm
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Christian Schoenebeck Sept. 5, 2022, 12:04 p.m. UTC | #3
On Dienstag, 23. August 2022 09:50:38 CEST tugy@chinatelecom.cn wrote:
> 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):
>   oslib-posix: 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      | 18 ++++++++++++++++++
>  util/oslib-posix.c          | 19 +++++++++++++++++++
>  4 files changed, 40 insertions(+), 2 deletions(-)

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Guoyi Tu Sept. 5, 2022, 12:27 p.m. UTC | #4
On 9/5/22 19:19, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Aug 23, 2022 at 12:00 PM <tugy@chinatelecom.cn 
> <mailto:tugy@chinatelecom.cn>> wrote:
> 
>     From: Guoyi Tu <tugy@chinatelecom.cn <mailto: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):
>        oslib-posix: Introduce qemu_socketpair()
>        vhost-user: Call qemu_socketpair() instead of socketpair()
> 
> 
> Looks like a good idea to me. We will eventually extend the support for 
> win32 (as discussed in "[PATCH 19/51] tests/qtest: Build 
> test-filter-{mirror, redirector} cases for posix only").
> 
> There are other places where you can replace existing socketpair() calls 
> in the code base. Why not do it?
> 
yeah, Thanks for reminding me, Maybe i can replace all the existing 
socketpair() calls in a separate patch if this patch can be merged to 
upstream.

--
Guoyi

> Current patches lgtm
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com 
> <mailto:marcandre.lureau@redhat.com>>
> 
> -- 
> Marc-André Lureau
Marc-André Lureau Sept. 5, 2022, 12:33 p.m. UTC | #5
Hi

On Mon, Sep 5, 2022 at 4:28 PM Guoyi Tu <tugy@chinatelecom.cn> wrote:

>
>
> On 9/5/22 19:19, Marc-André Lureau wrote:
> > Hi
> >
> > On Tue, Aug 23, 2022 at 12:00 PM <tugy@chinatelecom.cn
> > <mailto:tugy@chinatelecom.cn>> wrote:
> >
> >     From: Guoyi Tu <tugy@chinatelecom.cn <mailto: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):
> >        oslib-posix: Introduce qemu_socketpair()
> >        vhost-user: Call qemu_socketpair() instead of socketpair()
> >
> >
> > Looks like a good idea to me. We will eventually extend the support for
> > win32 (as discussed in "[PATCH 19/51] tests/qtest: Build
> > test-filter-{mirror, redirector} cases for posix only").
> >
> > There are other places where you can replace existing socketpair() calls
> > in the code base. Why not do it?
> >
> yeah, Thanks for reminding me, Maybe i can replace all the existing
> socketpair() calls in a separate patch if this patch can be merged to
> upstream.
>

It needs to be reviewed though, all may not want to set CLOEXEC, but most
should. Else, we should probably consider switching to GSpawn which does
dup or unset CLOEXEC for explicitely passed fds.


> --
> Guoyi
>
> > Current patches lgtm
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
> > <mailto:marcandre.lureau@redhat.com>>
> >
> > --
> > Marc-André Lureau
>
Guoyi Tu Sept. 5, 2022, 1 p.m. UTC | #6
On 9/5/22 20:33, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 5, 2022 at 4:28 PM Guoyi Tu <tugy@chinatelecom.cn 
> <mailto:tugy@chinatelecom.cn>> wrote:
> 
> 
> 
>     On 9/5/22 19:19, Marc-André Lureau wrote:
>      > Hi
>      >
>      > On Tue, Aug 23, 2022 at 12:00 PM <tugy@chinatelecom.cn
>     <mailto:tugy@chinatelecom.cn>
>      > <mailto:tugy@chinatelecom.cn <mailto:tugy@chinatelecom.cn>>> wrote:
>      >
>      >     From: Guoyi Tu <tugy@chinatelecom.cn
>     <mailto:tugy@chinatelecom.cn> <mailto:tugy@chinatelecom.cn
>     <mailto: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):
>      >        oslib-posix: Introduce qemu_socketpair()
>      >        vhost-user: Call qemu_socketpair() instead of socketpair()
>      >
>      >
>      > Looks like a good idea to me. We will eventually extend the
>     support for
>      > win32 (as discussed in "[PATCH 19/51] tests/qtest: Build
>      > test-filter-{mirror, redirector} cases for posix only").
>      >
>      > There are other places where you can replace existing
>     socketpair() calls
>      > in the code base. Why not do it?
>      >
>     yeah, Thanks for reminding me, Maybe i can replace all the existing
>     socketpair() calls in a separate patch if this patch can be merged to
>     upstream.
> 
> 
> It needs to be reviewed though, all may not want to set CLOEXEC, but 
> most should. Else, we should probably consider switching to GSpawn which 
> does dup or unset CLOEXEC for explicitely passed fds.

Got it. Special care will be taken when replace those functions.

Thanks again.

--
Guoyi
>     --
>     Guoyi
> 
>      > Current patches lgtm
>      > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
>     <mailto:marcandre.lureau@redhat.com>
>      > <mailto:marcandre.lureau@redhat.com
>     <mailto:marcandre.lureau@redhat.com>>>
>      >
>      > --
>      > Marc-André Lureau
> 
> 
> 
> -- 
> Marc-André Lureau
Guoyi Tu Sept. 19, 2022, 7:20 a.m. UTC | #7
Hi Peter,

what do you think about this patches? If this could be merged in 
upstream, i will start to update the test codes

On 8/23/22 15:50, tugy@chinatelecom.cn wrote:
> 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):
>    oslib-posix: 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      | 18 ++++++++++++++++++
>   util/oslib-posix.c          | 19 +++++++++++++++++++
>   4 files changed, 40 insertions(+), 2 deletions(-)
>