Re: [Qemu-devel] [PATCH 1/1] Protect stderr from non-blocking mode

[Stefan Hajnoczi]

On Thu, Jan 28, 2016 at 03:18:23PM +1100, Sam Bobroff wrote:
> On Linux, if QEMU is run from a shell with -d to enable debug logging
> but without directing it to a file (e.g. -D is not used, and
> qemu_logfile is set to stderr), and no shell redirection is used, it
> is possible for log messages to be lost under load.
> 
> This is caused by a combination of several factors:
> 
> * The shell (e.g. bash) may provide stdin, stdout and stderr as
>   duplicates of a single open file, so they share file status flags
>   including O_NONBLOCK.
> 
> * As character devices are registered (see qemu_chr_open_stdio()),
>   stdin and stdout are set non-blocking.
> 
> * The printf() family of functions, including fprintf(), are not
>   "non-blocking" aware: if they receive EAGAIN after partially
>   writing their output, they immediately return EAGAIN with no way to
>   discover how much output was written, if any.
> 
> So O_NONBLOCK is set on stdin/stdout, which causes O_NONBLOCK to be
> set on stderr, and qemu_logfile is set to stderr. Then, under load,
> fprintf()s to qemu_logfile return EAGAIN, which is ignored and data is
> lost. This can't be fixed by handling EAGAIN because an unknown amount
> of data has been written.
> 
> This patch works around the issue by re-opening the underlying tty
> file, which is available in Linux as /proc/self/fd/2, and duplicating
> it into fd 2 which causes stderr to refer to a new, unshared, "file
> description". Additionally, if we end up logging to a non-blocking
> file descriptor on a non-Linux platform, where no workaround has been
> implemented, we display a warning message.
> 
> Signed-off-by: Sam Bobroff <address@hidden>
> ---
> This is somewhat of an RFC as the issue probably hasn't been noticed
> very often (ever?) and there are several approaches to addressing it.
> The discussion was a while ago, here:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2015-12/msg00761.html
> 
>  include/sysemu/os-posix.h |  2 +-
>  include/sysemu/os-win32.h |  2 +-
>  os-posix.c                | 30 +++++++++++++++++++++++++++++-
>  os-win32.c                |  2 +-
>  vl.c                      |  2 +-
>  5 files changed, 33 insertions(+), 5 deletions(-)

Reviewed-by: Stefan Hajnoczi <address@hidden>

signature.asc
Description: PGP signature