From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60645) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1yVx-0001Yg-Ee for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:31:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1yVu-0003nQ-6k for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:31:17 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46028) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1yVu-0003nI-00 for qemu-devel@nongnu.org; Thu, 26 Nov 2015 10:31:14 -0500 Date: Thu, 26 Nov 2015 15:31:08 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20151126153108.GC18216@work-vm> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <565723D8.1070401@suse.de> Subject: Re: [Qemu-devel] [PATCH] migration: fix analyze-migration.py script List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Mark Cave-Ayland , qemu-devel@nongnu.org, quintela@redhat.com * 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? Dave > > > Alex -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK