Re: [Qemu-devel] [PATCH v2 00/15] qio: general non-default GMainContext support
On Thu, Mar 01, 2018 at 04:07:06PM +, 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! -- Peter Xu
Re: [Qemu-devel] [PATCH v2 00/15] qio: general non-default GMainContext support
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 -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PATCH v2 00/15] qio: general non-default GMainContext support
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(-)
--
2.14.3
