From: "Daniel P. Berrange" <berrange@redhat.com>

The GThread implementation is not functional enough to actually
run QEMU reliably. While it was potentially useful for debugging,
we have a scripts/qemugdb/coroutine.py to enable tracing of
ucontext coroutines in GDB, so that removes the only reason for
GThread to exist.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 configure                |  19 ++---
 util/coroutine-gthread.c | 198 -----------------------------------------------
 .travis.yml              |   5 +-
 3 files changed, 6 insertions(+), 216 deletions(-)
 delete mode 100644 util/coroutine-gthread.c

diff --git a/configure b/configure
index 7c020c0..39e727e 100755
--- a/configure
+++ b/configure
@@ -1334,7 +1334,7 @@ Advanced options (experts only):
   --oss-lib                path to OSS library
   --cpu=CPU                Build for host CPU [$cpu]
   --with-coroutine=BACKEND coroutine backend. Supported options:
-                           gthread, ucontext, sigaltstack, windows
+                           ucontext, sigaltstack, windows
   --enable-gcov            enable test coverage analysis with gcov
   --gcov=GCOV              use specified gcov [$gcov_tool]
   --disable-blobs          disable installing provided firmware blobs
@@ -4418,10 +4418,8 @@ fi
 # check and set a backend for coroutine
 
 # We prefer ucontext, but it's not always possible. The fallback
-# is sigcontext. gthread is not selectable except explicitly, because
-# it is not functional enough to run QEMU proper. (It is occasionally
-# useful for debugging purposes.)  On Windows the only valid backend
-# is the Windows-specific one.
+# is sigcontext. On Windows the only valid backend is the Windows
+# specific one.
 
 ucontext_works=no
 if test "$darwin" != "yes"; then
@@ -4460,7 +4458,7 @@ else
       feature_not_found "ucontext"
     fi
     ;;
-  gthread|sigaltstack)
+  sigaltstack)
     if test "$mingw32" = "yes"; then
       error_exit "only the 'windows' coroutine backend is valid for Windows"
     fi
@@ -4472,14 +4470,7 @@ else
 fi
 
 if test "$coroutine_pool" = ""; then
-  if test "$coroutine" = "gthread"; then
-    coroutine_pool=no
-  else
-    coroutine_pool=yes
-  fi
-fi
-if test "$coroutine" = "gthread" -a "$coroutine_pool" = "yes"; then
-  error_exit "'gthread' coroutine backend does not support pool (use --disable-coroutine-pool)"
+  coroutine_pool=yes
 fi
 
 if test "$debug_stack_usage" = "yes"; then
