[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] iotests: Test abnormally large size in compr
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v2] iotests: Test abnormally large size in compressed cluster descriptor |
Date: |
Mon, 26 Feb 2018 14:44:33 +0100 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
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