[Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Xu, Anthony]

Create libqemutrace.a for all trace.o
Currently all trace.o are linked into qemu-system, qemu-img, 
qemu-nbd, qemu-io etc., even the corresponding components 
are not included.
Create a libqemutrace.a that the linker would only pull in .o 
files containing symbols that are actually referenced by the 
program.
./trace.o, ./qapi/trace.o and ./util/trace.o are added into 
libqemuutil.a  to avoid recursive dependencies between 
libqemuutil.a and libqemutrace.a.

Signed-off -by: Anthony Xu <anthony.xu@intel.com>


diff --git a/Makefile b/Makefile
index 6c359b2..565f5c7 100644
--- a/Makefile
+++ b/Makefile
@@ -345,8 +345,8 @@ subdir-dtc:dtc/libfdt dtc/tests
 dtc/%:
        mkdir -p $@

-$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(chardev-obj-y) \
-       $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY)) $(trace-obj-y)
+$(SUBDIR_RULES): libqemutrace.a libqemuutil.a libqemustub.a $(common-obj-y) $(chardev-obj-y) \
+       $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))

 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 # Only keep -O and -g cflags
@@ -367,10 +367,11 @@ Makefile: $(version-obj-y)

 libqemustub.a: $(stub-obj-y)
 libqemuutil.a: $(util-obj-y)
+libqemutrace.a: $(trace-obj-y)

 ######################################################################

-COMMON_LDADDS = $(trace-obj-y) libqemuutil.a libqemustub.a
+COMMON_LDADDS = libqemutrace.a libqemuutil.a libqemustub.a

 qemu-img.o: qemu-img-cmds.h

diff --git a/Makefile.objs b/Makefile.objs
index 6167e7b..4289ef9 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -3,6 +3,7 @@
 stub-obj-y = stubs/ crypto/
 util-obj-y = util/ qobject/ qapi/
 util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
+util-obj-y += $(trace-root-obj-y)

 chardev-obj-y = chardev/

@@ -167,8 +168,9 @@ trace-events-subdirs += qapi

 trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events)

-trace-obj-y = trace-root.o
+trace-root-obj-y = trace-root.o
+trace-root-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
+trace-obj-y = $(trace-root-obj-y)
 trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
 trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
-trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
 trace-obj-$(CONFIG_TRACE_DTRACE) += $(trace-events-subdirs:%=%/trace-dtrace.o)
diff --git a/Makefile.target b/Makefile.target
index 7df2b8c..6f508c8 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -201,7 +201,7 @@ all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)

 $(QEMU_PROG_BUILD): config-devices.mak

-COMMON_LDADDS = $(trace-obj-y) ../libqemuutil.a ../libqemustub.a
+COMMON_LDADDS = ../libqemutrace.a ../libqemuutil.a ../libqemustub.a

 # build either PROG or PROGW
 $(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS)
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index 33906ff..d543d56 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -4,3 +4,5 @@ util-obj-y += string-input-visitor.o string-output-visitor.o
 util-obj-y += opts-visitor.o qapi-clone-visitor.o
 util-obj-y += qmp-event.o
 util-obj-y += qapi-util.o
+util-obj-y +=  trace.o
+util-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace.o
diff --git a/tests/Makefile.include b/tests/Makefile.include
index f3de81f..6cbd602 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -519,7 +519,7 @@ QEMU_CFLAGS += -I$(SRC_PATH)/tests


 # Deps that are common to various different sets of tests below
-test-util-obj-y = $(trace-obj-y) libqemuutil.a libqemustub.a
+test-util-obj-y = libqemutrace.a libqemuutil.a libqemustub.a
 test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y)
 test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
        tests/test-qapi-event.o tests/test-qmp-introspect.o \
diff --git a/util/Makefile.objs b/util/Makefile.objs
index c6205eb..e38b91e 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -43,3 +43,5 @@ util-obj-y += qdist.o
 util-obj-y += qht.o
 util-obj-y += range.o
 util-obj-y += systemd.o
+util-obj-y +=  trace.o
+util-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace.o
                                                           

Re: [Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

On Fri, Mar 24, 2017 at 04:28:32PM +0000, Xu, Anthony wrote:
> Create libqemutrace.a for all trace.o
> Currently all trace.o are linked into qemu-system, qemu-img, 
> qemu-nbd, qemu-io etc., even the corresponding components 
> are not included.
> Create a libqemutrace.a that the linker would only pull in .o 
> files containing symbols that are actually referenced by the 
> program.
> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into 
> libqemuutil.a  to avoid recursive dependencies between 
> libqemuutil.a and libqemutrace.a.

Why would libqemutrace.a depend on libqemuutil.a?

Tracing code shouldn't call other QEMU code.  That would could create
infinite recursion when a trace event is fired.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Xu, Anthony]

> > ./trace.o, ./qapi/trace.o and ./util/trace.o are added into
> > libqemuutil.a  to avoid recursive dependencies between
> > libqemuutil.a and libqemutrace.a.
> 
> Why would libqemutrace.a depend on libqemuutil.a?

Each trace.c calls trace_event_register_group to register events, 
trace_event_register_group is defined in trace/control.c , which 
is linked into libqemuutil.a.


> Tracing code shouldn't call other QEMU code.  That would could create
> infinite recursion when a trace event is fired.

If all trace.o needed by libqemuutil.a are linked into libqemuutil.a, 
libqemuutil.a will not depend on libqemutrace.a. This is what this patch 
take to break the infinite recursion.

