* [Qemu-devel] [PULL 02/41] qemu-thread: fix races on threads that exit very quickly
2017-12-21 8:35 [Qemu-devel] [PULL v2 00/41] First batch of misc patches for QEMU 2.12 Paolo Bonzini
@ 2017-12-21 8:35 ` Paolo Bonzini
2017-12-21 8:35 ` [Qemu-devel] [PULL 37/41] chardev: fix backend events regression with mux chardev Paolo Bonzini
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2017-12-21 8:35 UTC (permalink / raw)
To: qemu-devel; +Cc: linzhecheng
From: linzhecheng <linzhecheng@huawei.com>
If we create a thread with QEMU_THREAD_DETACHED mode, QEMU may get a segfault with low probability.
The backtrace is:
#0 0x00007f46c60291d7 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1 0x00007f46c602a8c8 in __GI_abort () at abort.c:90
#2 0x00000000008543c9 in PAT_abort ()
#3 0x000000000085140d in patchIllInsHandler ()
#4 <signal handler called>
#5 pthread_detach (th=139933037614848) at pthread_detach.c:50
#6 0x0000000000829759 in qemu_thread_create (thread=thread@entry=0x7ffdaa8205e0, name=name@entry=0x94d94a "io-task-worker", start_routine=start_routine@entry=0x7eb9a0 <qio_task_thread_worker>,
arg=arg@entry=0x3f5cf70, mode=mode@entry=1) at util/qemu_thread_posix.c:512
#7 0x00000000007ebc96 in qio_task_run_in_thread (task=0x31db2c0, worker=worker@entry=0x7e7e40 <qio_channel_socket_connect_worker>, opaque=0xcd23380, destroy=0x7f1180 <qapi_free_SocketAddress>)
at io/task.c:141
#8 0x00000000007e7f33 in qio_channel_socket_connect_async (ioc=ioc@entry=0x626c0b0, addr=<optimized out>, callback=callback@entry=0x55e080 <qemu_chr_socket_connected>, opaque=opaque@entry=0x42862c0,
destroy=destroy@entry=0x0) at io/channel_socket.c:194
#9 0x000000000055bdd1 in socket_reconnect_timeout (opaque=0x42862c0) at qemu_char.c:4744
#10 0x00007f46c72483b3 in g_timeout_dispatch () from /usr/lib64/libglib-2.0.so.0
#11 0x00007f46c724799a in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#12 0x000000000076c646 in glib_pollfds_poll () at main_loop.c:228
#13 0x000000000076c6eb in os_host_main_loop_wait (timeout=348000000) at main_loop.c:273
#14 0x000000000076c815 in main_loop_wait (nonblocking=nonblocking@entry=0) at main_loop.c:521
#15 0x000000000056a511 in main_loop () at vl.c:2076
#16 0x0000000000420705 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4940
The cause of this problem is a glibc bug; for more information, see
https://sourceware.org/bugzilla/show_bug.cgi?id=19951.
The solution for this bug is to use pthread_attr_setdetachstate.
There is a similar issue with pthread_setname_np, which is moved
from creating thread to created thread.
Signed-off-by: linzhecheng <linzhecheng@huawei.com>
Message-Id: <20171128044656.10592-1-linzhecheng@huawei.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
[Simplify the code by removing qemu_thread_set_name, and free the arguments
before invoking the start routine. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-thread-posix.c | 59 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 42 insertions(+), 17 deletions(-)
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 7306475899..959a57079f 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -479,15 +479,29 @@ static void __attribute__((constructor)) qemu_thread_atexit_init(void)
}
-/* Attempt to set the threads name; note that this is for debug, so
- * we're not going to fail if we can't set it.
- */
-static void qemu_thread_set_name(QemuThread *thread, const char *name)
-{
#ifdef CONFIG_PTHREAD_SETNAME_NP
- pthread_setname_np(thread->thread, name);
-#endif
+typedef struct {
+ void *(*start_routine)(void *);
+ void *arg;
+ char *name;
+} QemuThreadArgs;
+
+static void *qemu_thread_start(void *args)
+{
+ QemuThreadArgs *qemu_thread_args = args;
+ void *(*start_routine)(void *) = qemu_thread_args->start_routine;
+ void *arg = qemu_thread_args->arg;
+
+ /* Attempt to set the threads name; note that this is for debug, so
+ * we're not going to fail if we can't set it.
+ */
+ pthread_setname_np(pthread_self(), qemu_thread_args->name);
+ g_free(qemu_thread_args->name);
+ g_free(qemu_thread_args);
+ return start_routine(arg);
}
+#endif
+
void qemu_thread_create(QemuThread *thread, const char *name,
void *(*start_routine)(void*),
@@ -502,23 +516,34 @@ void qemu_thread_create(QemuThread *thread, const char *name,
error_exit(err, __func__);
}
+ if (mode == QEMU_THREAD_DETACHED) {
+ pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+ }
+
/* Leave signal handling to the iothread. */
sigfillset(&set);
pthread_sigmask(SIG_SETMASK, &set, &oldset);
- err = pthread_create(&thread->thread, &attr, start_routine, arg);
- if (err)
- error_exit(err, __func__);
+#ifdef CONFIG_PTHREAD_SETNAME_NP
if (name_threads) {
- qemu_thread_set_name(thread, name);
+ QemuThreadArgs *qemu_thread_args;
+ qemu_thread_args = g_new0(QemuThreadArgs, 1);
+ qemu_thread_args->name = g_strdup(name);
+ qemu_thread_args->start_routine = start_routine;
+ qemu_thread_args->arg = arg;
+
+ err = pthread_create(&thread->thread, &attr,
+ qemu_thread_start, qemu_thread_args);
+ } else
+#endif
+ {
+ err = pthread_create(&thread->thread, &attr,
+ start_routine, arg);
}
- if (mode == QEMU_THREAD_DETACHED) {
- err = pthread_detach(thread->thread);
- if (err) {
- error_exit(err, __func__);
- }
- }
+ if (err)
+ error_exit(err, __func__);
+
pthread_sigmask(SIG_SETMASK, &oldset, NULL);
pthread_attr_destroy(&attr);
--
2.14.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PULL 37/41] chardev: fix backend events regression with mux chardev
2017-12-21 8:35 [Qemu-devel] [PULL v2 00/41] First batch of misc patches for QEMU 2.12 Paolo Bonzini
2017-12-21 8:35 ` [Qemu-devel] [PULL 02/41] qemu-thread: fix races on threads that exit very quickly Paolo Bonzini
@ 2017-12-21 8:35 ` Paolo Bonzini
2017-12-21 8:35 ` [Qemu-devel] [PULL 41/41] chardev: convert the socket server to QIONetListener Paolo Bonzini
2017-12-21 18:19 ` [Qemu-devel] [PULL v2 00/41] First batch of misc patches for QEMU 2.12 Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2017-12-21 8:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Kirill noticied that on recent versions on QEMU he was not able to
trigger SysRq to invoke debug capabilites of Linux Kernel. He tracked
it down to qemu_chr_be_event() ignoring CHR_EVENT_BREAK due s->be
being NULL. The bug was introduced in 2.8, commit a4afa548fc6d ("char:
move front end handlers in CharBackend"). Since the commit, the
qemu_chr_be_event() failed to deliver CHR_EVENT_BREAK due to
qemu_chr_fe_init() does not set s->be in case of mux.
Let's fix this by teaching mux to send an event to the frontend with
the focus.
Reported-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Fixes: a4afa548fc6d ("char: move front end handlers in CharBackend")
Message-Id: <20171103152824.21948-2-marcandre.lureau@redhat.com>
Tested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
chardev/char-mux.c | 10 ++++++++++
chardev/char.c | 18 ++++++++++++------
include/chardev/char.h | 1 +
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index 4cda5e7458..567bf965cd 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -123,6 +123,15 @@ static void mux_chr_send_event(MuxChardev *d, int mux_nr, int event)
}
}
+static void mux_chr_be_event(Chardev *chr, int event)
+{
+ MuxChardev *d = MUX_CHARDEV(chr);
+
+ if (d->focus != -1) {
+ mux_chr_send_event(d, d->focus, event);
+ }
+}
+
static int mux_proc_byte(Chardev *chr, MuxChardev *d, int ch)
{
if (d->term_got_escape) {
@@ -346,6 +355,7 @@ static void char_mux_class_init(ObjectClass *oc, void *data)
cc->chr_write = mux_chr_write;
cc->chr_accept_input = mux_chr_accept_input;
cc->chr_add_watch = mux_chr_add_watch;
+ cc->chr_be_event = mux_chr_be_event;
}
static const TypeInfo char_mux_type_info = {
diff --git a/chardev/char.c b/chardev/char.c
index 2ae4f465ec..8c3765ee99 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -43,10 +43,19 @@ static Object *get_chardevs_root(void)
return container_get(object_get_root(), "/chardevs");
}
-void qemu_chr_be_event(Chardev *s, int event)
+static void chr_be_event(Chardev *s, int event)
{
CharBackend *be = s->be;
+ if (!be || !be->chr_event) {
+ return;
+ }
+
+ be->chr_event(be->opaque, event);
+}
+
+void qemu_chr_be_event(Chardev *s, int event)
+{
/* Keep track if the char device is open */
switch (event) {
case CHR_EVENT_OPENED:
@@ -57,11 +66,7 @@ void qemu_chr_be_event(Chardev *s, int event)
break;
}
- if (!be || !be->chr_event) {
- return;
- }
-
- be->chr_event(be->opaque, event);
+ CHARDEV_GET_CLASS(s)->chr_be_event(s, event);
}
/* Not reporting errors from writing to logfile, as logs are
@@ -244,6 +249,7 @@ static void char_class_init(ObjectClass *oc, void *data)
ChardevClass *cc = CHARDEV_CLASS(oc);
cc->chr_write = null_chr_write;
+ cc->chr_be_event = chr_be_event;
}
static void char_finalize(Object *obj)
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 43aabccef5..778d610295 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -248,6 +248,7 @@ typedef struct ChardevClass {
void (*chr_accept_input)(Chardev *chr);
void (*chr_set_echo)(Chardev *chr, bool echo);
void (*chr_set_fe_open)(Chardev *chr, int fe_open);
+ void (*chr_be_event)(Chardev *s, int event);
} ChardevClass;
Chardev *qemu_chardev_new(const char *id, const char *typename,
--
2.14.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PULL 41/41] chardev: convert the socket server to QIONetListener
2017-12-21 8:35 [Qemu-devel] [PULL v2 00/41] First batch of misc patches for QEMU 2.12 Paolo Bonzini
2017-12-21 8:35 ` [Qemu-devel] [PULL 02/41] qemu-thread: fix races on threads that exit very quickly Paolo Bonzini
2017-12-21 8:35 ` [Qemu-devel] [PULL 37/41] chardev: fix backend events regression with mux chardev Paolo Bonzini
@ 2017-12-21 8:35 ` Paolo Bonzini
2017-12-21 18:19 ` [Qemu-devel] [PULL v2 00/41] First batch of misc patches for QEMU 2.12 Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2017-12-21 8:35 UTC (permalink / raw)
To: qemu-devel
From: "Daniel P. Berrange" <berrange@redhat.com>
Instead of creating a QIOChannelSocket directly for the chardev
server socket, use a QIONetListener. This provides the ability
to listen on multiple sockets at the same time, so enables
full support for IPv4/IPv6 dual stack.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20171218135417.28301-2-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
chardev/char-socket.c | 73 +++++++++++++++++++++------------------------------
1 file changed, 30 insertions(+), 43 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 53eda8ef00..630a7f2995 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -25,6 +25,7 @@
#include "chardev/char.h"
#include "io/channel-socket.h"
#include "io/channel-tls.h"
+#include "io/net-listener.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
#include "qapi/clone-visitor.h"
@@ -40,8 +41,7 @@ typedef struct {
Chardev parent;
QIOChannel *ioc; /* Client I/O channel */
QIOChannelSocket *sioc; /* Client master channel */
- QIOChannelSocket *listen_ioc;
- guint listen_tag;
+ QIONetListener *listener;
QCryptoTLSCreds *tls_creds;
int connected;
int max_size;
@@ -93,9 +93,9 @@ static void check_report_connect_error(Chardev *chr,
qemu_chr_socket_restart_timer(chr);
}
-static gboolean tcp_chr_accept(QIOChannel *chan,
- GIOCondition cond,
- void *opaque);
+static void tcp_chr_accept(QIONetListener *listener,
+ QIOChannelSocket *cioc,
+ void *opaque);
static int tcp_chr_read_poll(void *opaque);
static void tcp_chr_disconnect(Chardev *chr);
@@ -401,9 +401,9 @@ static void tcp_chr_disconnect(Chardev *chr)
tcp_chr_free_connection(chr);
- if (s->listen_ioc && s->listen_tag == 0) {
- s->listen_tag = qio_channel_add_watch(
- QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
+ if (s->listener) {
+ qio_net_listener_set_client_func(s->listener, tcp_chr_accept,
+ chr, NULL);
}
update_disconnected_filename(s);
if (emit_close) {
@@ -702,9 +702,8 @@ static int tcp_chr_new_client(Chardev *chr, QIOChannelSocket *sioc)
if (s->do_nodelay) {
qio_channel_set_delay(s->ioc, false);
}
- if (s->listen_tag) {
- g_source_remove(s->listen_tag);
- s->listen_tag = 0;
+ if (s->listener) {
+ qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
}
if (s->tls_creds) {
@@ -736,24 +735,14 @@ static int tcp_chr_add_client(Chardev *chr, int fd)
return ret;
}
-static gboolean tcp_chr_accept(QIOChannel *channel,
- GIOCondition cond,
- void *opaque)
+static void tcp_chr_accept(QIONetListener *listener,
+ QIOChannelSocket *cioc,
+ void *opaque)
{
Chardev *chr = CHARDEV(opaque);
- QIOChannelSocket *sioc;
-
- sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(channel),
- NULL);
- if (!sioc) {
- return TRUE;
- }
-
- tcp_chr_new_client(chr, sioc);
- object_unref(OBJECT(sioc));
-
- return TRUE;
+ tcp_chr_set_client_ioc_name(chr, cioc);
+ tcp_chr_new_client(chr, cioc);
}
static int tcp_chr_wait_connected(Chardev *chr, Error **errp)
@@ -767,9 +756,10 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp)
if (s->is_listen) {
info_report("QEMU waiting for connection on: %s",
chr->filename);
- qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL);
- tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr);
- qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), false, NULL);
+ sioc = qio_net_listener_wait_client(s->listener);
+ tcp_chr_set_client_ioc_name(chr, sioc);
+ tcp_chr_new_client(chr, sioc);
+ object_unref(OBJECT(sioc));
} else {
sioc = qio_channel_socket_new();
tcp_chr_set_client_ioc_name(chr, sioc);
@@ -797,12 +787,9 @@ static void char_socket_finalize(Object *obj)
s->reconnect_timer = 0;
}
qapi_free_SocketAddress(s->addr);
- if (s->listen_tag) {
- g_source_remove(s->listen_tag);
- s->listen_tag = 0;
- }
- if (s->listen_ioc) {
- object_unref(OBJECT(s->listen_ioc));
+ if (s->listener) {
+ qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
+ object_unref(OBJECT(s->listener));
}
if (s->tls_creds) {
object_unref(OBJECT(s->tls_creds));
@@ -935,29 +922,29 @@ static void qmp_chardev_open_socket(Chardev *chr,
} else {
if (s->is_listen) {
char *name;
- sioc = qio_channel_socket_new();
+ s->listener = qio_net_listener_new();
name = g_strdup_printf("chardev-tcp-listener-%s", chr->label);
- qio_channel_set_name(QIO_CHANNEL(sioc), name);
+ qio_net_listener_set_name(s->listener, name);
g_free(name);
- if (qio_channel_socket_listen_sync(sioc, s->addr, errp) < 0) {
+ if (qio_net_listener_open_sync(s->listener, s->addr, errp) < 0) {
+ object_unref(OBJECT(s->listener));
+ s->listener = NULL;
goto error;
}
qapi_free_SocketAddress(s->addr);
- s->addr = socket_local_address(sioc->fd, errp);
+ s->addr = socket_local_address(s->listener->sioc[0]->fd, errp);
update_disconnected_filename(s);
- s->listen_ioc = sioc;
if (is_waitconnect &&
qemu_chr_wait_connected(chr, errp) < 0) {
return;
}
if (!s->ioc) {
- s->listen_tag = qio_channel_add_watch(
- QIO_CHANNEL(s->listen_ioc), G_IO_IN,
- tcp_chr_accept, chr, NULL);
+ qio_net_listener_set_client_func(s->listener, tcp_chr_accept,
+ chr, NULL);
}
} else if (qemu_chr_wait_connected(chr, errp) < 0) {
goto error;
--
2.14.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PULL v2 00/41] First batch of misc patches for QEMU 2.12
2017-12-21 8:35 [Qemu-devel] [PULL v2 00/41] First batch of misc patches for QEMU 2.12 Paolo Bonzini
` (2 preceding siblings ...)
2017-12-21 8:35 ` [Qemu-devel] [PULL 41/41] chardev: convert the socket server to QIONetListener Paolo Bonzini
@ 2017-12-21 18:19 ` Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2017-12-21 18:19 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On 21 December 2017 at 08:35, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 200780a3a3ed067dfb2e0d2210b0ed09e748ba26:
>
> Merge remote-tracking branch 'remotes/armbru/tags/pull-cmdline-2017-12-18-v2' into staging (2017-12-20 13:20:48 +0000)
>
> are available in the Git repository at:
>
> git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 194b7f0d448361dd58d2f7f189147cf075988255:
>
> chardev: convert the socket server to QIONetListener (2017-12-21 09:30:32 +0100)
>
> ----------------------------------------------------------------
> * NBD and chardev conversion to QIONetListener (Daniel)
> * MTTCG fixes (David)
> * Hyper-V fixes (Roman, Evgeny)
> * share-rw option (Fam)
> * Mux chardev event bugfix (Marc-André)
> * Add systemd unit files in contrib/ (me)
> * SCSI and block/iscsi.c bugfixes (me, Peter L.)
> * unassigned_mem_ops fixes (Peter M.)
> * VEX decoding fix (Peter M.)
> * "info pic" and "info irq" improvements (Peter Xu)
> * vmport trace events (Philippe)
> * Braille chardev bugfix (Samuel)
> * Compiler warnings fix (Stefan)
> * initial support for TCG smoke test of more boards (Thomas)
> * New CPU features (Yang)
> * Reduce startup memory usage (Yang)
> * QemuThread race fix (linhecheng)
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread