From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60943) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1yWz-0002qd-8L for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:32:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1yWw-0004Ji-0o for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:32:21 -0500 Received: from mx2.suse.de ([195.135.220.15]:34828) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1yWv-0004It-ND for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:32:17 -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> <565723D8.1070401@suse.de> <20151126153108.GC18216@work-vm> From: Alexander Graf Message-ID: <56572600.9080703@suse.de> Date: Thu, 26 Nov 2015 16:32:16 +0100 MIME-Version: 1.0 In-Reply-To: <20151126153108.GC18216@work-vm> 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: "Dr. David Alan Gilbert" Cc: Mark Cave-Ayland , qemu-devel@nongnu.org, quintela@redhat.com On 26.11.15 16:31, Dr. David Alan Gilbert wrote: > * Alexander Graf (agraf@suse.de) 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 >>>>>>> --- >>>>>>> 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