Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams

[Dr. David Alan Gilbert]

* Eric Blake (address@hidden) wrote:
> On 05/19/2015 05:29 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > 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 / address@hidden / Manchester, UK