[Qemu-devel] [PULL 09/10] scripts/dump-guest-memory: Synchronize with guest_phys_blocks_region_add

Paolo Bonzini posted 10 patches 7 years ago
[Qemu-devel] [PULL 09/10] scripts/dump-guest-memory: Synchronize with guest_phys_blocks_region_add
Posted by Paolo Bonzini 7 years ago
Recent patches have removed ram_device and nonvolatile RAM
from dump-guest-memory's output.  Do the same for dumps
that are extracted from a QEMU core file.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/dump-guest-memory.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 5a857ce..f04697b 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -417,7 +417,9 @@ def get_guest_phys_blocks():
         memory_region = flat_range["mr"].dereference()
 
         # we only care about RAM
-        if not memory_region["ram"]:
+        if not memory_region["ram"] \
+           or memory_region["ram_device"] \
+           or memory_region["nonvolatile"]:
             continue
 
         section_size = int128_get64(flat_range["addr"]["size"])
-- 
1.8.3.1



Re: [Qemu-devel] [PULL 09/10] scripts/dump-guest-memory: Synchronize with guest_phys_blocks_region_add
Posted by Laszlo Ersek 7 years ago
On 10/30/18 20:50, Paolo Bonzini wrote:
> Recent patches have removed ram_device and nonvolatile RAM
> from dump-guest-memory's output.  Do the same for dumps
> that are extracted from a QEMU core file.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/dump-guest-memory.py | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
> index 5a857ce..f04697b 100644
> --- a/scripts/dump-guest-memory.py
> +++ b/scripts/dump-guest-memory.py
> @@ -417,7 +417,9 @@ def get_guest_phys_blocks():
>          memory_region = flat_range["mr"].dereference()
>  
>          # we only care about RAM
> -        if not memory_region["ram"]:
> +        if not memory_region["ram"] \
> +           or memory_region["ram_device"] \
> +           or memory_region["nonvolatile"]:
>              continue
>  
>          section_size = int128_get64(flat_range["addr"]["size"])
> 

Sorry about the late comment, I've been away.

The line continuation style in the python script is inconsistent. When I
wrote the original version, my understanding was that the "Pythonic" way
to break up lines was to open a new parenthesized subexpression. This
way the logical "or" operator could be left at the end of the line. See
e.g. in the "get_guest_phys_blocks" method.

https://www.python.org/dev/peps/pep-0008/#maximum-line-length

> The preferred way of wrapping long lines is by using Python's implied
> line continuation inside parentheses, brackets and braces. Long lines
> can be broken over multiple lines by wrapping expressions in
> parentheses. These should be used in preference to using a backslash
> for line continuation.

However, several trailing backslashes have been added since, and I've
totally failed to catch them. I guess at this point either style should
be acceptable, in this script.

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

Thanks
Laszlo

Re: [Qemu-devel] [PULL 09/10] scripts/dump-guest-memory: Synchronize with guest_phys_blocks_region_add
Posted by Paolo Bonzini 7 years ago
On 05/11/2018 16:46, Laszlo Ersek wrote:
> On 10/30/18 20:50, Paolo Bonzini wrote:
>> Recent patches have removed ram_device and nonvolatile RAM
>> from dump-guest-memory's output.  Do the same for dumps
>> that are extracted from a QEMU core file.
>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  scripts/dump-guest-memory.py | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
>> index 5a857ce..f04697b 100644
>> --- a/scripts/dump-guest-memory.py
>> +++ b/scripts/dump-guest-memory.py
>> @@ -417,7 +417,9 @@ def get_guest_phys_blocks():
>>          memory_region = flat_range["mr"].dereference()
>>  
>>          # we only care about RAM
>> -        if not memory_region["ram"]:
>> +        if not memory_region["ram"] \
>> +           or memory_region["ram_device"] \
>> +           or memory_region["nonvolatile"]:
>>              continue
>>  
>>          section_size = int128_get64(flat_range["addr"]["size"])
>>
> 
> Sorry about the late comment, I've been away.
> 
> The line continuation style in the python script is inconsistent. When I
> wrote the original version, my understanding was that the "Pythonic" way
> to break up lines was to open a new parenthesized subexpression. This
> way the logical "or" operator could be left at the end of the line. See
> e.g. in the "get_guest_phys_blocks" method.
> 
> https://www.python.org/dev/peps/pep-0008/#maximum-line-length
> 
>> The preferred way of wrapping long lines is by using Python's implied
>> line continuation inside parentheses, brackets and braces. Long lines
>> can be broken over multiple lines by wrapping expressions in
>> parentheses. These should be used in preference to using a backslash
>> for line continuation.
> 
> However, several trailing backslashes have been added since, and I've
> totally failed to catch them. I guess at this point either style should
> be acceptable, in this script.
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Fixed this one, thanks.

Paolo