On 07/21/2016 05:00 PM, Daniel P. Berrange wrote:
On Thu, Jul 21, 2016 at 04:26:11PM +0800, Zhang Chen wrote:Add qemu_chr_add_handlers_full() API, we can use this API pass in a GMainContext,make handler run in the context rather than main_loop. This comments from Daniel P . Berrange. Cc: Daniel P . Berrange <berra...@redhat.com> Cc: Paolo Bonzini <pbonz...@redhat.com> Signed-off-by: Zhang Chen <zhangchen.f...@cn.fujitsu.com> Signed-off-by: Li Zhijian <lizhij...@cn.fujitsu.com> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> --- include/sysemu/char.h | 10 ++++ qemu-char.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) diff --git a/include/sysemu/char.h b/include/sysemu/char.h index 307fd8f..3ab897e 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -66,6 +66,8 @@ struct CharDriverState { const uint8_t *buf, int len); GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond); void (*chr_update_read_handler)(struct CharDriverState *s);I'd expect this to be deleted, and all existing impls given a new GMainContext * argument.
OK~~ I will fix it in next version.
+ void (*chr_update_read_handler_full)(struct CharDriverState *s, + GMainContext *context); int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg); int (*get_msgfds)(struct CharDriverState *s, int* fds, int num); int (*set_msgfds)(struct CharDriverState *s, int *fds, int num); @@ -388,6 +390,14 @@ void qemu_chr_add_handlers(CharDriverState *s, IOEventHandler *fd_event, void *opaque);+/* This API can make handler run in the context what you pass to. */+void qemu_chr_add_handlers_full(CharDriverState *s, + IOCanReadHandler *fd_can_read, + IOReadHandler *fd_read, + IOEventHandler *fd_event, + void *opaque, + GMainContext *context); + void qemu_chr_be_generic_open(CharDriverState *s); void qemu_chr_accept_input(CharDriverState *s); int qemu_chr_add_client(CharDriverState *s, int fd); diff --git a/qemu-char.c b/qemu-char.c index b597ee1..607b3b8 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -480,6 +480,40 @@ void qemu_chr_add_handlers(CharDriverState *s, } }+void qemu_chr_add_handlers_full(CharDriverState *s,+ IOCanReadHandler *fd_can_read, + IOReadHandler *fd_read, + IOEventHandler *fd_event, + void *opaque, + GMainContext *context) +{ + int fe_open; + + if (!opaque && !fd_can_read && !fd_read && !fd_event) { + fe_open = 0; + remove_fd_in_watch(s); + } else { + fe_open = 1; + } + s->chr_can_read = fd_can_read; + s->chr_read = fd_read; + s->chr_event = fd_event; + s->handler_opaque = opaque; + if (fe_open && s->chr_update_read_handler) {You should be checking s->chr_update_read_handler_full
Good Catch... will fix in next version.
+ s->chr_update_read_handler_full(s, context); + } + + if (!s->explicit_fe_open) { + qemu_chr_fe_set_open(s, fe_open); + } + + /* We're connecting to an already opened device, so let's make sure we + also get the open event */ + if (fe_open && s->be_open) { + qemu_chr_be_generic_open(s); + } +}I would expect the entire of the qemu_chr_add_handlers() method body to be deleted and have it simply call qemu_chr_add_handlers_full(), passing in a NULL GMainContext. That way we avoid code duplication.
Make sense. I got it~ Thanks a lot~~ Zhang Chen
Regards, Daniel
-- Thanks zhangchen
