From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuihA-00083J-9x for qemu-devel@nongnu.org; Tue, 19 May 2015 10:40:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yuih5-0001vn-Vu for qemu-devel@nongnu.org; Tue, 19 May 2015 10:40:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33540) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yuih5-0001vb-OR for qemu-devel@nongnu.org; Tue, 19 May 2015 10:40:31 -0400 Message-ID: <555B4B5D.2010209@redhat.com> Date: Tue, 19 May 2015 08:40:29 -0600 From: Eric Blake MIME-Version: 1.0 References: <1432034993-24431-1-git-send-email-dgilbert@redhat.com> <555B3FF1.6090205@redhat.com> <20150519140649.GB2127@work-vm> <555B4520.7000109@redhat.com> <20150519142838.GC2127@work-vm> In-Reply-To: <20150519142838.GC2127@work-vm> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4k4fXPDNoas2eCHWd7kXSC5h7u5ebKHRp" Subject: Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: amit.shah@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org, agraf@suse.de This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --4k4fXPDNoas2eCHWd7kXSC5h7u5ebKHRp Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/19/2015 08:28 AM, Dr. David Alan Gilbert wrote: > * Eric Blake (eblake@redhat.com) wrote: >> On 05/19/2015 08:06 AM, Dr. David Alan Gilbert wrote: >> >>>> Does it let us detect a corrupted >>>> stream earlier in the process? Or is the main benefit that it gives= >>>> better error messages at the point corruption is first detected? >>> >>> Both; there are two cases that often happen; both triggered by a sect= ion >>> reading too little or too much, and it gets back to the main loop and= >>> we read the next byte: >>> 1) the next byte on the stream is a 0x00 - that's read as an end-o= f-migration >>> marker, we start the VM and you get a hung VM with no errors. >>> >>> 2) the next byte is between 0x01..0x04 - and it looks like a secti= on header, >>> then we try and read the next few bytes to figure out which sec= tion; >>> this could a) result in an error saying it's an unknown section= or >>> b) Happen to match a section ID and then get an error about a p= roblem >>> in that section. In either case you don't get an error pointin= g to >>> the previous section which was the actual problem. >> >> Probably worth incorporating into the commit body then :) >=20 > Well the original text does say: > Badly formatted migration streams can go undetected or produce > misleading errors due to a lock of checking at the end of sections. > In particular a section that adds an extra 0x00 at the end > causes what looks like a normal end of stream and thus doesn't produc= e > any errors, and something that ends in a 0x01..0x04 kind of look > like real section headers and then fail when the section parser tries= > to figure out which section they are. This is made worse by the > choice of 0x00..0x04 being small numbers that are particularly common= > in normal section data. >=20 > which is pretty close to that; do you want me to flip that other explan= ation in? Up to you, but I found the bullet points a bit more descriptive (for example, bullet 1 mentions a hung VM, while the original text says "no errors" but doesn't mention "hung"). >> I'm not asking about the machine type defaults, but a command line >> override: -machine suppress-vmdesc=3Don, commit 9850c604 >=20 > That shouldn't be necessary; VMdesc was 'odd' in that it wrote data aft= er > the end marker which broke some implicit rules about the behaviour > of the streams and the way they interacted with the file buffers. > I'd have sympathy for the opposite direction - i.e. turning on the > footer protection for older machine types when you know you've got > modern qemu's; but it doesn't seem worth the extra boiler plate unless > someone wants to do it (especially since it looks from that like it tak= es > two functions and ~20 lines of code to add one boolean flag to the mach= ine > type!). We may still add it later. And since suppress-vmdesc was added later, I can live with the decision if you don't want to add command-line override on the initial commit series. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --4k4fXPDNoas2eCHWd7kXSC5h7u5ebKHRp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVW0tdAAoJEKeha0olJ0NqpvwIAKU7OrHRoeIxlZrakYLKWWsJ LWFW6ZH1T9u0vMR7GpE+wfhL/CS5yjEXNDK8kobcAwJnbHsDP0Eww+nrVdBxKhgK Z4vAYZY0PhPxjC1NCkGQm6LoZI4yw2EX+wNTbzMkPKpP+QBEguA22OMewsCh/ZdA v9YjXlJKIJ2OSrbqIv7gAeNZ3lIrmGtGkzSs+9J1BXuDDaZIVF/7yHuh7l4NEA8C y33rcRClGds88b2uUCCKDOxxWeN9R5pSZJFJAR5C3bhG8C1LH/geaGFSbgrFhqPS nI4q8cjNeqdV2xsPj/537DtzoZOMjbPVqZRKwvit2eYKt57cx9MLRlxdk+5KQEg= =DlWJ -----END PGP SIGNATURE----- --4k4fXPDNoas2eCHWd7kXSC5h7u5ebKHRp--