Hi
On Thu, May 31, 2018 at 1:03 PM, Sergio Lopez <s...@redhat.com> wrote:
> On Thu, May 31, 2018 at 11:52:01AM +0200, Marc-André Lureau wrote:
>> On Thu, May 31, 2018 at 9:46 AM, Sergio Lopez <s...@redhat.com> wrote:
>> > If writing to the frontend channel failed with EPIPE, don't set up a
>> > retry. EPIPE is not a recoverable error, so trying again is a waste of CPU
>> > cycles.
>> >
>> > If the vCPU writing to the serial device and emulator thread are pinned
>> > to the same pCPU, it can also compromise the stability of the Guest OS,
>> > as both threads will be competing for pCPU's time, with the vCPU
>> > actively polling the serial device and barely giving time to the
>> > emulator thread to make actual progress.
>> > ---
>> > hw/char/serial.c | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> > diff --git a/hw/char/serial.c b/hw/char/serial.c
>> > index 2c080c9..f26e86b 100644
>> > --- a/hw/char/serial.c
>> > +++ b/hw/char/serial.c
>> > @@ -262,6 +262,7 @@ static void serial_xmit(SerialState *s)
>> > /* in loopback mode, say that we just received a char */
>> > serial_receive1(s, &s->tsr, 1);
>> > } else if (qemu_chr_fe_write(&s->chr, &s->tsr, 1) != 1 &&
>> > + errno != EPIPE &&
>> > s->tsr_retry < MAX_XMIT_RETRY) {
>>
>> Instead of adding explicit handling of EPIPE, shouldn't the code be
>> rewritten to treat -1 return && errno != EAGAIN as fatal?
>
> What kind of action should it trigger if treating errno != EAGAIN as fatal?
"fatal" was perhaps the wrong word. I mean that we don't go into the
retry loop and treat the chardev as disconnected (without explicit
handling of EPIPE).
>
> If you meant something like calling 'abort()', I think that, while this
> could be considered as the "correct" behavior, it's a bit unpractical
> for the users. The most common situation for this to happen is
> restarting virtlogd while having the console/serial redirected to it.
> The VM will stop writing to the serial device, but otherwise the Guest
> OS can probably keep working without any issues.
>
> Sergio.
--
Marc-André Lureau
