Jan Kiszka wrote: > Anthony Liguori wrote: >> On 05/04/2010 11:01 AM, Alexander Graf wrote: >>> Am 04.05.2010 um 16:34 schrieb Anthony Liguori <anth...@codemonkey.ws>: >>> >>>> On 05/04/2010 09:30 AM, Alexander Graf wrote: >>>>> Am 04.05.2010 um 15:44 schrieb Anthony Liguori <anth...@codemonkey.ws>: >>>>> >>>>>> 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).
Nonsense.
In fact, the problem is the former issue: As the muxer reads the
character the front-end is currently unable to receive, polling may stop
until as the back-end has some further chars to deliver.
But interestingly, the stdio back-end has a (single-byte) fifo as well.
It just drives it a bit differently.
Alex, does this help as well?
diff --git a/qemu-char.c b/qemu-char.c
index ac65a1c..2b115a4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -404,6 +404,8 @@ static int mux_chr_can_read(void *opaque)
MuxDriver *d = chr->opaque;
int m = d->focus;
+ mux_chr_accept_input(opaque);
+
if ((d->prod[m] - d->cons[m]) < MUX_BUFFER_SIZE)
return 1;
if (d->chr_can_read[m])
@@ -418,8 +420,6 @@ static void mux_chr_read(void *opaque, const uint8_t *buf,
int size)
int m = d->focus;
int i;
- mux_chr_accept_input (opaque);
-
for(i = 0; i < size; i++)
if (mux_proc_byte(chr, d, buf[i])) {
if (d->prod[m] == d->cons[m] &&
I'm trying to reproduce in parallel.
Jan
signature.asc
Description: OpenPGP digital signature
