Re: [Qemu-devel] [PATCH] dump-guest-memory.py: fix "You can't do that without a process to debug"

[Laszlo Ersek]

On 12/12/17 17:35, Marc-André Lureau wrote:
> If the script is run with a core (no running process), it produces an
> error:
> 
> (gdb)  dump-guest-memory /tmp/vmcore X86_64
> guest RAM blocks:
> target_start     target_end       host_addr        message count
> ---------------- ---------------- ---------------- ------- -----
> 0000000000000000 00000000000a0000 00007f7935800000 added       1
> 00000000000a0000 00000000000b0000 00007f7934200000 added       2
> 00000000000c0000 00000000000ca000 00007f79358c0000 added       3
> 00000000000ca000 00000000000cd000 00007f79358ca000 joined      3
> 00000000000cd000 00000000000e8000 00007f79358cd000 joined      3
> 00000000000e8000 00000000000f0000 00007f79358e8000 joined      3
> 00000000000f0000 0000000000100000 00007f79358f0000 joined      3
> 0000000000100000 0000000080000000 00007f7935900000 joined      3
> 00000000fd000000 00000000fe000000 00007f7934200000 added       4
> 00000000fffc0000 0000000100000000 00007f7935600000 added       5
> Python Exception <class 'gdb.error'> You can't do that without a process to 
> debug.:
> Error occurred in Python command: You can't do that without a process
> to debug.
> 
> Replace the function call with a variable.

Can you state, "replace the object_resolve_path_type() function call
with an extern variable"?

> I tried to use static
> function variables, as suggested by Laszlo in earlier reviews, but the
> compiler always took the chance to optimize them away.

Did you qualify them "volatile" too?

E.g., in the vmcoreinfo_realize() function:

  static VMCoreInfoState * volatile vmcoreinfo_state;
  ...
  vmcoreinfo_state = s;

(Note: it's the pointer itself that has to be volatile, not the
pointed-to object.)

If you did try this, but GCC optimized it away, then that's a GCC bug.
In that case I guess I can't object to the present approach any longer.
Can you please confirm?

> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  scripts/dump-guest-memory.py | 3 +--
>  hw/misc/vmcoreinfo.c         | 4 ++++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 1af26c1a45..9dec796762 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -546,8 +546,7 @@ shape and this command should mostly work."""
>          return None
>  
>      def add_vmcoreinfo(self):
> -        vmci = '(VMCoreInfoState *)' + \
> -               'object_resolve_path_type("", "vmcoreinfo", 0)'
> +        vmci = 'vmcoreinfo_state'
>          if not gdb.parse_and_eval("%s" % vmci) \
>             or not gdb.parse_and_eval("(%s)->has_vmcoreinfo" % vmci):
>              return
> diff --git a/hw/misc/vmcoreinfo.c b/hw/misc/vmcoreinfo.c
> index 31db57ab44..d429a4f7d6 100644
> --- a/hw/misc/vmcoreinfo.c
> +++ b/hw/misc/vmcoreinfo.c
> @@ -31,6 +31,9 @@ static void vmcoreinfo_reset(void *dev)
>      s->vmcoreinfo.host_format = cpu_to_le16(VMCOREINFO_FORMAT_ELF);
>  }
>  
> +/* this variable is exported for gdb script dump-guest-memory.py */
> +VMCoreInfoState *vmcoreinfo_state;
> +
>  static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
>  {
>      VMCoreInfoState *s = VMCOREINFO(dev);
> @@ -56,6 +59,7 @@ static void vmcoreinfo_realize(DeviceState *dev, Error 
> **errp)
>                               &s->vmcoreinfo, sizeof(s->vmcoreinfo), false);
>  
>      qemu_register_reset(vmcoreinfo_reset, dev);
> +    vmcoreinfo_state = s;
>  }
>  
>  static const VMStateDescription vmstate_vmcoreinfo = {
> 

Thanks!
Laszlo