Re: [Qemu-devel] [PATCH 2/2] qtest: fix a "!chr->fd_in_tag" assertion error in qtest.

[Gal Hammer]

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" <address@hidden>
To: "Paolo Bonzini" <address@hidden>, address@hidden
Cc: "amit shah" <address@hidden>, "peter maydell" <address@hidden>, 
address@hidden
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 <address@hidden>
>> ---
>>  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.