From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59023) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1yO8-0006zq-Cd for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:23:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1yO4-0002BS-A9 for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:23:12 -0500 Received: from mx2.suse.de ([195.135.220.15]:34356) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1yO3-0002Aw-Tb for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:23:08 -0500 References: <1441482708-31848-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1441482708-31848-2-git-send-email-mark.cave-ayland@ilande.co.uk> <55EBFB0E.7080502@suse.de> <55EC2959.4080800@ilande.co.uk> <562DF6EF.1070400@ilande.co.uk> <56339FED.3030206@ilande.co.uk> From: Alexander Graf Message-ID: <565723D8.1070401@suse.de> Date: Thu, 26 Nov 2015 16:23:04 +0100 MIME-Version: 1.0 In-Reply-To: <56339FED.3030206@ilande.co.uk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] migration: fix analyze-migration.py script List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland Cc: quintela@redhat.com, qemu-devel@nongnu.org, dgilbert@redhat.com 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 >>>>> --- >>>>> 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? Alex