[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted mig
Dr. David Alan Gilbert
Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams
Tue, 19 May 2015 15:06:50 +0100
* 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
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
Dr. David Alan Gilbert / address@hidden / Manchester, UK
Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams, Amit Shah, 2015/05/20
- [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams, Dr. David Alan Gilbert (git), 2015/05/19
- [Qemu-devel] [PATCH 2/4] Disable section footers on older machine types, Dr. David Alan Gilbert (git), 2015/05/19
- [Qemu-devel] [PATCH 4/4] Teach analyze-migration.py about section footers, Dr. David Alan Gilbert (git), 2015/05/19
- [Qemu-devel] [PATCH 1/4] Merge section header writing, Dr. David Alan Gilbert (git), 2015/05/19
- [Qemu-devel] [PATCH 3/4] Add a protective section footer, Dr. David Alan Gilbert (git), 2015/05/19
- Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams, Eric Blake, 2015/05/19