Re: [Qemu-devel] [PATCH 2/2] trace: show MemoryRegion name, not address

[Stefan Hajnoczi <stefanha@gmail.com>]

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

On Tue, Nov 17, 2015 at 04:37:48PM -0800, Hollis Blanchard wrote:
> On 11/13/2015 02:23 AM, Stefan Hajnoczi wrote:
> >On Wed, Nov 11, 2015 at 05:09:58PM -0800, Hollis Blanchard wrote:
> >>Recording the MemoryRegion pointers isn't helpful, especially since no trace
> >>data allows us to correlate those pointers to devices. Instead, record the
> >>MemoryRegion name.
> >>
> >>Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com>
> >>---
> >>  memory.c     | 12 ++++++------
> >>  trace-events |  4 ++--
> >>  2 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/memory.c b/memory.c
> >>index c435c88..9bd4c31 100644
> >>--- a/memory.c
> >>+++ b/memory.c
> >>@@ -381,7 +381,7 @@ static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr,
> >>      uint64_t tmp;
> >>      tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
> >>-    trace_memory_region_ops_read(mr, addr, tmp, size);
> >>+    trace_memory_region_ops_read(mr->name, addr, tmp, size);
> >mr->name may be NULL.  There is a memory_region_name() function that
> >always produces a real string.  Perhaps it's best to use it.
> 
> Using memory_region_name() yields this:
> ** ERROR **: file qom/object.c: line 1427
> (object_get_canonical_path_component): assertion failed: (obj->parent !=
> NULL)
> aborting...
> 
> The offending MemoryRegion seems to be a subpage one, which has no name. I
> can tell because ops contains links to subpage_read() and subpage_write().
> 
> "info mtree" uses memory_region_name() and works fine, but perhaps that's
> because it only goes 2 levels deep?

I'm not very familiar with the memory API so I'm afraid I don't know the
best solution.  My concern about a NULL string pointer is that some
operating systems ship a libc that segfaults instead of snprintf(...,
"%s", NULL) to "(null)".  So the stderr trace backend could crash on
those operating systems.

Stefan

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