Patchwork build failure with coroutine-gthread

login
register
mail settings
Submitter Aneesh Kumar K.V
Date Aug. 8, 2011, 9:01 a.m.
Message ID <87fwlc9t0v.fsf@skywalker.in.ibm.com>
Download mbox | patch
Permalink /patch/108869/
State New
Headers show

Comments

Aneesh Kumar K.V - Aug. 8, 2011, 9:01 a.m.
LINK  qemu-ga
coroutine-gthread.o: In function `coroutine_init':
/home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init'
collect2: ld returned 1 exit status

The below patch fix the failure.  I also added the patch in the VirtFS
pull request.

commit 4b76a481ee28166d5f415ef97833c624f4fc0792
Author: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Date:   Mon Aug 8 13:04:05 2011 +0530

    coroutine: add gthread dependency
    
    Commit 1fc7bd4a86a2bfeafcec29445871eb97469a2699 removed the gthread and
    gio dependency since qemu-ga did not require it.  Coroutines require
    gthread, so add it back in.
    
    Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Stefan Hajnoczi - Aug. 8, 2011, 9:29 a.m.
On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
>
>
>  LINK  qemu-ga
> coroutine-gthread.o: In function `coroutine_init':
> /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init'
> collect2: ld returned 1 exit status
>
> The below patch fix the failure.  I also added the patch in the VirtFS
> pull request.

It appears coroutine-core v7 was merged instead of v8.  The
differences are minor:
 * Bisectability: introduce gthread implementation before ucontext/fibers
 * Depend on gthread, not just glib, for multithreaded programming

Here is the patchwork link to the gthread dependency patch in case a
committer wants to merge it immediately:
http://patchwork.ozlabs.org/patch/106810/

Stefan
Aneesh Kumar K.V - Aug. 8, 2011, 10:48 a.m.
On Mon, 8 Aug 2011 10:29:04 +0100, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >
> >
> >  LINK  qemu-ga
> > coroutine-gthread.o: In function `coroutine_init':
> > /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init'
> > collect2: ld returned 1 exit status
> >
> > The below patch fix the failure.  I also added the patch in the VirtFS
> > pull request.
> 
> It appears coroutine-core v7 was merged instead of v8.  The
> differences are minor:
>  * Bisectability: introduce gthread implementation before ucontext/fibers
>  * Depend on gthread, not just glib, for multithreaded programming
> 
> Here is the patchwork link to the gthread dependency patch in case a
> committer wants to merge it immediately:
> http://patchwork.ozlabs.org/patch/106810/
> 

I added a version that applies to master in the pull request

http://article.gmane.org/gmane.comp.emulators.qemu/112648

Message-id:"87bow09sw1.fsf@skywalker.in.ibm.com"

http://repo.or.cz/w/qemu/v9fs.git/commit/4b76a481ee28166d5f415ef97833c624f4fc0792

-aneesh
Blue Swirl - Aug. 9, 2011, 5:28 p.m.
On Mon, Aug 8, 2011 at 9:29 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V
> <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>
>>
>>  LINK  qemu-ga
>> coroutine-gthread.o: In function `coroutine_init':
>> /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init'
>> collect2: ld returned 1 exit status
>>
>> The below patch fix the failure.  I also added the patch in the VirtFS
>> pull request.
>
> It appears coroutine-core v7 was merged instead of v8.  The
> differences are minor:
>  * Bisectability: introduce gthread implementation before ucontext/fibers
>  * Depend on gthread, not just glib, for multithreaded programming
>
> Here is the patchwork link to the gthread dependency patch in case a
> committer wants to merge it immediately:
> http://patchwork.ozlabs.org/patch/106810/

