[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
Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams
Tue, 19 May 2015 08:40:29 -0600
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
On 05/19/2015 08:28 AM, Dr. David Alan Gilbert wrote:
> * Eric Blake (address@hidden) 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 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
>>> 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
>>> 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.
>> Probably worth incorporating into the commit body then :)
> 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 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.
> which is pretty close to that; do you want me to flip that other explanation
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=on, commit 9850c604
> That shouldn't be necessary; VMdesc was 'odd' in that it wrote data after
> 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 takes
> two functions and ~20 lines of code to add one boolean flag to the machine
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.
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/4] Add section footers to detect corrupted migration streams, Amit Shah, 2015/05/20
- Re: [Qemu-devel] [PATCH 2/4] Disable section footers on older machine types, (continued)