Hi On Thu, Feb 16, 2017 at 4:49 PM Hailiang Zhang < zhang.zhanghaili...@huawei.com> wrote:
> Hi, > > On 2017/2/16 18:40, Marc-André Lureau wrote: > > Hi > > > > On Thu, Feb 16, 2017 at 10:08 AM zhanghailiang < > > zhang.zhanghaili...@huawei.com> wrote: > > > >> We can call qemu_chr_fe_set_handlers() to add/remove fd been watched > >> in 'context' which can be either default main context or other explicit > >> context. But the original logic is not correct, we didn't remove > >> the right fd because we call g_main_context_find_source_by_id(NULL, tag) > >> which always try to find the Gsource from default context. > >> > >> Fix it by passing the right context to > g_main_context_find_source_by_id(). > >> > >> Cc: Paolo Bonzini <pbonz...@redhat.com> > >> Cc: Marc-André Lureau <marcandre.lur...@redhat.com> > >> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> > >> --- > >> chardev/char-io.c | 13 +++++++++---- > >> chardev/char-io.h | 2 ++ > >> chardev/char.c | 2 +- > >> 3 files changed, 12 insertions(+), 5 deletions(-) > >> > >> diff --git a/chardev/char-io.c b/chardev/char-io.c > >> index 7dfc3f2..a69cc61 100644 > >> --- a/chardev/char-io.c > >> +++ b/chardev/char-io.c > >> @@ -127,14 +127,14 @@ guint io_add_watch_poll(Chardev *chr, > >> return tag; > >> } > >> > >> -static void io_remove_watch_poll(guint tag) > >> +static void io_remove_watch_poll(guint tag, GMainContext *context) > >> { > >> GSource *source; > >> IOWatchPoll *iwp; > >> > >> g_return_if_fail(tag > 0); > >> > >> - source = g_main_context_find_source_by_id(NULL, tag); > >> + source = g_main_context_find_source_by_id(context, tag); > >> g_return_if_fail(source != NULL); > >> > >> iwp = io_watch_poll_from_source(source); > >> @@ -146,14 +146,19 @@ static void io_remove_watch_poll(guint tag) > >> g_source_destroy(&iwp->parent); > >> } > >> > >> -void remove_fd_in_watch(Chardev *chr) > >> +void qemu_remove_fd_in_watch(Chardev *chr, GMainContext *context) > >> { > >> if (chr->fd_in_tag) { > >> - io_remove_watch_poll(chr->fd_in_tag); > >> + io_remove_watch_poll(chr->fd_in_tag, context); > >> chr->fd_in_tag = 0; > >> } > >> } > >> > >> +void remove_fd_in_watch(Chardev *chr) > >> +{ > >> + qemu_remove_fd_in_watch(chr, NULL); > >> +} > >> + > >> > > > > I would just change the signature of remove_fd_in_watch() instead of > > introducing a function. > > > > In that case, i have to modify all the places call this helper, > About eleven places in six files,is it acceptable ? > Yes, that's fine. > > Thanks. > > > int io_channel_send_full(QIOChannel *ioc, > >> const void *buf, size_t len, > >> int *fds, size_t nfds) > >> diff --git a/chardev/char-io.h b/chardev/char-io.h > >> index d7ae5f1..117c888 100644 > >> --- a/chardev/char-io.h > >> +++ b/chardev/char-io.h > >> @@ -38,6 +38,8 @@ guint io_add_watch_poll(Chardev *chr, > >> > >> void remove_fd_in_watch(Chardev *chr); > >> > >> +void qemu_remove_fd_in_watch(Chardev *chr, GMainContext *context); > >> + > >> int io_channel_send(QIOChannel *ioc, const void *buf, size_t len); > >> > >> int io_channel_send_full(QIOChannel *ioc, const void *buf, size_t len, > >> diff --git a/chardev/char.c b/chardev/char.c > >> index abd525f..5563375 100644 > >> --- a/chardev/char.c > >> +++ b/chardev/char.c > >> @@ -560,7 +560,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b, > >> cc = CHARDEV_GET_CLASS(s); > >> if (!opaque && !fd_can_read && !fd_read && !fd_event) { > >> fe_open = 0; > >> - remove_fd_in_watch(s); > >> + qemu_remove_fd_in_watch(s, context); > >> } else { > >> fe_open = 1; > >> } > >> -- > >> 1.8.3.1 > >> > >> > >> > > otherwise, looks good to me > > > > -- Marc-André Lureau