diff mbox series

net/slirp: introduce slirp_os_socket to stay compatible with libslirp past 4.8.0

Message ID 20241005070753.1328079-1-mjt@tls.msk.ru (mailing list archive)
State New, archived
Headers show
Series net/slirp: introduce slirp_os_socket to stay compatible with libslirp past 4.8.0 | expand

Commit Message

Michael Tokarev Oct. 5, 2024, 7:07 a.m. UTC
libslirp introduced new typedef after 4.8.0, slirp_os_socket, which
is defined to SOCKET on windows, which, in turn, is a 64bit number.
qemu uses int, so callback function prorotypes changed.  Introduce
slirp_os_socket locally if SLIRP_INVALID_SOCKET is not defined (this
define has been introduced together wiht slirp_os_socket type), for
libslirp <= 4.8.0, and use it in callback function definitions.

Link: https://gitlab.freedesktop.org/slirp/libslirp/-/commit/72f85005a2307fd0961543e3cea861ad7a4d201e
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2603
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 net/slirp.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Samuel Thibault Oct. 9, 2024, 11:06 p.m. UTC | #1
Hello,

Michael Tokarev, le sam. 05 oct. 2024 10:07:53 +0300, a ecrit:
> libslirp introduced new typedef after 4.8.0, slirp_os_socket, which
> is defined to SOCKET on windows, which, in turn, is a 64bit number.
> qemu uses int, so callback function prorotypes changed.

I have fixed the code in upstream libslirp, to avoid breaking the API
and ABI, and instead provide new functions & methods so that
qemu/libslirp can upgrade smoothly.

> Introduce
> slirp_os_socket locally if SLIRP_INVALID_SOCKET is not defined (this
> define has been introduced together wiht slirp_os_socket type), for
> libslirp <= 4.8.0, and use it in callback function definitions.
> 
> Link: https://gitlab.freedesktop.org/slirp/libslirp/-/commit/72f85005a2307fd0961543e3cea861ad7a4d201e
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2603
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  net/slirp.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index eb9a456ed4..fa07268cf4 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -98,6 +98,10 @@ typedef struct SlirpState {
>      GSList *fwd;
>  } SlirpState;
>  
> +#ifndef SLIRP_INVALID_SOCKET /* after 4.8.0 */

You can instead use SLIRP_CONFIG_VERSION_MAX >= 6, and if so set
cfg.version to 6 and use the new fields and functions.

