On Fri 23 Feb 2018 02:30:14 PM CET, Eric Blake wrote: >> One possible task for the future is to make 'qemu-img check' verify >> the sizes of the compressed clusters, by trying to decompress the data >> and checking that the size stored in the L2 entry is correct. > > Indeed, but that means... > >> + >> +# Reduce size of compressed data to 4 sectors: this corrupts the image. >> +poke_file "$TEST_IMG" $((0x800000)) "\x40\x06" >> +$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | >> _filter_testdir >> + >> +# 'qemu-img check' however doesn't see anything wrong because it >> +# doesn't try to decompress the data and the refcounts are consistent. >> +_check_test_img > > ...this spot should have a TODO comment that mentions the test needs > updating if qemu-img check is taught to be pickier.
Hehe, I actually had a TODO there but decided to remove it in the last moment. > Hmm - I also wonder - does our refcount code properly account for a > compressed cluster that would affect the refcount of THREE clusters? > Remember, qemu will never emit a compressed cluster that touches more > than two clusters, but when you enlarge the size, if offset part of > the link was already in the tail of one cluster, then you can bleed > over into not just one, but two additional host clusters. Your test > didn't cover that, because it uses a compressed cluster that maps to > the start of the host cluster. Yes, just fine. I could actually check that by corrupting the second compressed cluster instead of the first one. Or both, in fact. I'll send v3 with this change then. Berto
