On 09.11.2012 09:26, Paolo Bonzini wrote:
Il 08/11/2012 20:05, Gerhard Wiesinger ha scritto:
Fixed a MAJOR BUG in VMDK files on file boundaries on reads
and ALSO ON WRITES WHICH MIGHT CORRUPT THE IMAGE AND DATA!!!!!!
Triggered for example with the following VMDK file (partly listed):
# Extent description
RW 4193792 FLAT "XP-W1-f001.vmdk" 0
RW 2097664 FLAT "XP-W1-f002.vmdk" 0
RW 4193792 FLAT "XP-W1-f003.vmdk" 0
RW 512 FLAT "XP-W1-f004.vmdk" 0
RW 4193792 FLAT "XP-W1-f005.vmdk" 0
RW 2097664 FLAT "XP-W1-f006.vmdk" 0
RW 4193792 FLAT "XP-W1-f007.vmdk" 0
RW 512 FLAT "XP-W1-f008.vmdk" 0
Patch includes:
1.) Patch fixes wrong calculation on extent boundaries. Especially it
fixes the relativeness of the sector number to the current extent.
Please just fix _this_ part. Everything else is not necessary for example
for distributions to fix this. It's an important bug, so we actually want
to make that as simple as this.
Sent.
2.) Added debug code to block.c and to block/vmdk.c to verify correctness
Same here. Also, please use the tracing infrastructure---a lot of the debug
messages you're adding, though not all, are in fact already available (not
saying the others aren't useful!)
Any chance that the patch with debug code only (after some cleaning)
would be accepted (other modules do debug logging, too)?
I don't like to do useless work.
Tracing infrastructure is quite limited to function calls only (as far
as I saw).
3.) Also optimized code which avoids multiplication and uses shifts.
The compiler can do this for you.
Most importantly, making it more complex for reviewers to find only the
"interesting" part.
Please check that the attached patch still works.
Made some tests and gcc is really clever in optimizing. Even other
multipliers (513, 514, ...) than 512 will be optimized to shifts and
adds until a limit of 512+8 (I think it was that) was reached. :-)
Bugfix only resent.
Ciao,
Gerhard
