From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a24H7-00026M-7J for qemu-devel@nongnu.org; Thu, 26 Nov 2015 16:40:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a24H3-0001r0-58 for qemu-devel@nongnu.org; Thu, 26 Nov 2015 16:40:21 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45928) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a24H2-0001qp-Tf for qemu-devel@nongnu.org; Thu, 26 Nov 2015 16:40:17 -0500 From: Juan Quintela In-Reply-To: <56572600.9080703@suse.de> (Alexander Graf's message of "Thu, 26 Nov 2015 16:32:16 +0100") 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> <56572600.9080703@suse.de> Date: Thu, 26 Nov 2015 22:40:08 +0100 Message-ID: <87egfcignb.fsf@emacs.mitica> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] migration: fix analyze-migration.py script Reply-To: quintela@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Mark Cave-Ayland , "Dr. David Alan Gilbert" , qemu-devel@nongnu.org Alexander Graf wrote: > 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. Opps, my fault. I do that by hand, and then sometimes I forget. Later, Juan.