From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ceLVW-0003qo-Me for qemu-devel@nongnu.org; Thu, 16 Feb 2017 07:49:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ceLVS-0007tO-Qm for qemu-devel@nongnu.org; Thu, 16 Feb 2017 07:49:58 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:2324) by eggs.gnu.org with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.71) (envelope-from ) id 1ceLVS-0007ss-6r for qemu-devel@nongnu.org; Thu, 16 Feb 2017 07:49:54 -0500 References: <1487224821-120916-1-git-send-email-zhang.zhanghailiang@huawei.com> <1487224821-120916-4-git-send-email-zhang.zhanghailiang@huawei.com> From: Hailiang Zhang Message-ID: <58A59FD9.3080900@huawei.com> Date: Thu, 16 Feb 2017 20:49:29 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 3/4] char: remove the right fd been watched in qemu_chr_fe_set_handlers() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , jasowang@redhat.com, zhangchen.fnst@cn.fujitsu.com, lizhijian@cn.fujitsu.com Cc: xuquan8@huawei.com, qemu-devel@nongnu.org, pss.wulizhen@huawei.com, Paolo Bonzini Hi, On 2017/2/16 18:40, Marc-André Lureau wrote: > Hi > > On Thu, Feb 16, 2017 at 10:08 AM zhanghailiang < > zhang.zhanghailiang@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 >> Cc: Marc-André Lureau >> Signed-off-by: zhanghailiang >> --- >> 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 ？ 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 >