On 26.11.15 16:31, Dr. David Alan Gilbert wrote: > * Alexander Graf ([email protected]) wrote: >> >> >> On 30.10.15 17:50, Mark Cave-Ayland wrote: >>> 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 <[email protected]> >>>>>>> --- >>>>>>> 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. >> >> Juan, David? > > Isn't this already in ( 96e5c9bc77acef8b7b56cbe23a8a2611feff9e34 ) - or is > this a different breakage?
Awesome, I didn't see an accepted email :). Sorry for the fuss. Alex
