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. ATB, Mark.
