Re: [Qemu-devel] [PATCH] migration: fix analyze-migration.py script

[Mark Cave-Ayland]

On 26/10/15 09:48, Mark Cave-Ayland wrote:

> On 06/09/15 12:54, Mark Cave-Ayland wrote:
> 
>> On 06/09/15 09:36, Alexander Graf wrote:
>>
>>> On 05.09.15 21:51, Mark Cave-Ayland wrote:
>>>> Commit 61964 "Add configuration section" broke the analyze-migration.py 
>>>> script
>>>> which terminates due to the unrecognised section. Fix the script by parsing
>>>> the contents of the configuration section directly into a new
>>>> ConfigurationSection object (although nothing is done with it yet).
>>>>
>>>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>>>> ---
>>>>  scripts/analyze-migration.py |   13 +++++++++++++
>>>>  1 file changed, 13 insertions(+)
>>>>
>>>> diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
>>>> index f6894be..1455387 100755
>>>> --- a/scripts/analyze-migration.py
>>>> +++ b/scripts/analyze-migration.py
>>>> @@ -252,6 +252,15 @@ class HTABSection(object):
>>>>      def getDict(self):
>>>>          return ""
>>>>  
>>>> +
>>>> +class ConfigurationSection(object):
>>>> +    def __init__(self, file):
>>>> +        self.file = file
>>>> +
>>>> +    def read(self):
>>>> +        name_len = self.file.read32()
>>>> +        name = self.file.readstr(len = name_len)
>>>> +
>>>>  class VMSDFieldGeneric(object):
>>>>      def __init__(self, desc, file):
>>>>          self.file = file
>>>> @@ -474,6 +483,7 @@ class MigrationDump(object):
>>>>      QEMU_VM_SECTION_FULL  = 0x04
>>>>      QEMU_VM_SUBSECTION    = 0x05
>>>>      QEMU_VM_VMDESCRIPTION = 0x06
>>>> +    QEMU_VM_CONFIGURATION = 0x07
>>>>      QEMU_VM_SECTION_FOOTER= 0x7e
>>>>  
>>>>      def __init__(self, filename):
>>>> @@ -514,6 +524,9 @@ class MigrationDump(object):
>>>>              section_type = file.read8()
>>>>              if section_type == self.QEMU_VM_EOF:
>>>>                  break
>>>> +            elif section_type == self.QEMU_VM_CONFIGURATION:
>>>> +                section = ConfigurationSection(file)
>>>> +                section.read()
>>>
>>> So since we don't have a normal section header, there is no version
>>> field either. That in turn means that the format is determined by the
>>> machine version only - bleks.
>>
>> Yes :(  I double-checked the output of a migration file with hexdump and
>> confirmed that just the raw fields are included without any additional
>> metadata, even though the state is held in a VMStateDescription.
>>
>>> So if there ever has to be more in the configuration section than the
>>> machine name, please move to a more detectable scheme. Ideally something
>>> that contains
>>>
>>>   * version
>>>   * length of dynamically sized fields
>>>   * lenght of full blob
>>>
>>> would be ideal, so that we have a chance to at least put code into the
>>> analyze script to examine it.
>>>
>>> For now, I think the hard coded solution in the analyze script is
>>> reasonable.
>>>
>>> However, I think we should print out the name if we find it. It should
>>> be as simple as adding a special case for the configuration section in
>>> MigrationDump.getDict().
>>
>> I did play with adding a separate JSON object for configuration but was
>> torn between whether configuration should have its own JSON object
>> (better if we include extra fields and metadata as above) or to just
>> embed it as a simple "machine" property similar to "page_size". I'll
>> wait until we hear back from David/Juan and submit a v2 accordingly.
> 
> Ping again from Juan/David? The analyze-migration.py script is currently
> broken without this patch (or another equivalent) applied.

Ping^2? FWIW I've added this to the wiki at
http://wiki.qemu.org/Planning/2.5 since without this patch or something
similar applied, this feature is completely broken.


ATB,

Mark.