From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46641) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAaKW-0004LG-TK for qemu-devel@nongnu.org; Thu, 02 Jul 2015 04:58:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZAaKT-0005zt-MV for qemu-devel@nongnu.org; Thu, 02 Jul 2015 04:58:48 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:57893) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZAaKT-0005zj-EA for qemu-devel@nongnu.org; Thu, 02 Jul 2015 04:58:45 -0400 Received: from /spool/local by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 2 Jul 2015 09:58:42 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id C54531B0804B for ; Thu, 2 Jul 2015 09:59:48 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t628wekO36307116 for ; Thu, 2 Jul 2015 08:58:40 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t628wcZk026844 for ; Thu, 2 Jul 2015 02:58:38 -0600 Message-ID: <5594FD3D.4060705@de.ibm.com> Date: Thu, 02 Jul 2015 10:58:37 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1435825323-2806-1-git-send-email-dgilbert@redhat.com> In-Reply-To: <1435825323-2806-1-git-send-email-dgilbert@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] check_section_footers: Check the correct section_id List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" , qemu-devel@nongnu.org Cc: amit.shah@redhat.com, quintela@redhat.com Am 02.07.2015 um 10:22 schrieb Dr. David Alan Gilbert (git): > From: "Dr. David Alan Gilbert" > > The section footers check was incorrectly checking the section_id > in the SaveStateEntry not the LoadStateEntry. These can validly be different > if the two QEMU instances have instantiated their devices in a > different order. The test only cares that we're finishing the same > section we started, and hence it's the LoadStateEntry that we care about. > > Signed-off-by: Dr. David Alan Gilbert > Reported-by: Christian Borntraeger Tested-by: Christian Borntraeger > --- > migration/savevm.c | 74 +++++++++++++++++++++++++++--------------------------- > 1 file changed, 37 insertions(+), 37 deletions(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index 1a9b00b..acbad9c 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -653,41 +653,6 @@ static void save_section_footer(QEMUFile *f, SaveStateEntry *se) > } > } > > -/* > - * Read a footer off the wire and check that it matches the expected section > - * > - * Returns: true if the footer was good > - * false if there is a problem (and calls error_report to say why) > - */ > -static bool check_section_footer(QEMUFile *f, SaveStateEntry *se) > -{ > - uint8_t read_mark; > - uint32_t read_section_id; > - > - if (skip_section_footers) { > - /* No footer to check */ > - return true; > - } > - > - read_mark = qemu_get_byte(f); > - > - if (read_mark != QEMU_VM_SECTION_FOOTER) { > - error_report("Missing section footer for %s", se->idstr); > - return false; > - } > - > - read_section_id = qemu_get_be32(f); > - if (read_section_id != se->section_id) { > - error_report("Mismatched section id in footer for %s -" > - " read 0x%x expected 0x%x", > - se->idstr, read_section_id, se->section_id); > - return false; > - } > - > - /* All good */ > - return true; > -} > - > bool qemu_savevm_state_blocked(Error **errp) > { > SaveStateEntry *se; > @@ -989,6 +954,41 @@ struct LoadStateEntry { > int version_id; > }; > > +/* > + * Read a footer off the wire and check that it matches the expected section > + * > + * Returns: true if the footer was good > + * false if there is a problem (and calls error_report to say why) > + */ > +static bool check_section_footer(QEMUFile *f, LoadStateEntry *le) > +{ > + uint8_t read_mark; > + uint32_t read_section_id; > + > + if (skip_section_footers) { > + /* No footer to check */ > + return true; > + } > + > + read_mark = qemu_get_byte(f); > + > + if (read_mark != QEMU_VM_SECTION_FOOTER) { > + error_report("Missing section footer for %s", le->se->idstr); > + return false; > + } > + > + read_section_id = qemu_get_be32(f); > + if (read_section_id != le->section_id) { > + error_report("Mismatched section id in footer for %s -" > + " read 0x%x expected 0x%x", > + le->se->idstr, read_section_id, le->section_id); > + return false; > + } > + > + /* All good */ > + return true; > +} > + > void loadvm_free_handlers(MigrationIncomingState *mis) > { > LoadStateEntry *le, *new_le; > @@ -1082,7 +1082,7 @@ int qemu_loadvm_state(QEMUFile *f) > " device '%s'", instance_id, idstr); > goto out; > } > - if (!check_section_footer(f, le->se)) { > + if (!check_section_footer(f, le)) { > ret = -EINVAL; > goto out; > } > @@ -1109,7 +1109,7 @@ int qemu_loadvm_state(QEMUFile *f) > section_id, le->se->idstr); > goto out; > } > - if (!check_section_footer(f, le->se)) { > + if (!check_section_footer(f, le)) { > ret = -EINVAL; > goto out; > } >