Re: [Qemu-devel] [PATCH v2] dump-guest-memory.py: fix python 2 support

[Laszlo Ersek]

On 01/19/18 20:48, Eric Blake wrote:
> On 01/19/2018 01:18 PM, Laszlo Ersek wrote:
>> On 01/19/18 17:25, Marc-André Lureau wrote:
>>> Python GDB support may use Python 2 or 3.
>>>
>>> Inferior.read_memory() may return a 'buffer' with Python 2 or a
>>> 'memoryview' with Python 3 (see also
>>> https://sourceware.org/gdb/onlinedocs/gdb/Inferiors-In-Python.html)
>>>
>>> The elf.add_vmcoreinfo_note() method expects a "bytes" object. Wrap
>>> the returned memory with bytes(), which works with both 'memoryview'
>>> and 'buffer'.
>>>
> 
> If I understand it, the issue stems from the fact that vmcoreinfo is a
> bytes buffer under python2, but a memoryview under python3.  Let's
> compare the patches:
> 
> The V1 patch:
> https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg04010.html
> 
>>
>>          if vmcoreinfo:
>> -            self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
>> +            # Python 2.7 returns a buffer
>> +            vmciview = memoryview(vmcoreinfo)
>> +            self.elf.add_vmcoreinfo_note(vmciview.tobytes())
> 
> did an unconditional memoryview(vmcoreinfo) followed by a .tobytes()
> call. Under python 2.7, vmcoreinfo is converted from a buffer to a
> memoryview, then back to bytes; under python 3, memoryview(vmcoreinfo)
> is a no-op (since it is already a memory view), then that is converted
> to bytes; the double conversion was necessary because bytes.tobytes()
> doesn't exist.  But python 2.6 doesn't have the memoryview conversion,
> so we've excluded older python users.
> 
> Now looking at the V2 patch:
> 
> 
>>> @@ -564,7 +564,7 @@ shape and this command should mostly work."""
>>>  
>>>          vmcoreinfo = self.phys_memory_read(addr, size)
>>>          if vmcoreinfo:
>>> -            self.elf.add_vmcoreinfo_note(vmcoreinfo.tobytes())
>>> +            self.elf.add_vmcoreinfo_note(bytes(vmcoreinfo))
> 
> Under python2, and bytes(buffer) is still bytes; under python3,
> vmcoreinfo is a memoryview but bytes(buffer) is equivalent to
> buffer.to_bytes().  We've avoided the intermediate conversion through
> memoryview, and thus work regardless of python version.

Heh, "semantics" :)

> 
>> Ugh, can you please point me to the v1->v2 difference?
> 
> Hope that helped (and hope I got it right, I'm learning as well from
> this thread).
> 

It sure helped a lot, thank you very much! In fact, you've done all the
review work, so technically, "R-b: Laszlo" would be misappropriation.
Can you give your R-b?

I'll manage an "I agree" statement:

Acked-by: Laszlo Ersek <address@hidden>

Thanks!
Laszlo