qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Cornelia Huck <cornelia.huck@de.ibm.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-arm@nongnu.org, qemu-ppc@nongnu.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Amit Shah <amit.shah@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/5] virtio-console: set frontend open permanently for console devs
Date: Wed, 3 Aug 2016 19:37:40 +0200	[thread overview]
Message-ID: <20160803193740.08c3a365.cornelia.huck@de.ibm.com> (raw)
In-Reply-To: <1470241360-3574-2-git-send-email-berrange@redhat.com>

On Wed,  3 Aug 2016 17:22:36 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> The virtio-console.c file handles both serial consoles
> and interactive consoles, since they're backed by the
> same device model.
> 
> Since serial devices are expected to be reliable and
> need to notify the guest when the backend is opened
> or closed, the virtio-console.c file wires up support
> for chardev events. This affects both serial consoles
> and interactive consoles, using a network connection
> based chardev backend such as 'socket', but not when
> using a PTY based backend or plain 'file' backends.
> 
> When the host side is not connected the handle_output()
> method in virtio-serial-bus.c will drop any data sent
> by the guest, before it even reaches the virtio-console.c
> code. This means that if the chardev has a logfile
> configured, the data will never get logged.
> 
> Consider for example, configuring a x86_64 guest with a
> plain UART serial port
> 
>   -chardev socket,id=charserial1,host=127.0.0.1,port=9001,server,nowait,logfile=console1.log,logappend=on
>   -device isa-serial,chardev=charserial1,id=serial1
> 
> vs a s390 guest which has to use the virtio-console port
> 
>   -chardev socket,id=charconsole1,host=127.0.0.1,port=9000,server,nowait,logfile=console2.log,logappend=on
>   -device virtconsole,chardev=charconsole1,id=console1
> 
> The isa-serial one gets data written to the log regardless
> of whether a client is connected, while the virtioconsole
> one only gets data written to the log when a client is
> connected.
> 
> There is no need for virtio-serial-bus.c to aggressively
> drop the data for console devices, as the chardev code is
> prefectly capable of discarding the data itself.
> 
> So this patch changes virtconsole devices so that they
> are always marked as having the host side open. This
> ensures that the guest OS will always send any data it
> has (Linux virtio-console hvc driver actually ignores
> the host open state and sends data regardless, but we
> should not rely on that), and also prevents the
> virtio-serial-bus code prematurely discarding data.
> 
> The behaviour of virtserialport devices is *not* changed,
> only virtconsole, because for the former, it is important
> that the guest OSknow exactly when the host side is opened
> / closed so it can do any protocol re-negotiation that may
> be required.
> 
> Fixes bug: https://bugs.launchpad.net/qemu/+bug/1599214
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  hw/char/virtio-console.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)

From my limited understanding of the character layer this sounds
reasonable, and I agree that this is 2.7 material.

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>

  parent reply	other threads:[~2016-08-03 17:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-03 16:22 [Qemu-devel] [PATCH 0/5] Global fix / workaround usage of qemu_chr_fe_write Daniel P. Berrange
2016-08-03 16:22 ` [Qemu-devel] [PATCH 1/5] virtio-console: set frontend open permanently for console devs Daniel P. Berrange
2016-08-03 16:37   ` Paolo Bonzini
2016-08-03 17:37   ` Cornelia Huck [this message]
2016-08-03 16:22 ` [Qemu-devel] [PATCH 2/5] impi: check return of qemu_chr_fe_write() for errors Daniel P. Berrange
2016-08-03 16:22 ` [Qemu-devel] [PATCH 3/5] sclpconsole: remove bogus check for -EAGAIN Daniel P. Berrange
2016-08-04  8:13   ` Cornelia Huck
2016-08-03 16:22 ` [Qemu-devel] [PATCH 4/5] hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all Daniel P. Berrange
2016-08-04  8:23   ` Cornelia Huck
2016-08-03 16:22 ` [Qemu-devel] [PATCH 5/5] char: convert qemu_chr_fe_write to qemu_chr_fe_write_all Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160803193740.08c3a365.cornelia.huck@de.ibm.com \
    --to=cornelia.huck@de.ibm.com \
    --cc=amit.shah@redhat.com \
    --cc=berrange@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).