On 07.02.20 14:55, Daniel P. Berrangé wrote: > When initializing the LUKS header the size with default encryption > parameters will currently be 2068480 bytes. This is rounded up to > a multiple of the cluster size, 2081792, with 64k sectors. If the > end of the header is not the same as the end of the cluster we fill > the extra space with zeros. This was forgetting that not even the > space allocated for the header will be fully initialized, as we > only write key material for the first key slot. The space left > for the other 7 slots is never written to. > > An optimization to the ref count checking code: > > commit a5fff8d4b4d928311a5005efa12d0991fe3b66f9 (refs/bisect/bad) > Author: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Date: Wed Feb 27 16:14:30 2019 +0300 > > qcow2-refcount: avoid eating RAM > > made the assumption that every cluster which was allocated would > have at least some data written to it. This was violated by way > the LUKS header is only partially written, with much space simply > reserved for future use. > > Depending on the cluster size this problem was masked by the > logic which wrote zeros between the end of the LUKS header and > the end of the cluster. > > $ qemu-img create --object secret,id=cluster_encrypt0,data=123456 \ > -f qcow2 -o cluster_size=2k,encrypt.iter-time=1,\ > encrypt.format=luks,encrypt.key-secret=cluster_encrypt0 \ > cluster_size_check.qcow2 100M > Formatting 'cluster_size_check.qcow2', fmt=qcow2 size=104857600 > encrypt.format=luks encrypt.key-secret=cluster_encrypt0 > encrypt.iter-time=1 cluster_size=2048 lazy_refcounts=off refcount_bits=16 > > $ qemu-img check --object secret,id=cluster_encrypt0,data=redhat \ > 'json:{"driver": "qcow2", "encrypt.format": "luks", \ > "encrypt.key-secret": "cluster_encrypt0", \ > "file.driver": "file", "file.filename": > "cluster_size_check.qcow2"}' > ERROR: counting reference for region exceeding the end of the file by one > cluster or more: offset 0x2000 size 0x1f9000 > Leaked cluster 4 refcount=1 reference=0 > ...snip... > Leaked cluster 130 refcount=1 reference=0 > > 1 errors were found on the image. > Data may be corrupted, or further writes to the image may corrupt it. > > 127 leaked clusters were found on the image. > This means waste of disk space, but no harm to data. > Image end offset: 268288 > > The problem only exists when the disk image is entirely empty. Writing > data to the disk image payload will solve the problem by causing the > end of the file to be extended further. > > The change fixes it by ensuring that the entire allocated LUKS header > region is fully initialized with zeros. The qemu-img check will still > fail for any pre-existing disk images created prior to this change, > unless at least 1 byte of the payload is written to. > > Fully writing zeros to the entire LUKS header is a good idea regardless > as it ensures that space has been allocated on the host filesystem (or > whatever block storage backend is used). > > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > --- > > In v2: > > - Cut down test scenarios to speed up > - Use _check_test_img helper > - Fix outdated comments > - Use space not tabs
Using eight spaces for indentation is... Interesting, but at least consistent. :-) > block/qcow2.c | 11 +++-- > tests/qemu-iotests/284 | 97 ++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/284.out | 62 ++++++++++++++++++++++++ > tests/qemu-iotests/group | 1 + > 4 files changed, 167 insertions(+), 4 deletions(-) > create mode 100755 tests/qemu-iotests/284 > create mode 100644 tests/qemu-iotests/284.out Thanks, applied to my block branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block Max
signature.asc
Description: OpenPGP digital signature