| Message ID | 20180301084438.13594-1-peterx@redhat.com |
|---|---|
| Headers | show |
| Series | qio: general non-default GMainContext support | expand |
On Thu, Mar 01, 2018 at 04:44:23PM +0800, Peter Xu wrote: > This is another preparation work for monitor OOB seires. > > V1: http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg06972.html > > V2 rewrote the bottom half of the code. The first 8 patches are > mostly the same, but I rewrote the last patches to solve both TLS and > reconnect use cases by introducing a machine_done hook for chardevs in > general. So if I copy the problems: > > - migration > - incoming side: still always running on main context, while we need > to be able to run some command in OOB thread [1] > - tcp chardev (non-tcp chardevs should all support non-NULL context now) > - server listening mode: QIO net listener used [2] > - TELNET session: an isolated GSource used (tcp_chr_telnet_init) [3] > - when "reconnect=N" is used, QIO threaded task is used [4] > - TLS session: QIO tls handshake is used (tcp_chr_tls_init) [5] > > Problem [1-3] are still fixed in the old way, but [4-5] now are fixed > by using the new machine_done notifier. The QIO code changes all look good to me know, aside from minor comments. I really dislike all of the chardev stuff though. I think it makes the chardev code even harder to follow & rationalize behaviour of. If you post a v3 series contaning just the qio/ directory changes, I'd queue those patches, while we discuss chardev stuff more. I struggle to suggest better approach, because its any missing context of how the changes are going to be used, presumably by patch series yet to be posted. Regards, Daniel
On Thu, Mar 01, 2018 at 04:07:06PM +0000, Daniel P. Berrangé wrote: > On Thu, Mar 01, 2018 at 04:44:23PM +0800, Peter Xu wrote: > > This is another preparation work for monitor OOB seires. > > > > V1: http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg06972.html > > > > V2 rewrote the bottom half of the code. The first 8 patches are > > mostly the same, but I rewrote the last patches to solve both TLS and > > reconnect use cases by introducing a machine_done hook for chardevs in > > general. So if I copy the problems: > > > > - migration > > - incoming side: still always running on main context, while we need > > to be able to run some command in OOB thread [1] > > - tcp chardev (non-tcp chardevs should all support non-NULL context now) > > - server listening mode: QIO net listener used [2] > > - TELNET session: an isolated GSource used (tcp_chr_telnet_init) [3] > > - when "reconnect=N" is used, QIO threaded task is used [4] > > - TLS session: QIO tls handshake is used (tcp_chr_tls_init) [5] > > > > Problem [1-3] are still fixed in the old way, but [4-5] now are fixed > > by using the new machine_done notifier. > > The QIO code changes all look good to me know, aside from minor > comments. I really dislike all of the chardev stuff though. I > think it makes the chardev code even harder to follow & rationalize > behaviour of. > > If you post a v3 series contaning just the qio/ directory changes, > I'd queue those patches, while we discuss chardev stuff more. > > I struggle to suggest better approach, because its any missing > context of how the changes are going to be used, presumably by > patch series yet to be posted. Yeah I think I'll split the series into two. Thank you and Paolo for the quick review comments!
This is another preparation work for monitor OOB seires. V1: http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg06972.html V2 rewrote the bottom half of the code. The first 8 patches are mostly the same, but I rewrote the last patches to solve both TLS and reconnect use cases by introducing a machine_done hook for chardevs in general. So if I copy the problems: - migration - incoming side: still always running on main context, while we need to be able to run some command in OOB thread [1] - tcp chardev (non-tcp chardevs should all support non-NULL context now) - server listening mode: QIO net listener used [2] - TELNET session: an isolated GSource used (tcp_chr_telnet_init) [3] - when "reconnect=N" is used, QIO threaded task is used [4] - TLS session: QIO tls handshake is used (tcp_chr_tls_init) [5] Problem [1-3] are still fixed in the old way, but [4-5] now are fixed by using the new machine_done notifier. I'll still try to provide a changelog beside the big change mentioned: v2: - collect r-bs - qio_channel_add_watch_full() should still return the same thing as the old one, and introduced qio_channel_add_watch_full() to return a GSource pointer. [Dan] - Fix commit message on RDMA. It's using QIO, but still, I am not touching it. [Dan] - use qio_net_listener_set_client_func_full() directly, and avoid introducing new API. [Dan] Please review. Thanks. Peter Xu (15): chardev: fix leak in tcp_chr_telnet_init_io() qio: rename qio_task_thread_result qio: introduce qio_channel_add_watch_{full|source} migration: let incoming side use thread context qio: refactor net listener source operations qio: store gsources for net listeners qio/chardev: update net listener gcontext chardev: allow telnet gsource to switch gcontext qio: non-default context for threaded qtask qio: non-default context for async conn qio: non-default context for TLS handshake chardev: introduce chr_machine_done hook char: use chardev's gcontext for async connect chardev: tcp: postpone async connection setup chardev: tcp: postpone TLS work until machine done chardev/char-mux.c | 29 ++++++++++ chardev/char-socket.c | 124 ++++++++++++++++++++++++++++++++--------- chardev/char.c | 43 ++++++-------- include/chardev/char.h | 2 + include/io/channel-socket.h | 4 +- include/io/channel-tls.h | 17 ++++++ include/io/channel.h | 44 +++++++++++++++ include/io/net-listener.h | 21 ++++++- include/io/task.h | 6 +- io/channel-socket.c | 12 ++-- io/channel-tls.c | 51 ++++++++++++----- io/channel.c | 40 +++++++++++-- io/dns-resolver.c | 3 +- io/net-listener.c | 112 +++++++++++++++++++++---------------- io/task.c | 22 +++++++- migration/exec.c | 9 ++- migration/fd.c | 9 ++- migration/socket.c | 13 +++-- tests/test-io-channel-socket.c | 2 +- tests/test-io-task.c | 2 + 20 files changed, 415 insertions(+), 150 deletions(-)