On Thu, 1 Jul 2010 10:18:41 +0100
Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> 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 <pre...@linux.vnet.ibm.com>
---
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
