[Qemu-devel] [RFC v2][PATCH][Tracing] Fix build errors for target i386-linux-user

[Prerna Saxena]

On Thu, 1 Jul 2010 10:18:41 +0100
Stefan Hajnoczi <address@hidden> wrote:

> On Wed, Jun 30, 2010 at 09:11:45PM +0530, Prerna Saxena wrote:
> > [PATCH 1/1] Move definitions of monitor command handlers (do_info_trace, 
> > do_info_all_trace_events) to monitor.c. This removes build errors for 
> > user targets such as i386-linux-user, which are not linked with monitor.
> > 
> > The export of trace_buf and trace_idx is an unfortunate side effect, 
> > since these are needed by do_info_trace which dumps buffer 
> > contents.
> 
> "git grep monitor_printf" suggests that other source files do their own
> monitor printing.  Did you look into alternatives that don't expose the
> trace buffer internals?

$grep "monitor_printf" would show this function being used in lot of
places, but all of those include "monitor.h" and are linked to the
monitor object as a part of the build process. I could not find any other 
definition of this function apart from the one in "monitor.c", and a 
dummy definition in qemu_tool.c.

> 
> I feel #ifndef CONFIG_USER_ONLY in simpletrace.c would be would be nicer
> than exposing the internals.  (CONFIG_USER_ONLY is just a guess, I
> haven't investigated which is the right thing to test for.)
> 
> Stefan
> 

Yes, I did explore some alternatives.

The monitor command handlers for tracing need knowledge of buffer internals.
The top-level trace object gets compiled earlier, and all target objects
include this later at link time. None of the user targets (linux-user,
bsd-user, darwin-user) are linked with monitor.o, and consequently the
trace object when linked with these targets still has dangling
references to monitor commands. This flags the linker errors.

Even if I were to enclose the monitor-specific interfaces with a
#ifdef...#endif , it would allow the trace-object to be built
homogenously for all targets -- either with or without the code blocks
enclosed in #ifdef..#endif. I am not being able to see how we can
utilize CONFIG_USER_ONLY here.

There are 3 ways I could think of, to fix this :

a) Do not link the trace object for user targets, that don't link with
monitor.o
Advantage : errors disappear.
Drawback : tracing not available for user targets.

b) Keep the trace buffer definitions intact. In places where the linker
doesn't find the monitor references, put a dummy file that has empty
definitions for these monitor interfaces, and link the target object
with these to remove the dangling references.
Advantage : trace buffer internals are safeguarded.
Drawback : Code & build process gets complicated ; not a clean approach.

c) Separate the trace buffer functions and the monitor interfaces. In
user targets, only the trace object is included which allows
trace-events to be built for these targets as well. Just that, the
monitor support is not available.
Advantage : Clean separation of monitor commands and basic trace
functionality.
Drawback : Trace buffer internals get exposed.

My earlier patch took approach 'c', the one below takes approach 'b'.
This patch flags a compile warning :

exec.o: warning: multiple common of `logfile'
qemu-tool.o: warning: previous common is here

I'm running short of ideas to fix this. Any suggestions would be very 
helpful.

[PATCH] User targets dont include monitor.o. Fix to allow compilation of 
trace object for these.

Signed-off-by: Prerna Saxena <address@hidden>
---
 Makefile.target |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 493233a..b05e37e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -86,7 +86,7 @@ $(call set-vpath, 
$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR
 QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user 
-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
 obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
       elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o \
-      qemu-malloc.o
+      qemu-malloc.o qemu-tool.o
 
 obj-$(TARGET_HAS_BFLT) += flatload.o
 
@@ -124,7 +124,7 @@ LDFLAGS+=-Wl,-segaddr,__STD_PROG_ZONE,0x1000 -image_base 
0x0e000000
 LIBS+=-lmx
 
 obj-y = main.o commpage.o machload.o mmap.o signal.o syscall.o thunk.o \
-        gdbstub.o
+        gdbstub.o qemu-tool.o
 
 obj-i386-y += ioport-user.o
 
@@ -146,7 +146,7 @@ $(call set-vpath, $(SRC_PATH)/bsd-user)
 QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ARCH)
 
 obj-y = main.o bsdload.o elfload.o mmap.o signal.o strace.o syscall.o \
-        gdbstub.o uaccess.o
+        gdbstub.o uaccess.o qemu-tool.o
 
 obj-i386-y += ioport-user.o
 
-- 


-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India