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 <mark.cave-ayl...@ilande.co.uk> >>>> --- >>>> 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.