> +typedef int slirp_os_socket;
> +#endif
> +
>  static struct slirp_config_str *slirp_configs;
>  static QTAILQ_HEAD(, SlirpState) slirp_stacks =
>      QTAILQ_HEAD_INITIALIZER(slirp_stacks);
> @@ -247,7 +251,7 @@ static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
>      timer_mod(&t->timer, expire_timer);
>  }
>  
> -static void net_slirp_register_poll_fd(int fd, void *opaque)
> +static void net_slirp_register_poll_fd(slirp_os_socket fd, void *opaque)
>  {
>  #ifdef WIN32
>      AioContext *ctxt = qemu_get_aio_context();
> @@ -260,7 +264,7 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)
>  #endif
>  }
>  
> -static void net_slirp_unregister_poll_fd(int fd, void *opaque)
> +static void net_slirp_unregister_poll_fd(slirp_os_socket fd, void *opaque)
>  {
>  #ifdef WIN32
>      if (WSAEventSelect(fd, NULL, 0) != 0) {
> @@ -314,7 +318,7 @@ static int slirp_poll_to_gio(int events)
>      return ret;
>  }
>  
> -static int net_slirp_add_poll(int fd, int events, void *opaque)
> +static int net_slirp_add_poll(slirp_os_socket fd, int events, void *opaque)
>  {
>      GArray *pollfds = opaque;
>      GPollFD pfd = {
> -- 
> 2.39.5
Samuel Thibault Jan. 30, 2025, 2:13 a.m. UTC | #2
Hello,

Samuel Thibault, le jeu. 10 oct. 2024 01:06:47 +0200, a ecrit:
> Michael Tokarev, le sam. 05 oct. 2024 10:07:53 +0300, a ecrit:
> > libslirp introduced new typedef after 4.8.0, slirp_os_socket, which
> > is defined to SOCKET on windows, which, in turn, is a 64bit number.
> > qemu uses int, so callback function prorotypes changed.
> 
> I have fixed the code in upstream libslirp, to avoid breaking the API
> and ABI, and instead provide new functions & methods so that
> qemu/libslirp can upgrade smoothly.

It is now released in version 4.9.0 of libslirp.

Samuel

> > Introduce
> > slirp_os_socket locally if SLIRP_INVALID_SOCKET is not defined (this
> > define has been introduced together wiht slirp_os_socket type), for
> > libslirp <= 4.8.0, and use it in callback function definitions.
> > 
> > Link: https://gitlab.freedesktop.org/slirp/libslirp/-/commit/72f85005a2307fd0961543e3cea861ad7a4d201e
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2603
> > Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> > ---
> >  net/slirp.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/slirp.c b/net/slirp.c
> > index eb9a456ed4..fa07268cf4 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -98,6 +98,10 @@ typedef struct SlirpState {
> >      GSList *fwd;
> >  } SlirpState;
> >  
> > +#ifndef SLIRP_INVALID_SOCKET /* after 4.8.0 */
> 
> You can instead use SLIRP_CONFIG_VERSION_MAX >= 6, and if so set
> cfg.version to 6 and use the new fields and functions.
> 
> > +typedef int slirp_os_socket;
> > +#endif
> > +
> >  static struct slirp_config_str *slirp_configs;
> >  static QTAILQ_HEAD(, SlirpState) slirp_stacks =
> >      QTAILQ_HEAD_INITIALIZER(slirp_stacks);
> > @@ -247,7 +251,7 @@ static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
> >      timer_mod(&t->timer, expire_timer);
> >  }
> >  
> > -static void net_slirp_register_poll_fd(int fd, void *opaque)
> > +static void net_slirp_register_poll_fd(slirp_os_socket fd, void *opaque)
> >  {
> >  #ifdef WIN32
> >      AioContext *ctxt = qemu_get_aio_context();
> > @@ -260,7 +264,7 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)
> >  #endif
> >  }
> >  
> > -static void net_slirp_unregister_poll_fd(int fd, void *opaque)
> > +static void net_slirp_unregister_poll_fd(slirp_os_socket fd, void *opaque)
> >  {
> >  #ifdef WIN32
> >      if (WSAEventSelect(fd, NULL, 0) != 0) {
> > @@ -314,7 +318,7 @@ static int slirp_poll_to_gio(int events)
> >      return ret;
> >  }
> >  
> > -static int net_slirp_add_poll(int fd, int events, void *opaque)
> > +static int net_slirp_add_poll(slirp_os_socket fd, int events, void *opaque)
> >  {
> >      GArray *pollfds = opaque;
> >      GPollFD pfd = {
> > -- 
> > 2.39.5
Thomas Huth Jan. 30, 2025, 10:09 a.m. UTC | #3
On 30/01/2025 03.13, Samuel Thibault wrote:
> Hello,
> 
> Samuel Thibault, le jeu. 10 oct. 2024 01:06:47 +0200, a ecrit:
>> Michael Tokarev, le sam. 05 oct. 2024 10:07:53 +0300, a ecrit:
>>> libslirp introduced new typedef after 4.8.0, slirp_os_socket, which
>>> is defined to SOCKET on windows, which, in turn, is a 64bit number.
>>> qemu uses int, so callback function prorotypes changed.
>>
>> I have fixed the code in upstream libslirp, to avoid breaking the API
>> and ABI, and instead provide new functions & methods so that
>> qemu/libslirp can upgrade smoothly.
> 
> It is now released in version 4.9.0 of libslirp.

Seems like 4.9.0 breaks the CI:

https://gitlab.com/thuth/qemu/-/jobs/8994301595#L4373

Could you please have a look?

  Thanks,
   Thomas
Michael Tokarev Jan. 30, 2025, 10:12 a.m. UTC | #4
30.01.2025 13:09, Thomas Huth wrote:
> On 30/01/2025 03.13, Samuel Thibault wrote:
>> Hello,
>>
>> Samuel Thibault, le jeu. 10 oct. 2024 01:06:47 +0200, a ecrit:
>>> Michael Tokarev, le sam. 05 oct. 2024 10:07:53 +0300, a ecrit:
>>>> libslirp introduced new typedef after 4.8.0, slirp_os_socket, which
>>>> is defined to SOCKET on windows, which, in turn, is a 64bit number.
>>>> qemu uses int, so callback function prorotypes changed.
>>>
>>> I have fixed the code in upstream libslirp, to avoid breaking the API
>>> and ABI, and instead provide new functions & methods so that
>>> qemu/libslirp can upgrade smoothly.
>>
>> It is now released in version 4.9.0 of libslirp.
> 
> Seems like 4.9.0 breaks the CI:
> 
> https://gitlab.com/thuth/qemu/-/jobs/8994301595#L4373

Yes, it's because the old method used by qemu is now deprecated,
and we build with -Werror.

The prob here is that apparently we don't have a good way to use
slirp on win64.  Because the new slirp callback uses SOCKET type
instead of int, and SOCKET is 64bit on win64, and qemu main loop
is based on GPollFD which uses int.

/mjt
Samuel Thibault Jan. 30, 2025, 10:15 a.m. UTC | #5
Michael Tokarev, le jeu. 30 janv. 2025 13:12:39 +0300, a ecrit:
> 30.01.2025 13:09, Thomas Huth wrote:
> > On 30/01/2025 03.13, Samuel Thibault wrote:
> > > Hello,
> > > 
> > > Samuel Thibault, le jeu. 10 oct. 2024 01:06:47 +0200, a ecrit:
> > > > Michael Tokarev, le sam. 05 oct. 2024 10:07:53 +0300, a ecrit:
> > > > > libslirp introduced new typedef after 4.8.0, slirp_os_socket, which
> > > > > is defined to SOCKET on windows, which, in turn, is a 64bit number.
> > > > > qemu uses int, so callback function prorotypes changed.
> > > > 
> > > > I have fixed the code in upstream libslirp, to avoid breaking the API
> > > > and ABI, and instead provide new functions & methods so that
> > > > qemu/libslirp can upgrade smoothly.
> > > 
> > > It is now released in version 4.9.0 of libslirp.
> > 
> > Seems like 4.9.0 breaks the CI:
> > 
> > https://gitlab.com/thuth/qemu/-/jobs/8994301595#L4373
> 
> Yes, it's because the old method used by qemu is now deprecated,
> and we build with -Werror.
> 
> The prob here is that apparently we don't have a good way to use
> slirp on win64.  Because the new slirp callback uses SOCKET type
> instead of int, and SOCKET is 64bit on win64, and qemu main loop
> is based on GPollFD which uses int.

? GPollFD uses a gint64 on win64.

Samuel
diff mbox series

Patch

diff --git a/net/slirp.c b/net/slirp.c
index eb9a456ed4..fa07268cf4 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -98,6 +98,10 @@  typedef struct SlirpState {
     GSList *fwd;
 } SlirpState;
 
+#ifndef SLIRP_INVALID_SOCKET /* after 4.8.0 */
+typedef int slirp_os_socket;
+#endif
+
 static struct slirp_config_str *slirp_configs;
 static QTAILQ_HEAD(, SlirpState) slirp_stacks =
     QTAILQ_HEAD_INITIALIZER(slirp_stacks);
@@ -247,7 +251,7 @@  static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
     timer_mod(&t->timer, expire_timer);
 }
 
-static void net_slirp_register_poll_fd(int fd, void *opaque)
+static void net_slirp_register_poll_fd(slirp_os_socket fd, void *opaque)
 {
 #ifdef WIN32
     AioContext *ctxt = qemu_get_aio_context();
@@ -260,7 +264,7 @@  static void net_slirp_register_poll_fd(int fd, void *opaque)
 #endif
 }
 
-static void net_slirp_unregister_poll_fd(int fd, void *opaque)
+static void net_slirp_unregister_poll_fd(slirp_os_socket fd, void *opaque)
 {
 #ifdef WIN32
     if (WSAEventSelect(fd, NULL, 0) != 0) {
@@ -314,7 +318,7 @@  static int slirp_poll_to_gio(int events)
     return ret;
 }
 
-static int net_slirp_add_poll(int fd, int events, void *opaque)
+static int net_slirp_add_poll(slirp_os_socket fd, int events, void *opaque)
 {
     GArray *pollfds = opaque;
     GPollFD pfd = {