From: "Richard W.M. Jones" <rjones@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] char: kick main loop after adding a watch
Date: Fri, 31 Mar 2017 18:09:30 +0100 [thread overview]
Message-ID: <20170331170930.GJ13825@redhat.com> (raw)
In-Reply-To: <20170331164322.24020-1-stefanha@redhat.com>
On Fri, Mar 31, 2017 at 05:43:22PM +0100, Stefan Hajnoczi wrote:
> The ISA serial port device's output can hang when the pipe on stdout
> becomes full. This is a race condition where the vcpu thread executing
> serial emulation code adds a watch on stdout while the main loop thread
> is blocked in ppoll(2). If no timer or other event wakes up the main
> loop, there will be no further output from the serial device even when
> the pipe becomes writable.
>
> Richard W. M. Jones was able to reproduce the hang on recent versions of
> guestfs-tools-c and libglib2 on Fedora 26 hosts.
>
> This patch kicks the main loop so the next iteration invokes ppoll(2)
> with the watch fd.
>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1435432
> Reported-by: Richard W. M. Jones <rjones@redhat.com>
> Tested-by: Richard W. M. Jones <rjones@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> chardev/char.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 3df1163..6c99c34 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -1059,6 +1059,11 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
> tag = g_source_attach(src, NULL);
> g_source_unref(src);
>
> + /* The main loop may be in blocked waiting on events in another thread.
> + * Kick it so the new watch will be added.
> + */
> + qemu_notify_event();
> +
> return tag;
> }
>
Obviously Paolo has provided further insights in the other part of
this thread, but from my point of view this does appear to fix the
problem. I tested it on 3 different physical machines which
previously showed hangs, and on all 3 the test ran for > 30 mins
[before I killed the test] which is far longer than seen before.
Therefore: ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org
prev parent reply other threads:[~2017-03-31 17:09 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-31 16:43 [Qemu-devel] [PATCH] char: kick main loop after adding a watch Stefan Hajnoczi
2017-03-31 16:53 ` Paolo Bonzini
2017-03-31 17:31 ` Richard W.M. Jones
2017-03-31 17:31 ` Paolo Bonzini
2017-03-31 17:40 ` Richard W.M. Jones
2017-04-01 10:08 ` Paolo Bonzini
2017-04-03 13:20 ` Stefan Hajnoczi
2017-03-31 17:09 ` Richard W.M. Jones [this message]
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=20170331170930.GJ13825@redhat.com \
--to=rjones@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.