Hi,
After reading the code again. It make sense to leave the call to
remove_fd_in_watch() because I changed the code so the chr_update_read_handler
function is called only when the front end is opened.
Submitting a new version of the patch.
Gal.
----- Original Message -----
From: "Gal Hammer" <gham...@redhat.com>
To: "Paolo Bonzini" <pbonz...@redhat.com>, qemu-devel@nongnu.org
Cc: "amit shah" <amit.s...@redhat.com>, "peter maydell"
<peter.mayd...@linaro.org>, anth...@codemonkey.ws
Sent: Monday, February 24, 2014 1:44:31 PM
Subject: Re: [Qemu-devel] [PATCH 2/2] qtest: fix a "!chr->fd_in_tag" assertion
error in qtest.
On 24/02/2014 12:32, Paolo Bonzini wrote:
> Il 24/02/2014 11:16, Gal Hammer ha scritto:
>> Replacement of the default chardev handlers now requires a call to
>> release the current handlers.
>>
>> Signed-off-by: Gal Hammer <gham...@redhat.com>
>> ---
>> monitor.c | 1 +
>> qtest.c | 2 ++
>> 2 files changed, 3 insertions(+)
>>
>> diff --git a/monitor.c b/monitor.c
>> index de90fba..db52e7f 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -5024,6 +5024,7 @@ void monitor_init(CharDriverState *chr, int flags)
>> if (monitor_ctrl_mode(mon)) {
>> mon->mc = g_malloc0(sizeof(MonitorControl));
>> /* Control mode requires special handlers */
>> + qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL);
>> qemu_chr_add_handlers(chr, monitor_can_read,
>> monitor_control_read,
>> monitor_control_event, mon);
>> qemu_chr_fe_set_echo(chr, true);
>> diff --git a/qtest.c b/qtest.c
>> index ae941d6..a5682ee 100644
>> --- a/qtest.c
>> +++ b/qtest.c
>> @@ -519,6 +519,8 @@ void qtest_init(const char *qtest_chrdev, const
>> char *qtest_log, Error **errp)
>> return;
>> }
>>
>> + /* Replace the default tcp's handlers with qtest's handlers. */
>> + qemu_chr_add_handlers(chr, NULL, NULL, NULL, NULL);
>> qemu_chr_add_handlers(chr, qtest_can_read, qtest_read,
>> qtest_event, chr);
>> qemu_chr_fe_set_echo(chr, true);
>>
>>
>
> Why can't qemu_chr_add_handlers call remove_fd_in_watch when handlers
> are replaced, before setting the new ones?
>
> Paolo
It used to be like that (see patch 1/2 for the change). However, I
removed it after I saw that the function remove_fd_in_watch() is always
called when setting all the handlers to NULL (qemu-char.c +3872) so a
second call in every chr_update_read_handler callback seems redundant.
As far as I can tell only the qtest change the read handlers and not
using the default ones.
Gal.