Or we can link trace/*.o to libqemutrace.a, hope it breaks the infinite 
recursion. But trace/*.o may still depend on libqemuutil.a

Or we can just link all trace.o to libqemuutil.a.


Anthony

Re: [Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Paolo Bonzini]

On 27/03/2017 20:21, Xu, Anthony wrote:
>>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into
>>> libqemuutil.a  to avoid recursive dependencies between
>>> libqemuutil.a and libqemutrace.a.
>> Why would libqemutrace.a depend on libqemuutil.a?
> Each trace.c calls trace_event_register_group to register events, 
> trace_event_register_group is defined in trace/control.c , which 
> is linked into libqemuutil.a.

Ah:

util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
util-obj-y += control.o
util-obj-y += qmp.o

With the introduction of libqemutrace.a, I believe these should be moved
into libqemutrace.a.

Paolo

> 
>> Tracing code shouldn't call other QEMU code.  That would could create
>> infinite recursion when a trace event is fired.
> If all trace.o needed by libqemuutil.a are linked into libqemuutil.a, 
> libqemuutil.a will not depend on libqemutrace.a. This is what this patch 
> take to break the infinite recursion.
> 
> Or we can link trace/*.o to libqemutrace.a, hope it breaks the infinite 
> recursion. But trace/*.o may still depend on libqemuutil.a
> 
> Or we can just link all trace.o to libqemuutil.a.

Re: [Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Xu, Anthony]

> >>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into
> >>> libqemuutil.a  to avoid recursive dependencies between
> >>> libqemuutil.a and libqemutrace.a.
> >> Why would libqemutrace.a depend on libqemuutil.a?
> > Each trace.c calls trace_event_register_group to register events,
> > trace_event_register_group is defined in trace/control.c , which
> > is linked into libqemuutil.a.
> 
> Ah:
> 
> util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
> util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
> util-obj-y += control.o
> util-obj-y += qmp.o
> 
> With the introduction of libqemutrace.a, I believe these should be moved
> into libqemutrace.a.

Agreed,
But it doesn't solve infinite recursion issue. register_module_init is 
needed by libqemutrace.a, which is defined util/module.c.

it is hard to remove libqemutrace.a dependency on libqemuutil.a.

Removing libqemuutil.a dependency on libqemutrace.a is feasible.
Just like what I did in this patch, include all util related trace.o 
to libqemuutila.

The other simple way is to include all trace.o into libqemuutil.a

What's your opinion?

Thanks
Anthony

Re: [Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Daniel P. Berrange]

On Tue, Mar 28, 2017 at 07:00:34PM +0000, Xu, Anthony wrote:
> > >>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into
> > >>> libqemuutil.a  to avoid recursive dependencies between
> > >>> libqemuutil.a and libqemutrace.a.
> > >> Why would libqemutrace.a depend on libqemuutil.a?
> > > Each trace.c calls trace_event_register_group to register events,
> > > trace_event_register_group is defined in trace/control.c , which
> > > is linked into libqemuutil.a.
> > 
> > Ah:
> > 
> > util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
> > util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
> > util-obj-y += control.o
> > util-obj-y += qmp.o
> > 
> > With the introduction of libqemutrace.a, I believe these should be moved
> > into libqemutrace.a.
> 
> Agreed,
> But it doesn't solve infinite recursion issue. register_module_init is 
> needed by libqemutrace.a, which is defined util/module.c.
> 
> it is hard to remove libqemutrace.a dependency on libqemuutil.a.
> 
> Removing libqemuutil.a dependency on libqemutrace.a is feasible.
> Just like what I did in this patch, include all util related trace.o 
> to libqemuutila.
> 
> The other simple way is to include all trace.o into libqemuutil.a

Yeah, I'd suggest doing that - i don't see any benefit in having
them in a separate libqemutrace.a


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PATCH] Create libqemutrace.a for all trace.o

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

On Tue, Mar 28, 2017 at 07:00:34PM +0000, Xu, Anthony wrote:
> > >>> ./trace.o, ./qapi/trace.o and ./util/trace.o are added into
> > >>> libqemuutil.a  to avoid recursive dependencies between
> > >>> libqemuutil.a and libqemutrace.a.
> > >> Why would libqemutrace.a depend on libqemuutil.a?
> > > Each trace.c calls trace_event_register_group to register events,
> > > trace_event_register_group is defined in trace/control.c , which
> > > is linked into libqemuutil.a.
> > 
> > Ah:
> > 
> > util-obj-$(CONFIG_TRACE_SIMPLE) += simple.o
> > util-obj-$(CONFIG_TRACE_FTRACE) += ftrace.o
> > util-obj-y += control.o
> > util-obj-y += qmp.o
> > 
> > With the introduction of libqemutrace.a, I believe these should be moved
> > into libqemutrace.a.
> 
> Agreed,
> But it doesn't solve infinite recursion issue. register_module_init is 
> needed by libqemutrace.a, which is defined util/module.c.
> 
> it is hard to remove libqemutrace.a dependency on libqemuutil.a.
> 
> Removing libqemuutil.a dependency on libqemutrace.a is feasible.
> Just like what I did in this patch, include all util related trace.o 
> to libqemuutila.
> 
> The other simple way is to include all trace.o into libqemuutil.a
> 
> What's your opinion?

There's no harm in adding all trace objects into libqemuutil.a.  I'd go
that route.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]