diff mbox series

[v3,13/16] slirp: open-code qemu_socket_(un)select()

Message ID 20230221124802.4103554-14-marcandre.lureau@redhat.com
State New
Headers show
Series win32: do not mix SOCKET and fd space | expand

Commit Message

Marc-André Lureau Feb. 21, 2023, 12:47 p.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

We are about to make the QEMU socket API use file-descriptor space only,
but libslirp gives us SOCKET as fd, still.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 net/slirp.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Stefan Berger March 6, 2023, 1:59 p.m. UTC | #1 
On 2/21/23 07:47, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> We are about to make the QEMU socket API use file-descriptor space only,
> but libslirp gives us SOCKET as fd, still.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   net/slirp.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index a7c35778a6..c33b3e02e7 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)

Shouldn't this int fd rather be a SOCKET s instead? Or do you get compiler warnings then?

>   #ifdef WIN32
>       AioContext *ctxt = qemu_get_aio_context();
> 
> -    qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
> +    if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
>                          FD_READ | FD_ACCEPT | FD_CLOSE |
> -                       FD_CONNECT | FD_WRITE | FD_OOB, NULL);
> +                       FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
> +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
> +    }
>   #endif
>   }
> >   static void net_slirp_unregister_poll_fd(int fd, void *opaque)

Same here.

>   {
>   #ifdef WIN32
> -    qemu_socket_unselect(fd, NULL);
> +    if (WSAEventSelect(fd, NULL, 0) != 0) {
> +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
> +    }
>   #endif
>   }
>
Marc-André Lureau March 6, 2023, 2:03 p.m. UTC | #2 
Hi

On Mon, Mar 6, 2023 at 5:59 PM Stefan Berger <stefanb@linux.ibm.com> wrote:

>
>
> On 2/21/23 07:47, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > We are about to make the QEMU socket API use file-descriptor space only,
> > but libslirp gives us SOCKET as fd, still.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   net/slirp.c | 10 +++++++---
> >   1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/slirp.c b/net/slirp.c
> > index a7c35778a6..c33b3e02e7 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd,
> void *opaque)
>
> Shouldn't this int fd rather be a SOCKET s instead? Or do you get compiler
> warnings then?
>
>
Yes, you would get compiler warnings, because the "int fd" argument is from
the slirp API, whether it is posix or win32.


> >   #ifdef WIN32
> >       AioContext *ctxt = qemu_get_aio_context();
> >
> > -    qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
> > +    if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
> >                          FD_READ | FD_ACCEPT | FD_CLOSE |
> > -                       FD_CONNECT | FD_WRITE | FD_OOB, NULL);
> > +                       FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
> > +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to
> WSAEventSelect()");
> > +    }
> >   #endif
> >   }
> > >   static void net_slirp_unregister_poll_fd(int fd, void *opaque)
>
> Same here.
>
> >   {
> >   #ifdef WIN32
> > -    qemu_socket_unselect(fd, NULL);
> > +    if (WSAEventSelect(fd, NULL, 0) != 0) {
> > +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to
> WSAEventSelect()");
> > +    }
> >   #endif
> >   }
> >
>
>
Stefan Berger March 6, 2023, 2:16 p.m. UTC | #3 
On 2/21/23 07:47, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> We are about to make the QEMU socket API use file-descriptor space only,
> but libslirp gives us SOCKET as fd, still.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   net/slirp.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index a7c35778a6..c33b3e02e7 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)
>   #ifdef WIN32
>       AioContext *ctxt = qemu_get_aio_context();
> 
> -    qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
> +    if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
>                          FD_READ | FD_ACCEPT | FD_CLOSE |
> -                       FD_CONNECT | FD_WRITE | FD_OOB, NULL);
> +                       FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
> +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
> +    }
>   #endif
>   }
> 
>   static void net_slirp_unregister_poll_fd(int fd, void *opaque)
>   {
>   #ifdef WIN32
> -    qemu_socket_unselect(fd, NULL);
> +    if (WSAEventSelect(fd, NULL, 0) != 0) {
> +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
> +    }
>   #endif
>   }
> 

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Stefan Berger March 6, 2023, 2:47 p.m. UTC | #4 
On 3/6/23 09:03, Marc-André Lureau wrote:

> 
> On Mon, Mar 6, 2023 at 5:59 PM Stefan Berger <stefanb@linux.ibm.com <mailto:stefanb@linux.ibm.com>> wrote:
> 
> 
> 
>     On 2/21/23 07:47, marcandre.lureau@redhat.com <mailto:marcandre.lureau@redhat.com> wrote:
>      > From: Marc-André Lureau <marcandre.lureau@redhat.com <mailto:marcandre.lureau@redhat.com>>
>      >
>      > We are about to make the QEMU socket API use file-descriptor space only,
>      > but libslirp gives us SOCKET as fd, still.
>      >
>      > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com <mailto:marcandre.lureau@redhat.com>>
>      > ---
>      >   net/slirp.c | 10 +++++++---
>      >   1 file changed, 7 insertions(+), 3 deletions(-)
>      >
>      > diff --git a/net/slirp.c b/net/slirp.c
>      > index a7c35778a6..c33b3e02e7 100644
>      > --- a/net/slirp.c
>      > +++ b/net/slirp.c
>      > @@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)
> 
>     Shouldn't this int fd rather be a SOCKET s instead? Or do you get compiler warnings then?
> 
> 
> Yes, you would get compiler warnings, because the "int fd" argument is from the slirp API, whether it is posix or win32.


Right, this is shared code.

> 
>      >   #ifdef WIN32
>      >       AioContext *ctxt = qemu_get_aio_context();
>      >
>      > -    qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
>      > +    if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
>      >                          FD_READ | FD_ACCEPT | FD_CLOSE |
>      > -                       FD_CONNECT | FD_WRITE | FD_OOB, NULL);
>      > +                       FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
>      > +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
>      > +    }
>      >   #endif
>      >   }
>      > >   static void net_slirp_unregister_poll_fd(int fd, void *opaque)
> 
>     Same here.
> 
>      >   {
>      >   #ifdef WIN32
>      > -    qemu_socket_unselect(fd, NULL);
>      > +    if (WSAEventSelect(fd, NULL, 0) != 0) {
>      > +        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
>      > +    }
>      >   #endif
>      >   }
>      >
>
diff mbox series

Patch

diff --git a/net/slirp.c b/net/slirp.c
index a7c35778a6..c33b3e02e7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -251,16 +251,20 @@  static void net_slirp_register_poll_fd(int fd, void *opaque)
 #ifdef WIN32
     AioContext *ctxt = qemu_get_aio_context();
 
-    qemu_socket_select(fd, event_notifier_get_handle(&ctxt->notifier),
+    if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
                        FD_READ | FD_ACCEPT | FD_CLOSE |
-                       FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+                       FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
+        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
+    }
 #endif
 }
 
 static void net_slirp_unregister_poll_fd(int fd, void *opaque)
 {
 #ifdef WIN32
-    qemu_socket_unselect(fd, NULL);
+    if (WSAEventSelect(fd, NULL, 0) != 0) {
+        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
+    }
 #endif
 }