1. Patchew
  2. QEMU
  3. View series

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

Marc-André Lureau posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171212172208.13588-1-marcandre.lureau@redhat.com
Test checkpatch failed
Test docker passed
Test ppc passed
Test s390x passed
scripts/dump-guest-memory.py | 3 +--
hw/misc/vmcoreinfo.c         | 3 +++
2 files changed, 4 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
Posted by Marc-André Lureau 6 years, 4 months ago 
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 object_resolve_path_type() function call call with a
local volatile variable.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---

v2:
 - use a vmcoreinfo_realize() local volatile variable
 - tweak commit message

scripts/dump-guest-memory.py | 3 +--
 hw/misc/vmcoreinfo.c         | 3 +++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 1af26c1a45..09bec92b50 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_realize::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..a2805527cb 100644
--- a/hw/misc/vmcoreinfo.c
+++ b/hw/misc/vmcoreinfo.c
@@ -35,6 +35,8 @@ static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
 {
     VMCoreInfoState *s = VMCOREINFO(dev);
     FWCfgState *fw_cfg = fw_cfg_find();
+    /* for gdb script dump-guest-memory.py */
+    static VMCoreInfoState * volatile vmcoreinfo_state G_GNUC_UNUSED;
 
     /* Given that this function is executing, there is at least one VMCOREINFO
      * device. Check if there are several.
@@ -56,6 +58,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 = {
-- 
2.15.1.355.g36791d7216
Re: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
Posted by no-reply@patchew.org 6 years, 4 months ago 
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20171212172208.13588-1-marcandre.lureau@redhat.com
Subject: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20171207203036.14993-1-eblake@redhat.com -> patchew/20171207203036.14993-1-eblake@redhat.com
 * [new tag]               patchew/20171212172208.13588-1-marcandre.lureau@redhat.com -> patchew/20171212172208.13588-1-marcandre.lureau@redhat.com
Switched to a new branch 'test'
7c0544b1a7 dump-guest-memory.py: fix "You can't do that without a process to debug"

=== OUTPUT BEGIN ===
Checking PATCH 1/1: dump-guest-memory.py: fix "You can't do that without a process to debug"...
ERROR: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
#46: FILE: hw/misc/vmcoreinfo.c:39:
+    static VMCoreInfoState * volatile vmcoreinfo_state G_GNUC_UNUSED;

total: 1 errors, 0 warnings, 24 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
Posted by Laszlo Ersek 6 years, 4 months ago 
On 12/12/17 18:26, no-reply@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Message-id: 20171212172208.13588-1-marcandre.lureau@redhat.com
> Subject: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
> Type: series
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> 
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
>     echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
>     if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
>         failed=1
>         echo
>     fi
>     n=$((n+1))
> done
> 
> exit $failed
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  t [tag update]            patchew/20171207203036.14993-1-eblake@redhat.com -> patchew/20171207203036.14993-1-eblake@redhat.com
>  * [new tag]               patchew/20171212172208.13588-1-marcandre.lureau@redhat.com -> patchew/20171212172208.13588-1-marcandre.lureau@redhat.com
> Switched to a new branch 'test'
> 7c0544b1a7 dump-guest-memory.py: fix "You can't do that without a process to debug"
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/1: dump-guest-memory.py: fix "You can't do that without a process to debug"...
> ERROR: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
> #46: FILE: hw/misc/vmcoreinfo.c:39:
> +    static VMCoreInfoState * volatile vmcoreinfo_state G_GNUC_UNUSED;
> 
> total: 1 errors, 0 warnings, 24 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@freelists.org
> 

"volatile" is required in this case; otherwise GCC would be entirely
justified to eliminate the variable.
Re: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
Posted by Fam Zheng 6 years, 4 months ago 
On Tue, 12/12 18:30, Laszlo Ersek wrote:
> On 12/12/17 18:26, no-reply@patchew.org wrote:
> > Hi,
> > 
> > This series seems to have some coding style problems. See output below for
> > more information:
> > 
> > Message-id: 20171212172208.13588-1-marcandre.lureau@redhat.com
> > Subject: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
> > Type: series
> > 
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> > 
> > BASE=base
> > n=1
> > total=$(git log --oneline $BASE.. | wc -l)
> > failed=0
> > 
> > git config --local diff.renamelimit 0
> > git config --local diff.renames True
> > 
> > commits="$(git log --format=%H --reverse $BASE..)"
> > for c in $commits; do
> >     echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> >     if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> >         failed=1
> >         echo
> >     fi
> >     n=$((n+1))
> > done
> > 
> > exit $failed
> > === TEST SCRIPT END ===
> > 
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > From https://github.com/patchew-project/qemu
> >  t [tag update]            patchew/20171207203036.14993-1-eblake@redhat.com -> patchew/20171207203036.14993-1-eblake@redhat.com
> >  * [new tag]               patchew/20171212172208.13588-1-marcandre.lureau@redhat.com -> patchew/20171212172208.13588-1-marcandre.lureau@redhat.com
> > Switched to a new branch 'test'
> > 7c0544b1a7 dump-guest-memory.py: fix "You can't do that without a process to debug"
> > 
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/1: dump-guest-memory.py: fix "You can't do that without a process to debug"...
> > ERROR: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
> > #46: FILE: hw/misc/vmcoreinfo.c:39:
> > +    static VMCoreInfoState * volatile vmcoreinfo_state G_GNUC_UNUSED;
> > 
> > total: 1 errors, 0 warnings, 24 lines checked
> > 
> > Your patch has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > CHECKPATCH in MAINTAINERS.
> > 
> > === OUTPUT END ===
> > 
> > Test command exited with code: 1
> > 
> > 
> > ---
> > Email generated automatically by Patchew [http://patchew.org/].
> > Please send your feedback to patchew-devel@freelists.org
> > 
> 
> "volatile" is required in this case; otherwise GCC would be entirely
> justified to eliminate the variable.

OK, maybe we should relax it to warning instead of error for `volatile` usages,
as long as it follows a comment?

Fam
Re: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix "You can't do that without a process to debug"
Posted by Laszlo Ersek 6 years, 4 months ago 
On 12/12/17 18:22, 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 object_resolve_path_type() function call call with a
> local volatile variable.

Patch looks great and the above is fine with me too -- I just note we
have "call call", which could be improved. But, I don't insist.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 
> v2:
>  - use a vmcoreinfo_realize() local volatile variable
>  - tweak commit message
> 
> scripts/dump-guest-memory.py | 3 +--
>  hw/misc/vmcoreinfo.c         | 3 +++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 1af26c1a45..09bec92b50 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_realize::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..a2805527cb 100644
> --- a/hw/misc/vmcoreinfo.c
> +++ b/hw/misc/vmcoreinfo.c
> @@ -35,6 +35,8 @@ static void vmcoreinfo_realize(DeviceState *dev, Error **errp)
>  {
>      VMCoreInfoState *s = VMCOREINFO(dev);
>      FWCfgState *fw_cfg = fw_cfg_find();
> +    /* for gdb script dump-guest-memory.py */
> +    static VMCoreInfoState * volatile vmcoreinfo_state G_GNUC_UNUSED;
>  
>      /* Given that this function is executing, there is at least one VMCOREINFO
>       * device. Check if there are several.
> @@ -56,6 +58,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 = {
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.