[Qemu-devel] [PATCH v2 3/4] chardev: use per-dev context for io_add_watch_poll

Peter Xu posted 4 patches 8 years, 1 month ago
[Qemu-devel] [PATCH v2 3/4] chardev: use per-dev context for io_add_watch_poll
Posted by Peter Xu 8 years, 1 month ago
It was only passed in by chr_update_read_handlers().  However when
reconnect, we'll lose that context information.  So if a chardev was
running on another context (rather than the default context, the NULL
pointer), it'll switch back to the default context if reconnection
happens.  But, it should really stick to the old context.

Convert all the callers of io_add_watch_poll() to use the internally
cached gcontext.  Then the context should be able to survive even after
reconnections.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 chardev/char-fd.c     | 2 +-
 chardev/char-pty.c    | 2 +-
 chardev/char-socket.c | 4 ++--
 chardev/char-udp.c    | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index 6a62a54..09fbb07 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -94,7 +94,7 @@ static void fd_chr_update_read_handler(Chardev *chr,
         chr->gsource = io_add_watch_poll(chr, s->ioc_in,
                                            fd_chr_read_poll,
                                            fd_chr_read, chr,
-                                           context);
+                                           chr->gcontext);
     }
 }
 
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index e5d20a0..d239c04 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -219,7 +219,7 @@ static void pty_chr_state(Chardev *chr, int connected)
             chr->gsource = io_add_watch_poll(chr, s->ioc,
                                                pty_chr_read_poll,
                                                pty_chr_read,
-                                               chr, NULL);
+                                               chr, chr->gcontext);
         }
     }
 }
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 1ae730a..ee71cbe 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -516,7 +516,7 @@ static void tcp_chr_connect(void *opaque)
         chr->gsource = io_add_watch_poll(chr, s->ioc,
                                            tcp_chr_read_poll,
                                            tcp_chr_read,
-                                           chr, NULL);
+                                           chr, chr->gcontext);
     }
     qemu_chr_be_event(chr, CHR_EVENT_OPENED);
 }
@@ -535,7 +535,7 @@ static void tcp_chr_update_read_handler(Chardev *chr,
         chr->gsource = io_add_watch_poll(chr, s->ioc,
                                            tcp_chr_read_poll,
                                            tcp_chr_read, chr,
-                                           context);
+                                           chr->gcontext);
     }
 }
 
diff --git a/chardev/char-udp.c b/chardev/char-udp.c
index 4ee11d3..106dee1 100644
--- a/chardev/char-udp.c
+++ b/chardev/char-udp.c
@@ -110,7 +110,7 @@ static void udp_chr_update_read_handler(Chardev *chr,
         chr->gsource = io_add_watch_poll(chr, s->ioc,
                                            udp_chr_read_poll,
                                            udp_chr_read, chr,
-                                           context);
+                                           chr->gcontext);
     }
 }
 
-- 
2.7.4


Re: [Qemu-devel] [PATCH v2 3/4] chardev: use per-dev context for io_add_watch_poll
Posted by Paolo Bonzini 8 years, 1 month ago
On 21/09/2017 08:35, Peter Xu wrote:
> It was only passed in by chr_update_read_handlers().  However when
> reconnect, we'll lose that context information.  So if a chardev was
> running on another context (rather than the default context, the NULL
> pointer), it'll switch back to the default context if reconnection
> happens.  But, it should really stick to the old context.
> 
> Convert all the callers of io_add_watch_poll() to use the internally
> cached gcontext.  Then the context should be able to survive even after
> reconnections.

Why keep the argument then?

Thanks,

Paolo

> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  chardev/char-fd.c     | 2 +-
>  chardev/char-pty.c    | 2 +-
>  chardev/char-socket.c | 4 ++--
>  chardev/char-udp.c    | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/chardev/char-fd.c b/chardev/char-fd.c
> index 6a62a54..09fbb07 100644
> --- a/chardev/char-fd.c
> +++ b/chardev/char-fd.c
> @@ -94,7 +94,7 @@ static void fd_chr_update_read_handler(Chardev *chr,
>          chr->gsource = io_add_watch_poll(chr, s->ioc_in,
>                                             fd_chr_read_poll,
>                                             fd_chr_read, chr,
> -                                           context);
> +                                           chr->gcontext);
>      }
>  }
>  
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index e5d20a0..d239c04 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -219,7 +219,7 @@ static void pty_chr_state(Chardev *chr, int connected)
>              chr->gsource = io_add_watch_poll(chr, s->ioc,
>                                                 pty_chr_read_poll,
>                                                 pty_chr_read,
> -                                               chr, NULL);
> +                                               chr, chr->gcontext);
>          }
>      }
>  }
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 1ae730a..ee71cbe 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -516,7 +516,7 @@ static void tcp_chr_connect(void *opaque)
>          chr->gsource = io_add_watch_poll(chr, s->ioc,
>                                             tcp_chr_read_poll,
>                                             tcp_chr_read,
> -                                           chr, NULL);
> +                                           chr, chr->gcontext);
>      }
>      qemu_chr_be_event(chr, CHR_EVENT_OPENED);
>  }
> @@ -535,7 +535,7 @@ static void tcp_chr_update_read_handler(Chardev *chr,
>          chr->gsource = io_add_watch_poll(chr, s->ioc,
>                                             tcp_chr_read_poll,
>                                             tcp_chr_read, chr,
> -                                           context);
> +                                           chr->gcontext);
>      }
>  }
>  
> diff --git a/chardev/char-udp.c b/chardev/char-udp.c
> index 4ee11d3..106dee1 100644
> --- a/chardev/char-udp.c
> +++ b/chardev/char-udp.c
> @@ -110,7 +110,7 @@ static void udp_chr_update_read_handler(Chardev *chr,
>          chr->gsource = io_add_watch_poll(chr, s->ioc,
>                                             udp_chr_read_poll,
>                                             udp_chr_read, chr,
> -                                           context);
> +                                           chr->gcontext);
>      }
>  }
>  
> 


Re: [Qemu-devel] [PATCH v2 3/4] chardev: use per-dev context for io_add_watch_poll
Posted by Peter Xu 8 years, 1 month ago
On Thu, Sep 21, 2017 at 04:11:33PM +0200, Paolo Bonzini wrote:
> On 21/09/2017 08:35, Peter Xu wrote:
> > It was only passed in by chr_update_read_handlers().  However when
> > reconnect, we'll lose that context information.  So if a chardev was
> > running on another context (rather than the default context, the NULL
> > pointer), it'll switch back to the default context if reconnection
> > happens.  But, it should really stick to the old context.
> > 
> > Convert all the callers of io_add_watch_poll() to use the internally
> > cached gcontext.  Then the context should be able to survive even after
> > reconnections.
> 
> Why keep the argument then?

Yeah it's in next patch. I see that this series has been queued
already. Thanks!

-- 
Peter Xu