Coroutines only need gthreads when both ucontext and win32 versions
are unavailable. This is handled better by my patch:
http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00998.html
Aneesh Kumar K.V - Aug. 9, 2011, 5:46 p.m.
On Tue, 9 Aug 2011 17:28:17 +0000, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Mon, Aug 8, 2011 at 9:29 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V
> > <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >>
> >>
> >>  LINK  qemu-ga
> >> coroutine-gthread.o: In function `coroutine_init':
> >> /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init'
> >> collect2: ld returned 1 exit status
> >>
> >> The below patch fix the failure.  I also added the patch in the VirtFS
> >> pull request.
> >
> > It appears coroutine-core v7 was merged instead of v8.  The
> > differences are minor:
> >  * Bisectability: introduce gthread implementation before ucontext/fibers
> >  * Depend on gthread, not just glib, for multithreaded programming
> >
> > Here is the patchwork link to the gthread dependency patch in case a
> > committer wants to merge it immediately:
> > http://patchwork.ozlabs.org/patch/106810/
> 
> Coroutines only need gthreads when both ucontext and win32 versions
> are unavailable. This is handled better by my patch:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00998.html

We would soon need that for VirtFS. So that will be another check of 
if (linux && atttr )

-aneesh
Stefan Hajnoczi - Aug. 10, 2011, 9:14 a.m.
On Tue, Aug 09, 2011 at 05:28:17PM +0000, Blue Swirl wrote:
> On Mon, Aug 8, 2011 at 9:29 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> > On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V
> > <aneesh.kumar@linux.vnet.ibm.com> wrote:
> >>
> >>
> >>  LINK  qemu-ga
> >> coroutine-gthread.o: In function `coroutine_init':
> >> /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init'
> >> collect2: ld returned 1 exit status
> >>
> >> The below patch fix the failure.  I also added the patch in the VirtFS
> >> pull request.
> >
> > It appears coroutine-core v7 was merged instead of v8.  The
> > differences are minor:
> >  * Bisectability: introduce gthread implementation before ucontext/fibers
> >  * Depend on gthread, not just glib, for multithreaded programming
> >
> > Here is the patchwork link to the gthread dependency patch in case a
> > committer wants to merge it immediately:
> > http://patchwork.ozlabs.org/patch/106810/
> 
> Coroutines only need gthreads when both ucontext and win32 versions
> are unavailable. This is handled better by my patch:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00998.html

Instead of adding more conditions around glib we should unconditionally
depend on it.  Dependencies always have an associated cost but I intend
to use glib instead of reimplementing platform abstractions and common
data structures.

The cost of maintaining non-emulation/non-virtualization code isn't
worth avoiding the dependency.  In fact I wish glib provided coroutines
so we didn't need to implement them ourselves.

Stefan
Blue Swirl - Aug. 10, 2011, 5:24 p.m.
On Wed, Aug 10, 2011 at 9:14 AM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> On Tue, Aug 09, 2011 at 05:28:17PM +0000, Blue Swirl wrote:
>> On Mon, Aug 8, 2011 at 9:29 AM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> > On Mon, Aug 8, 2011 at 10:01 AM, Aneesh Kumar K.V
>> > <aneesh.kumar@linux.vnet.ibm.com> wrote:
>> >>
>> >>
>> >>  LINK  qemu-ga
>> >> coroutine-gthread.o: In function `coroutine_init':
>> >> /home/opensource/sources/qemu/qemu-upstream/coroutine-gthread.c:39: undefined reference to `g_thread_init'
>> >> collect2: ld returned 1 exit status
>> >>
>> >> The below patch fix the failure.  I also added the patch in the VirtFS
>> >> pull request.
>> >
>> > It appears coroutine-core v7 was merged instead of v8.  The
>> > differences are minor:
>> >  * Bisectability: introduce gthread implementation before ucontext/fibers
>> >  * Depend on gthread, not just glib, for multithreaded programming
>> >
>> > Here is the patchwork link to the gthread dependency patch in case a
>> > committer wants to merge it immediately:
>> > http://patchwork.ozlabs.org/patch/106810/
>>
>> Coroutines only need gthreads when both ucontext and win32 versions
>> are unavailable. This is handled better by my patch:
>> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00998.html
>
> Instead of adding more conditions around glib we should unconditionally
> depend on it.  Dependencies always have an associated cost but I intend
> to use glib instead of reimplementing platform abstractions and common
> data structures.

I also thought glib dependency was going to be unconditional, but then
this conditional dependency was introduced. Either make it
unconditional or fix the condition.

> The cost of maintaining non-emulation/non-virtualization code isn't
> worth avoiding the dependency.  In fact I wish glib provided coroutines
> so we didn't need to implement them ourselves.
>
> Stefan
>

Patch

diff --git a/configure b/configure
index 0c67a4a..a6687f7 100755
--- a/configure
+++ b/configure
@@ -1844,16 +1844,14 @@  fi
 
 ##########################################
 # glib support probe
-if test "$guest_agent" != "no" ; then
-    if $pkg_config --modversion glib-2.0 > /dev/null 2>&1 ; then
-        glib_cflags=`$pkg_config --cflags glib-2.0 2>/dev/null`
-        glib_libs=`$pkg_config --libs glib-2.0 2>/dev/null`
-        libs_softmmu="$glib_libs $libs_softmmu"
-        libs_tools="$glib_libs $libs_tools"
-    else
-        echo "glib-2.0 required to compile QEMU"
-        exit 1
-    fi
+if $pkg_config --modversion gthread-2.0 > /dev/null 2>&1 ; then
+    glib_cflags=`$pkg_config --cflags gthread-2.0 2>/dev/null`
+    glib_libs=`$pkg_config --libs gthread-2.0 2>/dev/null`
+    libs_softmmu="$glib_libs $libs_softmmu"
+    libs_tools="$glib_libs $libs_tools"
+else
+    echo "glib-2.0 required to compile QEMU"
+    exit 1
 fi
 
 ##########################################