| Message ID | CAAu8pHvdmHsQUn7ed4-A3goTQsWSRW=e_Uz3X8-gkNKggyMp4Q@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Blue Swirl writes: > 957f1f99f263d57612807a9535f75ca4473f05f0 didn't consider > that qemu-timer-common.o is needed by simpletrace. > Fix by adding it to qga object list. > Signed-off-by: Blue Swirl <blauwirbel@gmail.com> > --- > Makefile.objs | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > diff --git a/Makefile.objs b/Makefile.objs > index d1f3e5d..df11aec 100644 > --- a/Makefile.objs > +++ b/Makefile.objs > @@ -404,6 +404,9 @@ qga-obj-y = $(addprefix qga/, $(qga-nested-y)) > qga-obj-y += qemu-ga.o qemu-tool.o qemu-error.o qemu-sockets.o > module.o qemu-option.o cutils.o osdep.o > qga-obj-$(CONFIG_WIN32) += oslib-win32.o > qga-obj-$(CONFIG_POSIX) += oslib-posix.o > +ifeq ($(TRACE_BACKEND),simple) > +qga-obj-y += qemu-timer-common.o > +endif > vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) I sent a patch that should fix it for everybody linking with the tracing objects: http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html Lluis
On Fri, Aug 26, 2011 at 7:12 PM, Lluís <xscript@gmx.net> wrote: > Blue Swirl writes: > >> 957f1f99f263d57612807a9535f75ca4473f05f0 didn't consider >> that qemu-timer-common.o is needed by simpletrace. > >> Fix by adding it to qga object list. > >> Signed-off-by: Blue Swirl <blauwirbel@gmail.com> >> --- >> Makefile.objs | 3 +++ >> 1 files changed, 3 insertions(+), 0 deletions(-) > >> diff --git a/Makefile.objs b/Makefile.objs >> index d1f3e5d..df11aec 100644 >> --- a/Makefile.objs >> +++ b/Makefile.objs >> @@ -404,6 +404,9 @@ qga-obj-y = $(addprefix qga/, $(qga-nested-y)) >> qga-obj-y += qemu-ga.o qemu-tool.o qemu-error.o qemu-sockets.o >> module.o qemu-option.o cutils.o osdep.o >> qga-obj-$(CONFIG_WIN32) += oslib-win32.o >> qga-obj-$(CONFIG_POSIX) += oslib-posix.o >> +ifeq ($(TRACE_BACKEND),simple) >> +qga-obj-y += qemu-timer-common.o >> +endif > >> vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) > > I sent a patch that should fix it for everybody linking with the tracing > objects: > > http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html With your patch, there are warnings from linker: ../qemu-timer-common.o: warning: multiple common of `use_rt_clock' ../qemu-timer-common.o: warning: previous common is here
Blue Swirl writes: > On Fri, Aug 26, 2011 at 7:12 PM, Lluís <xscript@gmx.net> wrote: >> Blue Swirl writes: >> >>> 957f1f99f263d57612807a9535f75ca4473f05f0 didn't consider >>> that qemu-timer-common.o is needed by simpletrace. >> >>> Fix by adding it to qga object list. >> >>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com> >>> --- >>> Makefile.objs | 3 +++ >>> 1 files changed, 3 insertions(+), 0 deletions(-) >> >>> diff --git a/Makefile.objs b/Makefile.objs >>> index d1f3e5d..df11aec 100644 >>> --- a/Makefile.objs >>> +++ b/Makefile.objs >>> @@ -404,6 +404,9 @@ qga-obj-y = $(addprefix qga/, $(qga-nested-y)) >>> qga-obj-y += qemu-ga.o qemu-tool.o qemu-error.o qemu-sockets.o >>> module.o qemu-option.o cutils.o osdep.o >>> qga-obj-$(CONFIG_WIN32) += oslib-win32.o >>> qga-obj-$(CONFIG_POSIX) += oslib-posix.o >>> +ifeq ($(TRACE_BACKEND),simple) >>> +qga-obj-y += qemu-timer-common.o >>> +endif >> >>> vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) >> >> I sent a patch that should fix it for everybody linking with the tracing >> objects: >> >> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html > With your patch, there are warnings from linker: > ../qemu-timer-common.o: warning: multiple common of `use_rt_clock' > ../qemu-timer-common.o: warning: previous common is here Ah, yes. These extra errors are fixed by the duplicate elimination patch :) http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02987.html So, you need both to keep it clean. Lluis
On Sat, Aug 27, 2011 at 5:56 PM, Lluís <xscript@gmx.net> wrote: > Blue Swirl writes: > >> On Fri, Aug 26, 2011 at 7:12 PM, Lluís <xscript@gmx.net> wrote: >>> Blue Swirl writes: >>> >>>> 957f1f99f263d57612807a9535f75ca4473f05f0 didn't consider >>>> that qemu-timer-common.o is needed by simpletrace. >>> >>>> Fix by adding it to qga object list. >>> >>>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com> >>>> --- >>>> Makefile.objs | 3 +++ >>>> 1 files changed, 3 insertions(+), 0 deletions(-) >>> >>>> diff --git a/Makefile.objs b/Makefile.objs >>>> index d1f3e5d..df11aec 100644 >>>> --- a/Makefile.objs >>>> +++ b/Makefile.objs >>>> @@ -404,6 +404,9 @@ qga-obj-y = $(addprefix qga/, $(qga-nested-y)) >>>> qga-obj-y += qemu-ga.o qemu-tool.o qemu-error.o qemu-sockets.o >>>> module.o qemu-option.o cutils.o osdep.o >>>> qga-obj-$(CONFIG_WIN32) += oslib-win32.o >>>> qga-obj-$(CONFIG_POSIX) += oslib-posix.o >>>> +ifeq ($(TRACE_BACKEND),simple) >>>> +qga-obj-y += qemu-timer-common.o >>>> +endif >>> >>>> vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) >>> >>> I sent a patch that should fix it for everybody linking with the tracing >>> objects: >>> >>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html > >> With your patch, there are warnings from linker: >> ../qemu-timer-common.o: warning: multiple common of `use_rt_clock' >> ../qemu-timer-common.o: warning: previous common is here > > Ah, yes. These extra errors are fixed by the duplicate elimination patch > :) > > http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02987.html > > So, you need both to keep it clean. Using the sort function looks hackish to me. Maybe the linkage should be controlled by configure instead?
Blue Swirl writes: > On Sat, Aug 27, 2011 at 5:56 PM, Lluís <xscript@gmx.net> wrote: >>>> I sent a patch that should fix it for everybody linking with the tracing >>>> objects: >>>> >>>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html >> >>> With your patch, there are warnings from linker: >>> ../qemu-timer-common.o: warning: multiple common of `use_rt_clock' >>> ../qemu-timer-common.o: warning: previous common is here >> >> Ah, yes. These extra errors are fixed by the duplicate elimination patch >> :) >> >> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02987.html >> >> So, you need both to keep it clean. > Using the sort function looks hackish to me. Maybe the linkage should > be controlled by configure instead? What do you mean? Moving the logic for selecting the object files to link with on each top-level target out into the configure? In any case, I think that adding qemu-timer-common.o into trace-obj-y is the cleanest way, as otherwise the object needs to be added again and again depending on conditions that are checked multiple times, which I think will lead to to makefile maintenance headaches in the long run. Lluis
On Sun, Aug 28, 2011 at 6:13 PM, Lluís <xscript@gmx.net> wrote: > Blue Swirl writes: > >> On Sat, Aug 27, 2011 at 5:56 PM, Lluís <xscript@gmx.net> wrote: >>>>> I sent a patch that should fix it for everybody linking with the tracing >>>>> objects: >>>>> >>>>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html >>> >>>> With your patch, there are warnings from linker: >>>> ../qemu-timer-common.o: warning: multiple common of `use_rt_clock' >>>> ../qemu-timer-common.o: warning: previous common is here >>> >>> Ah, yes. These extra errors are fixed by the duplicate elimination patch >>> :) >>> >>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02987.html >>> >>> So, you need both to keep it clean. > >> Using the sort function looks hackish to me. Maybe the linkage should >> be controlled by configure instead? > > What do you mean? Moving the logic for selecting the object files to > link with on each top-level target out into the configure? Add CONFIG_QEMU_TIMER, configure sets it to 'y' when it is needed by simpletrace or other cases. > In any case, I think that adding qemu-timer-common.o into trace-obj-y is > the cleanest way, as otherwise the object needs to be added again and > again depending on conditions that are checked multiple times, which I > think will lead to to makefile maintenance headaches in the long run. That is not needed if the logic resides in configure: obj-$(CONFIG_QEMU_TIMER) += qemu-timer-common.o
On Sun, Aug 28, 2011 at 10:08 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > On Sun, Aug 28, 2011 at 6:13 PM, Lluís <xscript@gmx.net> wrote: >> Blue Swirl writes: >> >>> On Sat, Aug 27, 2011 at 5:56 PM, Lluís <xscript@gmx.net> wrote: >>>>>> I sent a patch that should fix it for everybody linking with the tracing >>>>>> objects: >>>>>> >>>>>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html >>>> >>>>> With your patch, there are warnings from linker: >>>>> ../qemu-timer-common.o: warning: multiple common of `use_rt_clock' >>>>> ../qemu-timer-common.o: warning: previous common is here >>>> >>>> Ah, yes. These extra errors are fixed by the duplicate elimination patch >>>> :) >>>> >>>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02987.html >>>> >>>> So, you need both to keep it clean. >> >>> Using the sort function looks hackish to me. Maybe the linkage should >>> be controlled by configure instead? >> >> What do you mean? Moving the logic for selecting the object files to >> link with on each top-level target out into the configure? > > Add CONFIG_QEMU_TIMER, configure sets it to 'y' when it is needed by > simpletrace or other cases. The $(sort) approach is simpler because it is implicit. I'm not sure that explicitly managing these dependencies is necessary. But the configure approach works for me too. Blue: Are you going to post the CONFIG_QEMU_TIMER patch? Stefan
Blue Swirl writes: >>> Using the sort function looks hackish to me. Maybe the linkage should >>> be controlled by configure instead? >> >> What do you mean? Moving the logic for selecting the object files to >> link with on each top-level target out into the configure? > Add CONFIG_QEMU_TIMER, configure sets it to 'y' when it is needed by > simpletrace or other cases. >> In any case, I think that adding qemu-timer-common.o into trace-obj-y is >> the cleanest way, as otherwise the object needs to be added again and >> again depending on conditions that are checked multiple times, which I >> think will lead to to makefile maintenance headaches in the long run. > That is not needed if the logic resides in configure: > obj-$(CONFIG_QEMU_TIMER) += qemu-timer-common.o Well, this looks more hackish than the 'sort' approach to me, but I can live with that :) Lluis
On 08/29/2011 07:28 AM, Stefan Hajnoczi wrote: > On Sun, Aug 28, 2011 at 10:08 PM, Blue Swirl<blauwirbel@gmail.com> wrote: >> On Sun, Aug 28, 2011 at 6:13 PM, Lluís<xscript@gmx.net> wrote: >>> Blue Swirl writes: >>> >>>> On Sat, Aug 27, 2011 at 5:56 PM, Lluís<xscript@gmx.net> wrote: >>>>>>> I sent a patch that should fix it for everybody linking with the tracing >>>>>>> objects: >>>>>>> >>>>>>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg03150.html >>>>> >>>>>> With your patch, there are warnings from linker: >>>>>> ../qemu-timer-common.o: warning: multiple common of `use_rt_clock' >>>>>> ../qemu-timer-common.o: warning: previous common is here >>>>> >>>>> Ah, yes. These extra errors are fixed by the duplicate elimination patch >>>>> :) >>>>> >>>>> http://lists.gnu.org/archive/html/qemu-devel/2011-08/msg02987.html >>>>> >>>>> So, you need both to keep it clean. >>> >>>> Using the sort function looks hackish to me. Maybe the linkage should >>>> be controlled by configure instead? >>> >>> What do you mean? Moving the logic for selecting the object files to >>> link with on each top-level target out into the configure? >> >> Add CONFIG_QEMU_TIMER, configure sets it to 'y' when it is needed by >> simpletrace or other cases. > > The $(sort) approach is simpler because it is implicit. I'm not sure > that explicitly managing these dependencies is necessary. But the It also mirrors how most projects handle duplicate header includes using a guard. Plus, it really sucks to not be able to create a self-sufficient group of objects just because someone that includes your group happens to also include a common object (in this case, common-obj-y). trace-obj-y should absolutely be able to note qemu-timer-common.o as an explicit dependency regardless of whether or not it happens to get linked in by something else in the common case. Plus with talk of breaking up QEMU into shared objects I think we'd end up needing to have self-sufficient object groups anyway. But, in the meantime, I broke something again so I put together a patch with Blue's suggestions that seems to do the trick fairly reasonably. I'll send it as a response for reference, though I'd really prefer the $(sort) approach. > configure approach works for me too. > > Blue: Are you going to post the CONFIG_QEMU_TIMER patch? > > Stefan >
diff --git a/Makefile.objs b/Makefile.objs index d1f3e5d..df11aec 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -404,6 +404,9 @@ qga-obj-y = $(addprefix qga/, $(qga-nested-y)) qga-obj-y += qemu-ga.o qemu-tool.o qemu-error.o qemu-sockets.o module.o qemu-option.o cutils.o osdep.o qga-obj-$(CONFIG_WIN32) += oslib-win32.o qga-obj-$(CONFIG_POSIX) += oslib-posix.o +ifeq ($(TRACE_BACKEND),simple) +qga-obj-y += qemu-timer-common.o +endif vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
957f1f99f263d57612807a9535f75ca4473f05f0 didn't consider that qemu-timer-common.o is needed by simpletrace. Fix by adding it to qga object list. Signed-off-by: Blue Swirl <blauwirbel@gmail.com> --- Makefile.objs | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)