diff --git a/util/coroutine-gthread.c b/util/coroutine-gthread.c
deleted file mode 100644
index 62bfb40..0000000
--- a/util/coroutine-gthread.c
+++ /dev/null
@@ -1,198 +0,0 @@
-/*
- * GThread coroutine initialization code
- *
- * Copyright (C) 2006  Anthony Liguori <anthony@codemonkey.ws>
- * Copyright (C) 2011  Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.0 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "qemu/coroutine_int.h"
-
-typedef struct {
-    Coroutine base;
-    GThread *thread;
-    bool runnable;
-    bool free_on_thread_exit;
-    CoroutineAction action;
-} CoroutineGThread;
-
-static CompatGMutex coroutine_lock;
-static CompatGCond coroutine_cond;
-
-/* GLib 2.31 and beyond deprecated various parts of the thread API,
- * but the new interfaces are not available in older GLib versions
- * so we have to cope with both.
- */
-#if GLIB_CHECK_VERSION(2, 31, 0)
-/* Awkwardly, the GPrivate API doesn't provide a way to update the
- * GDestroyNotify handler for the coroutine key dynamically. So instead
- * we track whether or not the CoroutineGThread should be freed on
- * thread exit / coroutine key update using the free_on_thread_exit
- * field.
- */
-static void coroutine_destroy_notify(gpointer data)
-{
-    CoroutineGThread *co = data;
-    if (co && co->free_on_thread_exit) {
-        g_free(co);
-    }
-}
-
-static GPrivate coroutine_key = G_PRIVATE_INIT(coroutine_destroy_notify);
-
-static inline CoroutineGThread *get_coroutine_key(void)
-{
-    return g_private_get(&coroutine_key);
-}
-
-static inline void set_coroutine_key(CoroutineGThread *co,
-                                     bool free_on_thread_exit)
-{
-    /* Unlike g_static_private_set() this does not call the GDestroyNotify
-     * if the previous value of the key was NULL. Fortunately we only need
-     * the GDestroyNotify in the non-NULL key case.
-     */
-    co->free_on_thread_exit = free_on_thread_exit;
-    g_private_replace(&coroutine_key, co);
-}
-
-static inline GThread *create_thread(GThreadFunc func, gpointer data)
-{
-    return g_thread_new("coroutine", func, data);
-}
-
-#else
-
-/* Handle older GLib versions */
-
-static GStaticPrivate coroutine_key = G_STATIC_PRIVATE_INIT;
-
-static inline CoroutineGThread *get_coroutine_key(void)
-{
-    return g_static_private_get(&coroutine_key);
-}
-
-static inline void set_coroutine_key(CoroutineGThread *co,
-                                     bool free_on_thread_exit)
-{
-    g_static_private_set(&coroutine_key, co,
-                         free_on_thread_exit ? (GDestroyNotify)g_free : NULL);
-}
-
-static inline GThread *create_thread(GThreadFunc func, gpointer data)
-{
-    return g_thread_create_full(func, data, 0, TRUE, TRUE,
-                                G_THREAD_PRIORITY_NORMAL, NULL);
-}
-
-#endif
-
-
-static void __attribute__((constructor)) coroutine_init(void)
-{
-#if !GLIB_CHECK_VERSION(2, 31, 0)
-    if (!g_thread_supported()) {
-        g_thread_init(NULL);
-    }
-#endif
-}
-
-static void coroutine_wait_runnable_locked(CoroutineGThread *co)
-{
-    while (!co->runnable) {
-        g_cond_wait(&coroutine_cond, &coroutine_lock);
-    }
-}
-
-static void coroutine_wait_runnable(CoroutineGThread *co)
-{
-    g_mutex_lock(&coroutine_lock);
-    coroutine_wait_runnable_locked(co);
-    g_mutex_unlock(&coroutine_lock);
-}
-
-static gpointer coroutine_thread(gpointer opaque)
-{
-    CoroutineGThread *co = opaque;
-
-    set_coroutine_key(co, false);
-    coroutine_wait_runnable(co);
-    co->base.entry(co->base.entry_arg);
-    qemu_coroutine_switch(&co->base, co->base.caller, COROUTINE_TERMINATE);
-    return NULL;
-}
-
-Coroutine *qemu_coroutine_new(void)
-{
-    CoroutineGThread *co;
-
-    co = g_malloc0(sizeof(*co));
-    co->thread = create_thread(coroutine_thread, co);
-    if (!co->thread) {
-        g_free(co);
-        return NULL;
-    }
-    return &co->base;
-}
-
-void qemu_coroutine_delete(Coroutine *co_)
-{
-    CoroutineGThread *co = DO_UPCAST(CoroutineGThread, base, co_);
-
-    g_thread_join(co->thread);
-    g_free(co);
-}
-
-CoroutineAction qemu_coroutine_switch(Coroutine *from_,
-                                      Coroutine *to_,
-                                      CoroutineAction action)
-{
-    CoroutineGThread *from = DO_UPCAST(CoroutineGThread, base, from_);
-    CoroutineGThread *to = DO_UPCAST(CoroutineGThread, base, to_);
-
-    g_mutex_lock(&coroutine_lock);
-    from->runnable = false;
-    from->action = action;
-    to->runnable = true;
-    to->action = action;
-    g_cond_broadcast(&coroutine_cond);
-
-    if (action != COROUTINE_TERMINATE) {
-        coroutine_wait_runnable_locked(from);
-    }
-    g_mutex_unlock(&coroutine_lock);
-    return from->action;
-}
-
-Coroutine *qemu_coroutine_self(void)
-{
-    CoroutineGThread *co = get_coroutine_key();
-    if (!co) {
-        co = g_malloc0(sizeof(*co));
-        co->runnable = true;
-        set_coroutine_key(co, true);
-    }
-
-    return &co->base;
-}
-
-bool qemu_in_coroutine(void)
-{
-    CoroutineGThread *co = get_coroutine_key();
-
-    return co && co->base.caller;
-}
diff --git a/.travis.yml b/.travis.yml
index 9008a79..27a2d9c 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -86,9 +86,6 @@ matrix:
     - env: CONFIG="--enable-trace-backends=ust"
            TEST_CMD=""
       compiler: gcc
