[for-3.2,06/41] slirp: add a callback for qemu_chr_fe_write_all()

Replace strong dependency on QEMU.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 slirp/libslirp.h | 1 +
 net/slirp.c      | 6 ++++++
 slirp/slirp.c    | 2 +-
 3 files changed, 8 insertions(+), 1 deletion(-)

On 14/11/2018 13:36, Marc-André Lureau wrote:
> Replace strong dependency on QEMU.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  slirp/libslirp.h | 1 +
>  net/slirp.c      | 6 ++++++
>  slirp/slirp.c    | 2 +-
>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> index 36d5fb9163..16ced2aa0f 100644
> --- a/slirp/libslirp.h
> +++ b/slirp/libslirp.h
> @@ -7,6 +7,7 @@ typedef struct Slirp Slirp;
>  
>  typedef struct SlirpCb {
>      void (*output)(void *opaque, const uint8_t *pkt, int pkt_len);
> +    int (*chr_write_all)(void *chr, const void *buf, size_t len);
>  } SlirpCb;
>  
>  
> diff --git a/net/slirp.c b/net/slirp.c
> index 233f66b1ef..5c1f676487 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -140,8 +140,14 @@ static NetClientInfo net_slirp_info = {
>      .cleanup = net_slirp_cleanup,
>  };
>  
> +static int net_slirp_chr_write_all(void *chr, const void *buf, size_t len)
> +{
> +    return qemu_chr_fe_write_all(chr, buf, len);
> +}
> +
>  static SlirpCb slirp_cb = {
>      .output = net_slirp_output,
> +    .chr_write_all = net_slirp_chr_write_all,


>  };
>  
>  static int net_slirp_init(NetClientState *peer, const char *model,
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 7213915bf3..4e809e5d7f 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -1099,7 +1099,7 @@ ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags)
>      if (so->s == -1 && so->chardev) {
>          /* XXX this blocks entire thread. Rewrite to use
>           * qemu_chr_fe_write and background I/O callbacks */
> -        qemu_chr_fe_write_all(so->chardev, buf, len);
> +        so->slirp->cb->chr_write_all(so->chardev, buf, len);
>          return len;
>      }
>  
> 

Rather than this, I would split out add_exec's do_pty==3 case to a new
function add_guestfwd, and pass the function pointer to add_guestfwd.  Then:

1) add_guestfwd can store the function pointer in struct ex_list,

2) tcp_ctl can store the ex_ptr in struct socket directly, instead of
storing ex_ptr->ex_exec (that is the chardev field becomes "struct
ex_list *guestfwd" or something)

3) slirp_send can use so->chardev to retrieve both the function pointer
and the void* (renaming it to so->exec maybe)

Paolo

Hi

On Thu, Nov 15, 2018 at 5:15 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 14/11/2018 13:36, Marc-André Lureau wrote:
> > Replace strong dependency on QEMU.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  slirp/libslirp.h | 1 +
> >  net/slirp.c      | 6 ++++++
> >  slirp/slirp.c    | 2 +-
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/slirp/libslirp.h b/slirp/libslirp.h
> > index 36d5fb9163..16ced2aa0f 100644
> > --- a/slirp/libslirp.h
> > +++ b/slirp/libslirp.h
> > @@ -7,6 +7,7 @@ typedef struct Slirp Slirp;
> >
> >  typedef struct SlirpCb {
> >      void (*output)(void *opaque, const uint8_t *pkt, int pkt_len);
> > +    int (*chr_write_all)(void *chr, const void *buf, size_t len);
> >  } SlirpCb;
> >
> >
> > diff --git a/net/slirp.c b/net/slirp.c
> > index 233f66b1ef..5c1f676487 100644
> > --- a/net/slirp.c
> > +++ b/net/slirp.c
> > @@ -140,8 +140,14 @@ static NetClientInfo net_slirp_info = {
> >      .cleanup = net_slirp_cleanup,
> >  };
> >
> > +static int net_slirp_chr_write_all(void *chr, const void *buf, size_t len)
> > +{
> > +    return qemu_chr_fe_write_all(chr, buf, len);
> > +}
> > +
> >  static SlirpCb slirp_cb = {
> >      .output = net_slirp_output,
> > +    .chr_write_all = net_slirp_chr_write_all,
>
>
> >  };
> >
> >  static int net_slirp_init(NetClientState *peer, const char *model,
> > diff --git a/slirp/slirp.c b/slirp/slirp.c
> > index 7213915bf3..4e809e5d7f 100644
> > --- a/slirp/slirp.c
> > +++ b/slirp/slirp.c
> > @@ -1099,7 +1099,7 @@ ssize_t slirp_send(struct socket *so, const void *buf, size_t len, int flags)
> >      if (so->s == -1 && so->chardev) {
> >          /* XXX this blocks entire thread. Rewrite to use
> >           * qemu_chr_fe_write and background I/O callbacks */
> > -        qemu_chr_fe_write_all(so->chardev, buf, len);
> > +        so->slirp->cb->chr_write_all(so->chardev, buf, len);
> >          return len;
> >      }
> >
> >
>
> Rather than this, I would split out add_exec's do_pty==3 case to a new
> function add_guestfwd, and pass the function pointer to add_guestfwd.  Then:
>
> 1) add_guestfwd can store the function pointer in struct ex_list,
>
> 2) tcp_ctl can store the ex_ptr in struct socket directly, instead of
> storing ex_ptr->ex_exec (that is the chardev field becomes "struct
> ex_list *guestfwd" or something)
>
> 3) slirp_send can use so->chardev to retrieve both the function pointer
> and the void* (renaming it to so->exec maybe)

Ok, please check the upcoming series, patch "slirp: generalize
guestfwd with a callback based approach". I think it is what you
propose here.

thanks