qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org, qemu-ppc@nongnu.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Amit Shah <amit.shah@redhat.com>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"Daniel P. Berrange" <berrange@redhat.com>
Subject: [Qemu-devel] [PATCH 0/5] Global fix / workaround usage of qemu_chr_fe_write
Date: Wed,  3 Aug 2016 17:22:35 +0100	[thread overview]
Message-ID: <1470241360-3574-1-git-send-email-berrange@redhat.com> (raw)

This series does a global fix and/or workaround for bad usage of
qemu_chr_fe_write(). Essentially only about 4/5 places in the code
got the usage correct, by handling errors. Everything else would
silently loose data if used with a chardev backend that can return
EAGAIN. One specific instance of that bug was highlighted when I
fixed the socket backend, such that in connect mode, it would be
non-blocking. https://bugs.launchpad.net/qemu/+bug/1586756

The "correct" fix is to check for short writes, or EAGAIN and
then schedule an event callback to re-try the write later. A
couple of places do that correctly (for example hw/char/serial.c
and hw/char/virtioconsole.c).  Changing existing serial port
impls todo this though is a fair amount of amount, so as a
temporary fix, this series changes all the broken code to simply
use qemu_chr_fe_write_all() instead. Thus we at least stop
silently loosing data, albeit at the cost of blocking the guest
execution while we wait. So we still need to do a proper fix
in all these devices models at some point, hence I've left
comments in the code as a reminder.

This series also includes a patch to virtio-console I sent
previously, but with a fixed commit message that more accurately
describes what is going on.

Daniel P. Berrange (5):
  virtio-console: set frontend open permanently for console devs
  impi: check return of qemu_chr_fe_write() for errors
  sclpconsole: remove bogus check for -EAGAIN
  hw: replace most use of qemu_chr_fe_write with qemu_chr_fe_write_all
  char: convert qemu_chr_fe_write to qemu_chr_fe_write_all

 backends/rng-egd.c          |  4 +++-
 gdbstub.c                   |  4 +++-
 hw/arm/omap2.c              |  8 +++++---
 hw/arm/pxa2xx.c             |  4 +++-
 hw/arm/strongarm.c          |  4 +++-
 hw/char/bcm2835_aux.c       |  4 +++-
 hw/char/debugcon.c          |  4 +++-
 hw/char/digic-uart.c        |  2 ++
 hw/char/escc.c              |  4 +++-
 hw/char/etraxfs_ser.c       |  4 +++-
 hw/char/exynos4210_uart.c   |  4 +++-
 hw/char/grlib_apbuart.c     |  4 +++-
 hw/char/imx_serial.c        |  4 +++-
 hw/char/ipoctal232.c        |  4 +++-
 hw/char/lm32_juart.c        |  2 ++
 hw/char/lm32_uart.c         |  2 ++
 hw/char/mcf_uart.c          |  4 +++-
 hw/char/parallel.c          |  4 +++-
 hw/char/pl011.c             |  4 +++-
 hw/char/sclpconsole-lm.c    | 22 ++++++----------------
 hw/char/sclpconsole.c       |  2 ++
 hw/char/sh_serial.c         |  4 +++-
 hw/char/spapr_vty.c         |  5 +++--
 hw/char/stm32f2xx_usart.c   |  2 ++
 hw/char/virtio-console.c    | 46 +++++++++++++++++++++++++++++++++++++++++----
 hw/char/xilinx_uartlite.c   |  4 +++-
 hw/ipmi/ipmi_bmc_extern.c   |  8 ++++++--
 hw/usb/ccid-card-passthru.c |  7 +++++--
 hw/usb/dev-serial.c         |  4 +++-
 qemu-char.c                 | 18 ++++++++++++------
 slirp/slirp.c               |  4 +++-
 31 files changed, 146 insertions(+), 54 deletions(-)

-- 
2.7.4

             reply	other threads:[~2016-08-03 16:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-03 16:22 Daniel P. Berrange [this message]
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
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=1470241360-3574-1-git-send-email-berrange@redhat.com \
    --to=berrange@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=cornelia.huck@de.ibm.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).