-    - env: CONFIG="--with-coroutine=gthread"
-           TEST_CMD=""
-      compiler: gcc
     - env: CONFIG=""
       os: osx
       compiler: clang
@@ -191,7 +188,7 @@ matrix:
       compiler: none
       env:
         - COMPILER_NAME=gcc CXX=g++-5 CC=gcc-5
-        - CONFIG="--cc=gcc-5 --cxx=g++-5 --disable-pie --disable-linux-user --with-coroutine=gthread"
+        - CONFIG="--cc=gcc-5 --cxx=g++-5 --disable-pie --disable-linux-user"
         - TEST_CMD=""
       before_script:
         - ./configure ${CONFIG} --extra-cflags="-g3 -O0 -fsanitize=thread -fuse-ld=gold" || cat config.log
-- 
2.9.3

On 05/12/2017 09:37 AM, Stefan Hajnoczi wrote:
> From: "Daniel P. Berrange" <berrange@redhat.com>
> 
> The GThread implementation is not functional enough to actually
> run QEMU reliably. While it was potentially useful for debugging,
> we have a scripts/qemugdb/coroutine.py to enable tracing of
> ucontext coroutines in GDB, so that removes the only reason for
> GThread to exist.
> 

Quick question (unrelated to the pull request): Can we get a patch to
scripts/qemugdb/coroutine.py or nearby that mentions the preferred way
to actually use the script?  I didn't know the script existed before
seeing this commit message, and was sad to see that it was not
self-documenting (or else a scripts/qemugdb/README file) on how best to
utilize the script (is it as simple as running 'gdb --some-arg
/path/to/coroutine.py'? do you have to install some magic file to tell
gdb to autoload the file any time you are debugging qemu? something
else???).

Okay, I searched a bit harder, and found that scripts/qemu-gdb.py loads
these subscripts, if you type 'source scripts/qemu-gdb.py' at the (gdb)
prompt.  Still, is there a way to automate that so that the scripts are
loaded every time you fire up gdb, instead of having to remember to do
it by hand? And if so, can that be documented?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On Fri, May 12, 2017 at 03:19:18PM -0500, Eric Blake wrote:
> On 05/12/2017 09:37 AM, Stefan Hajnoczi wrote:
> > From: "Daniel P. Berrange" <berrange@redhat.com>
> > 
> > The GThread implementation is not functional enough to actually
> > run QEMU reliably. While it was potentially useful for debugging,
> > we have a scripts/qemugdb/coroutine.py to enable tracing of
> > ucontext coroutines in GDB, so that removes the only reason for
> > GThread to exist.
> > 
> 
> Quick question (unrelated to the pull request): Can we get a patch to
> scripts/qemugdb/coroutine.py or nearby that mentions the preferred way
> to actually use the script?  I didn't know the script existed before
> seeing this commit message, and was sad to see that it was not
> self-documenting (or else a scripts/qemugdb/README file) on how best to
> utilize the script (is it as simple as running 'gdb --some-arg
> /path/to/coroutine.py'? do you have to install some magic file to tell
> gdb to autoload the file any time you are debugging qemu? something
> else???).
> 
> Okay, I searched a bit harder, and found that scripts/qemu-gdb.py loads
> these subscripts, if you type 'source scripts/qemu-gdb.py' at the (gdb)
> prompt.  Still, is there a way to automate that so that the scripts are
> loaded every time you fire up gdb, instead of having to remember to do
> it by hand? And if so, can that be documented?

I'll send a patch to add a .gdbinit file.

Stefan