Re: [Qemu-devel] [PATCH] [RESEND] Make char muxer more robust wrt small FIFOs

[Jan Kiszka]

Anthony Liguori wrote:
> On 05/04/2010 11:01 AM, Alexander Graf wrote:
>> Am 04.05.2010 um 16:34 schrieb Anthony Liguori <address@hidden>:
>>
>>> On 05/04/2010 09:30 AM, Alexander Graf wrote:
>>>>
>>>> Am 04.05.2010 um 15:44 schrieb Anthony Liguori <address@hidden>:
>>>>
>>>>> On 04/20/2010 11:56 AM, Alexander Graf wrote:
>>>>>> Virtio-Console can only process one character at a time. Using it 
>>>>>> on S390
>>>>>> gave me strage "lags" where I got the character I pressed before when
>>>>>> pressing one. So I typed in "abc" and only received "a", then 
>>>>>> pressed "d"
>>>>>> but the guest received "b" and so on.
>>>>>>
>>>>>> While the stdio driver calls a poll function that just processes 
>>>>>> on its
>>>>>> queue in case virtio-console can't take multiple characters at 
>>>>>> once, the
>>>>>> muxer does not have such callbacks, so it can't empty its queue.
>>>>>>
>>>>>> To work around that limitation, I introduced a new timer that only 
>>>>>> gets
>>>>>> active when the guest can not receive any more characters. In that 
>>>>>> case
>>>>>> it polls again after a while to check if the guest is now 
>>>>>> receiving input.
>>>>>>
>>>>>> This patch fixes input when using -nographic on s390 for me.
>>>>>>
>>>>> I think this is really a kvm issue.  I assume it's because s390 
>>>>> idles in the kernel so you never drop to userspace to repoll the 
>>>>> descriptor.
>>>> There is no polling for the muxer. That's why it never knows when 
>>>> virtio-console can receive again.
>>> Maybe I'm missing something simple, but it looks to me like the muxer 
>>> is polling.  mux_chr_can_read() is going to eventually poll the muxed 
>>> devices to figure this out.
>>>
>>> If the root of the problem is that mux_chr_can_read() isn't being 
>>> invoked for a prolonged period of time, the real issue is the problem 
>>> I described.
>> The problem is that the select list of fds includes the stdio fd, so 
>> that gets notified and is coupled with virtio-console, but there's 
>> nothing passing that on to mux and I don't think it'd be clever to 
>> expose internal data to the muxer to tell it about the backend fds.
> 
> When stdio is readable, it should invoke qemu_chr_read() with the read 
> data which in turn ought to invoke mux_chr_read().
> 
> I'm not sure I understand what signalling is missing.  Jan, does the 
> problem Alex describes ring a bell?  I seem to recall you saying that 
> mux was still fundamentally broken but ought to work most of the time...

That problem was (and still is) that the muxer needs to accept
characters even if the active front-end device is not in order to filter
out control sequences. Once its queue is full, it will start dropping
those the active device would not if directly connected. Could only be
solved via some peek service on pending front-end data.

I think Alex' problem can be addressed by registering
qemu_set_fd_handler2(..., backend->read_poll, mux_chr_read, ...). That
means the backend has to tell us about its read poll handler (if any).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux