Re: [Qemu-devel] [PATCH] fix monitor

[Gerd Hoffmann <kraxel@redhat.com>]

On 03/19/13 15:45, Laszlo Ersek wrote:
> On 03/19/13 15:04, Gerd Hoffmann wrote:
>> chardev flow control broke monitor, fix it by adding watch support.
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> v2: fix tyops
> 
> Well played, Sir :)
> 
>> ---
>>  monitor.c |   23 +++++++++++++++++++++--
>>  1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 112e920..74807f9 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -261,11 +261,30 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
>>      }
>>  }
>>  
>> +static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
>> +                                  void *opaque)
>> +{
>> +    monitor_flush(opaque);
>> +    return FALSE;
>> +}
>> +
>>  void monitor_flush(Monitor *mon)
>>  {
>> +    int rc;
>> +
>>      if (mon && mon->outbuf_index != 0 && !mon->mux_out) {
>> -        qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
>> -        mon->outbuf_index = 0;
>> +        rc = qemu_chr_fe_write(mon->chr, mon->outbuf, mon->outbuf_index);
>> +        if (rc == mon->outbuf_index) {
>> +            /* all flushed */
>> +            mon->outbuf_index = 0;
>> +            return;
>> +        }
>> +        if (rc > 0) {
>> +            /* partial write */
>> +            memmove(mon->outbuf, mon->outbuf + rc, mon->outbuf_index - rc);
>> +            mon->outbuf_index -= rc;
>> +        }
>> +        qemu_chr_fe_add_watch(mon->chr, G_IO_OUT, monitor_unblocked, mon);
>>      }
>>  }
>>  
>>
> 
> Looks good to me.
> 
> Should we maybe forego (re)installing the (new) watch when rc<0?
> (Especially because on a persistent error the fd might immediately
> report writability.)

Yes, it is probably wise to check for errno == EAGAIN (assuming
qemu_chr_fe_write makes sure errno it set to something sensible on error).

cheers,
  Gerd