Re: [Qemu-devel] Thoughts around dtrace linking...
On 26/03/2012 11:14, Stefan Hajnoczi wrote: On Fri, Mar 23, 2012 at 2:11 PM, Lee Essen wrote: On 23 Mar 2012, at 08:08, Stefan Hajnoczi wrote: On Thu, Mar 22, 2012 at 05:00:53PM +, Lee Essen wrote: On 22/03/2012 16:28, Stefan Hajnoczi wrote: On Wed, Mar 21, 2012 at 1:01 PM, Andreas Färberwrote: Hi, Am 21.03.2012 11:45, schrieb Lee Essen: I've been trying to find a sensible way to solve the Solaris/Illumos dtrace requirement to pass all the objs to the dtrace command so that the resultant object file contains all the symbols needed to properly link the relevant binary. If you're able to try out the dependency-based approach that would be a good starting point. You may hit a point where it turns out to be too ugly or complicated - in that case, please post your progress and maybe someone can help find a way to structure it. I'm not a make guru but I can try to give feedback on your patches. Stefan Ok, here's an attempt … it works for me, but I'm not sure how it would work on a different dtrace platform so it will probably need some different guarding … but it shows what it will look like. The code makes sense to me. There's a feeling that there must be a way to remove the duplication in this approach, but my make foo isn't good enough to see how. As you mentioned, we need a way to distinguish between Solaris DTrace, which requires the global .o linking approach, and other implementations. You could use CONFIG_SOLARIS and CONFIG_TRACE_DTRACE together to enable the global .o linking. So there is another way that's less duplicative on the non-target stuff. If you build all of the objects needed for ALL of the binaries, you could then build the dtrace object so that it was a superset, then link with that one in all of the final binary linking. To be honest I think it's probably more confusing to do that, the previous approach is more duplicative but easier to follow. Let me pull a full patch together and see what people think. Lee.
Re: [Qemu-devel] Thoughts around dtrace linking...
On Fri, Mar 23, 2012 at 2:11 PM, Lee Essen wrote: > > On 23 Mar 2012, at 08:08, Stefan Hajnoczi wrote: > >> On Thu, Mar 22, 2012 at 05:00:53PM +, Lee Essen wrote: >>> On 22/03/2012 16:28, Stefan Hajnoczi wrote: On Wed, Mar 21, 2012 at 1:01 PM, Andreas Färber wrote: > Hi, > > Am 21.03.2012 11:45, schrieb Lee Essen: >> I've been trying to find a sensible way to solve the Solaris/Illumos >> dtrace requirement to pass all the objs to the dtrace command so that >> the resultant object file contains all the symbols needed to properly >> link the relevant binary. >> >> >> If you're able to try out the dependency-based approach that would be a >> good starting point. You may hit a point where it turns out to be too >> ugly or complicated - in that case, please post your progress and maybe >> someone can help find a way to structure it. I'm not a make guru but I >> can try to give feedback on your patches. >> >> Stefan >> > > Ok, here's an attempt … it works for me, but I'm not sure how it would work > on a different dtrace platform so it will probably need some different > guarding … but it shows what it will look like. The code makes sense to me. There's a feeling that there must be a way to remove the duplication in this approach, but my make foo isn't good enough to see how. As you mentioned, we need a way to distinguish between Solaris DTrace, which requires the global .o linking approach, and other implementations. You could use CONFIG_SOLARIS and CONFIG_TRACE_DTRACE together to enable the global .o linking. Stefan
Re: [Qemu-devel] Thoughts around dtrace linking...
On 23 Mar 2012, at 08:08, Stefan Hajnoczi wrote: > On Thu, Mar 22, 2012 at 05:00:53PM +, Lee Essen wrote: >> On 22/03/2012 16:28, Stefan Hajnoczi wrote: >>> On Wed, Mar 21, 2012 at 1:01 PM, Andreas Färber wrote: Hi, Am 21.03.2012 11:45, schrieb Lee Essen: > I've been trying to find a sensible way to solve the Solaris/Illumos > dtrace requirement to pass all the objs to the dtrace command so that > the resultant object file contains all the symbols needed to properly > link the relevant binary. > > > If you're able to try out the dependency-based approach that would be a > good starting point. You may hit a point where it turns out to be too > ugly or complicated - in that case, please post your progress and maybe > someone can help find a way to structure it. I'm not a make guru but I > can try to give feedback on your patches. > > Stefan > Ok, here's an attempt … it works for me, but I'm not sure how it would work on a different dtrace platform so it will probably need some different guarding … but it shows what it will look like. Lee. diff --git a/Makefile b/Makefile index 1bc3cb0..b300364 100644 --- a/Makefile +++ b/Makefile @@ -157,9 +157,28 @@ tools-obj-y = $(oslib-obj-y) $(trace-obj-y) qemu-tool.o qemu-timer.o \ qemu-timer-common.o main-loop.o notify.o iohandler.o cutils.o async.o tools-obj-$(CONFIG_POSIX) += compatfd.o -qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y) -qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y) -qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) +qemu-img-trace-objs=qemu-img.o $(tools-obj-y) $(block-obj-y) +qemu-nbd-trace-objs=qemu-nbd.o $(tools-obj-y) $(block-obj-y) +qemu-io-trace-objs=qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y) +qemu-img-all-objs=$(qemu-img-trace-objs) +qemu-nbd-all-objs=$(qemu-nbd-trace-objs) +qemu-io-all-objs=$(qemu-io-trace-objs) +ifdef CONFIG_TRACE_DTRACE +qemu-img-all-objs+=qemu-img.dtrace.o +qemu-nbd-all-objs+=qemu-nbd.dtrace.o +qemu-io-trace-objs+=qemu-img.dtrace.o +endif + +qemu-img.dtrace.o: trace-dtrace.dtrace $(qemu-img-trace-objs) + $(call DTRACE,$@,trace-dtrace.dtrace,$(qemu-img-trace-objs)) +qemu-nbd.dtrace.o: trace-dtrace.dtrace $(qemu-nbd-trace-objs) + $(call DTRACE,$@,trace-dtrace.dtrace,$(qemu-nbd-trace-objs)) +qemu-io.dtrace.o: trace-dtrace.dtrace $(qemu-io-trace-objs) + $(call DTRACE,$@,trace-dtrace.dtrace,$(qemu-io-trace-objs)) + +qemu-img$(EXESUF): $(qemu-img-all-objs) +qemu-nbd$(EXESUF): $(qemu-nbd-all-objs) +qemu-io$(EXESUF): $(qemu-io-all-objs) qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o qemu-bridge-helper.o: $(GENERATED_HEADERS) @@ -205,7 +224,17 @@ QGALIB_GEN=$(addprefix $(qapi-dir)/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-c $(QGALIB_OBJ): $(QGALIB_GEN) $(GENERATED_HEADERS) $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) $(GENERATED_HEADERS) -qemu-ga$(EXESUF): qemu-ga.o $(qga-obj-y) $(tools-obj-y) $(qapi-obj-y) $(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ) +qemu-ga-trace-objs=qemu-ga.o $(qga-obj-y) $(tools-obj-y) $(qapi-obj-y) $(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ) +qemu-ga-all-objs=$(qemu-ga-trace-objs) +ifdef CONFIG_TRACE_DTRACE +qemu-ga-all-objs+=qemu-ga.dtrace.o +endif + +qemu-ga.dtrace.o: trace-dtrace.dtrace $(qemu-ga-trace-objs) + $(call DTRACE,$@,trace-dtrace.dtrace,$(qemu-ga-trace-objs)) + +qemu-ga$(EXESUF): $(qemu-ga-all-objs) + QEMULIBS=libhw32 libhw64 libuser libdis libdis-user diff --git a/Makefile.target b/Makefile.target index 63cf769..ed0d824 100644 --- a/Makefile.target +++ b/Makefile.target @@ -441,7 +441,17 @@ $(QEMU_PROGW): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) $(QEMU_PROG): $(QEMU_PROGW) $(call quiet-command,$(OBJCOPY) --subsystem console $(QEMU_PROGW) $(QEMU_PROG)," GEN $(TARGET_DIR)$(QEMU_PROG)") else -$(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) + +target-trace-objs=$(obj-y) $(obj-$(TARGET_BASE_ARCH)-y) +target-all-objs=$(target-trace-objs) +ifdef CONFIG_TRACE_DTRACE +target-all-objs+=$(QEMU_PROG).dtrace.o +endif + +$(QEMU_PROG).dtrace.o: ../trace-dtrace.dtrace $(target-trace-objs) + $(call DTRACE,$@,../trace-dtrace.dtrace,$(target-trace-objs)) + +$(QEMU_PROG): $(target-all-objs) $(call LINK,$^) endif diff --git a/rules.mak b/rules.mak index 04a9198..31ad59c 100644 --- a/rules.mak +++ b/rules.mak @@ -33,6 +33,8 @@ endif LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $(sort $(1)) $(LIBS)," LINK $(TARGET_DIR)$@") +DTRACE = $(call quiet-command,dtrace $(CONFIG_DTRACE_FLAGS) -o $(1) -G -s $(2) $(3), " GEN $(TARGET_DIR)$(1)") + %$(EXESUF): %.o $(call LINK,$^)
Re: [Qemu-devel] Thoughts around dtrace linking...
On Thu, Mar 22, 2012 at 05:00:53PM +, Lee Essen wrote: > On 22/03/2012 16:28, Stefan Hajnoczi wrote: > >On Wed, Mar 21, 2012 at 1:01 PM, Andreas Färber wrote: > >>Hi, > >> > >>Am 21.03.2012 11:45, schrieb Lee Essen: > >>>I've been trying to find a sensible way to solve the Solaris/Illumos > >>>dtrace requirement to pass all the objs to the dtrace command so that > >>>the resultant object file contains all the symbols needed to properly > >>>link the relevant binary. > >>> > >>>The easiest way to do this is just prior to linking the binary, so > >>>something like this (in rules.mak): > >>> > >>> LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) > >>> $(LDFLAGS) -o $@ $(sort $(1)) $(LIBS)," LINK $(TARGET_DIR)$@") > >>> > >>> DTRACE = $(call quiet-command,dtrace $(CONFIG_DTRACE_FLAGS) -o > >>> $(1)-dtrace.o -G -s $(2) $(3), " GEN $(TARGET_DIR)$(1)-dtrace.o") > >>> > >>> %$(EXESUF): %.o > >>> $(call DTRACE,$*,trace-dtrace.dtrace,$^) > >>> $(call LINK,$^ $*-dtrace.o) > > > >What I find slightly surprising is that you're putting the -dtrace.o > >generation step as a command in the executable's target. > > > >I would expect the -dtrace.o to be a target itself, which also allows > >make to use it's timestamping on dependencies to ensure we only > >rebuild when necessary. i.e. specifying dependencies is the make way > >of doing things, and I think we should try where possible. > > Yes, that's the way I had it the first time around, but it means > quite a bit more complexity in the makefiles and having to touch > each executable section, I had thought the rules.mak approach was > cleaner. > > For example: > > qemu-ga-all-objs=qemu-ga.o $(qga-obj-y) $(tools-obj-y) $(qapi-obj-y) > $(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ) > #ifdef USE_SOLARIS_DTRACE_APPROACH > qemu-ga.dtrace.o: $(qemu-ga-all-objs) [assuming rule in rules.mak] > > qemu-ga-all-objs+=qemu-ga.dtrace.o > #endif > qemu-ga$(EXESUF): qemu-ga-all-objs > > There's also a complication with the creation of the .dtrace.o in > Makefile.target because of it being one level down in the directory > structure and needing access to trace-dtrace.dtrace. > > None of it is unsurmountable, but it gets a bit untidy. > > TBH, I can do this either way, just let me know which approach you > prefer and I'll put a patch together. > > >>> > >>>Obviously with the relevant tests around it to check the trace backend, > >>>and also an adjustment in Makefile.target to cause the right thing to > >>>happen for each target. > >>>Or, is there a better way? > >> > >>The two issues I see (as info for Stefan et al.) are > >>a) compiling DTrace probes into .o files requires linking those objects > >>with that additional .o file to avoid linker errors (even for tools > >>where using DTrace probes does not seem to make much sense), > > > >qemu-tool binaries are built with tracing enabled. But this is a good > >point, we need to check that all binaries buildable from the QEMU > >source tree continue to work with this change. > > > > Actually this is a good point ... if you are thinking of removing > tracing from some of the binaries then the rules.mak approach > doesn't really make sense. > > Let me know how you want to proceed. If you're able to try out the dependency-based approach that would be a good starting point. You may hit a point where it turns out to be too ugly or complicated - in that case, please post your progress and maybe someone can help find a way to structure it. I'm not a make guru but I can try to give feedback on your patches. Stefan
Re: [Qemu-devel] Thoughts around dtrace linking...
On 22/03/2012 16:28, Stefan Hajnoczi wrote: On Wed, Mar 21, 2012 at 1:01 PM, Andreas Färber wrote: Hi, Am 21.03.2012 11:45, schrieb Lee Essen: I've been trying to find a sensible way to solve the Solaris/Illumos dtrace requirement to pass all the objs to the dtrace command so that the resultant object file contains all the symbols needed to properly link the relevant binary. The easiest way to do this is just prior to linking the binary, so something like this (in rules.mak): LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $(sort $(1)) $(LIBS)," LINK $(TARGET_DIR)$@") DTRACE = $(call quiet-command,dtrace $(CONFIG_DTRACE_FLAGS) -o $(1)-dtrace.o -G -s $(2) $(3), " GEN $(TARGET_DIR)$(1)-dtrace.o") %$(EXESUF): %.o $(call DTRACE,$*,trace-dtrace.dtrace,$^) $(call LINK,$^ $*-dtrace.o) What I find slightly surprising is that you're putting the -dtrace.o generation step as a command in the executable's target. I would expect the -dtrace.o to be a target itself, which also allows make to use it's timestamping on dependencies to ensure we only rebuild when necessary. i.e. specifying dependencies is the make way of doing things, and I think we should try where possible. Yes, that's the way I had it the first time around, but it means quite a bit more complexity in the makefiles and having to touch each executable section, I had thought the rules.mak approach was cleaner. For example: qemu-ga-all-objs=qemu-ga.o $(qga-obj-y) $(tools-obj-y) $(qapi-obj-y) $(qobject-obj-y) $(version-obj-y) $(QGALIB_OBJ) #ifdef USE_SOLARIS_DTRACE_APPROACH qemu-ga.dtrace.o: $(qemu-ga-all-objs) [assuming rule in rules.mak] qemu-ga-all-objs+=qemu-ga.dtrace.o #endif qemu-ga$(EXESUF): qemu-ga-all-objs There's also a complication with the creation of the .dtrace.o in Makefile.target because of it being one level down in the directory structure and needing access to trace-dtrace.dtrace. None of it is unsurmountable, but it gets a bit untidy. TBH, I can do this either way, just let me know which approach you prefer and I'll put a patch together. Obviously with the relevant tests around it to check the trace backend, and also an adjustment in Makefile.target to cause the right thing to happen for each target. Or, is there a better way? The two issues I see (as info for Stefan et al.) are a) compiling DTrace probes into .o files requires linking those objects with that additional .o file to avoid linker errors (even for tools where using DTrace probes does not seem to make much sense), qemu-tool binaries are built with tracing enabled. But this is a good point, we need to check that all binaries buildable from the QEMU source tree continue to work with this change. Actually this is a good point ... if you are thinking of removing tracing from some of the binaries then the rules.mak approach doesn't really make sense. Let me know how you want to proceed. Cheers, Lee.
Re: [Qemu-devel] Thoughts around dtrace linking...
On Wed, Mar 21, 2012 at 1:01 PM, Andreas Färber wrote: > Hi, > > Am 21.03.2012 11:45, schrieb Lee Essen: >> I've been trying to find a sensible way to solve the Solaris/Illumos >> dtrace requirement to pass all the objs to the dtrace command so that >> the resultant object file contains all the symbols needed to properly >> link the relevant binary. >> >> The easiest way to do this is just prior to linking the binary, so >> something like this (in rules.mak): >> >> LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) >> $(LDFLAGS) -o $@ $(sort $(1)) $(LIBS)," LINK $(TARGET_DIR)$@") >> >> DTRACE = $(call quiet-command,dtrace $(CONFIG_DTRACE_FLAGS) -o >> $(1)-dtrace.o -G -s $(2) $(3), " GEN $(TARGET_DIR)$(1)-dtrace.o") >> >> %$(EXESUF): %.o >> $(call DTRACE,$*,trace-dtrace.dtrace,$^) >> $(call LINK,$^ $*-dtrace.o) What I find slightly surprising is that you're putting the -dtrace.o generation step as a command in the executable's target. I would expect the -dtrace.o to be a target itself, which also allows make to use it's timestamping on dependencies to ensure we only rebuild when necessary. i.e. specifying dependencies is the make way of doing things, and I think we should try where possible. >> >> Obviously with the relevant tests around it to check the trace backend, >> and also an adjustment in Makefile.target to cause the right thing to >> happen for each target. >> Or, is there a better way? > > The two issues I see (as info for Stefan et al.) are > a) compiling DTrace probes into .o files requires linking those objects > with that additional .o file to avoid linker errors (even for tools > where using DTrace probes does not seem to make much sense), qemu-tool binaries are built with tracing enabled. But this is a good point, we need to check that all binaries buildable from the QEMU source tree continue to work with this change. Stefan
Re: [Qemu-devel] Thoughts around dtrace linking...
On 21/03/2012 13:01, Andreas Färber wrote: Hi, Am 21.03.2012 11:45, schrieb Lee Essen: I've been trying to find a sensible way to solve the Solaris/Illumos dtrace requirement to pass all the objs to the dtrace command so that the resultant object file contains all the symbols needed to properly link the relevant binary. The two issues I see (as info for Stefan et al.) are a) compiling DTrace probes into .o files requires linking those objects with that additional .o file to avoid linker errors (even for tools where using DTrace probes does not seem to make much sense), b) the additional .o file depends on its input .o files, so must be unique per executable. This seems to be guaranteed when doing it at rules.mak level. Haven't checked all the variables being passed back and forth here yet though; I really am not comfortable reviewing code that's HTML-formatted in my mailer. ;) Sorry ... I've been jumping between machines and I'd thought I'd switched them all to plaintext, but obviously not. Hopefully this is done now. New version below ... How compatible is this with FreeBSD - it doesn't sound like it's needed at all? Don't know about FreeBSD but on Darwin that step is indeed not needed or supported, so needs to be guarded. Andreas Ok, so perhaps the easiest way to guard would be a new variable that matches both a backend of dtrace and OS of Solaris established via configure, for the moment I've just put a backend check, but clearly this wouldn't be enough. LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ $(sort $(1)) $(LIBS)," LINK $(TARGET_DIR)$@") DTRACE = $(call quiet-command,dtrace $(CONFIG_DTRACE_FLAGS) -o $(1)-dtrace.o -G -s $(2) $(3), " GEN $(TARGET_DIR)$(1)-dtrace.o") %$(EXESUF): %.o ifeq ($(TRACE_BACKEND),dtrace) $(call DTRACE,$*,trace-dtrace.dtrace,$^) $(call LINK,$^ $*-dtrace.o) else $(call LINK,$^) endif If you're happy that this is a sensible approach I'll put together a proper patch. Regards, Lee.
Re: [Qemu-devel] Thoughts around dtrace linking...
Hi, Am 21.03.2012 11:45, schrieb Lee Essen: > I've been trying to find a sensible way to solve the Solaris/Illumos > dtrace requirement to pass all the objs to the dtrace command so that > the resultant object file contains all the symbols needed to properly > link the relevant binary. > > The easiest way to do this is just prior to linking the binary, so > something like this (in rules.mak): > > LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) > $(LDFLAGS) -o $@ $(sort $(1)) $(LIBS)," LINK $(TARGET_DIR)$@") > > DTRACE = $(call quiet-command,dtrace $(CONFIG_DTRACE_FLAGS) -o > $(1)-dtrace.o -G -s $(2) $(3), " GEN $(TARGET_DIR)$(1)-dtrace.o") > > %$(EXESUF): %.o > $(call DTRACE,$*,trace-dtrace.dtrace,$^) > $(call LINK,$^ $*-dtrace.o) > > Obviously with the relevant tests around it to check the trace backend, > and also an adjustment in Makefile.target to cause the right thing to > happen for each target. > Or, is there a better way? The two issues I see (as info for Stefan et al.) are a) compiling DTrace probes into .o files requires linking those objects with that additional .o file to avoid linker errors (even for tools where using DTrace probes does not seem to make much sense), b) the additional .o file depends on its input .o files, so must be unique per executable. This seems to be guaranteed when doing it at rules.mak level. Haven't checked all the variables being passed back and forth here yet though; I really am not comfortable reviewing code that's HTML-formatted in my mailer. ;) > How compatible is this with FreeBSD - it doesn't sound like it's needed > at all? Don't know about FreeBSD but on Darwin that step is indeed not needed or supported, so needs to be guarded. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
