From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47379) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuiAh-0003VW-Kx for qemu-devel@nongnu.org; Tue, 19 May 2015 10:07:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YuiAb-0002as-3J for qemu-devel@nongnu.org; Tue, 19 May 2015 10:07:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50479) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YuiAa-0002al-Sg for qemu-devel@nongnu.org; Tue, 19 May 2015 10:06:57 -0400 Date: Tue, 19 May 2015 15:06:50 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20150519140649.GB2127@work-vm> References: <1432034993-24431-1-git-send-email-dgilbert@redhat.com> <555B3FF1.6090205@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <555B3FF1.6090205@redhat.com> 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: Eric Blake Cc: amit.shah@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org, agraf@suse.de * Eric Blake (eblake@redhat.com) wrote: > On 05/19/2015 05:29 AM, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" > > > > 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 produce > > 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. > > > > This patch series adds a section footer consisting of a marker (0x7e - ~) > > followed by the section-id that was also sent in the header. If > > they mismatch then it throws an error explaining which section was > > being loaded. > > Good idea. > Is it redundant with the recent addition of self-describing > json that newer machine types send? No, that self-describing json goes at the end, and is for parsing by an external-entity; it doesn't help detect corruption on loading. > 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 section 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-of-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 section header, then we try and read the next few bytes to figure out which section; 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 problem in that section. In either case you don't get an error pointing to the previous section which was the actual problem. > > > > The footers are tied to new machine types (on both pc types). > > Good that you tied it to machine type, but is it enough? When we added > the optional section for giving the json representation of the stream, > we ended up having to add a knob to turn off that section, so that > backwards migration from a new qemu to an older one did not send it. > I'm wondering if we'll need to expose a knob to turn off footers, again > for the sake of backwards migration in downstream distros. That knob is already the knob that I've created and tied to the machine type; the downstream distros will just turn the same knob in their old machine types. Dave > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK