Re: [Qemu-devel] [PATCH for-2.12 v5] iotests: Test abnormally large size in compressed cluster descriptor

From: Eric Blake <eblake@redhat.com>
To: Alberto Garcia <berto@igalia.com>, qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH for-2.12 v5] iotests: Test abnormally large size in compressed cluster descriptor
Date: Thu, 29 Mar 2018 09:37:06 -0500	[thread overview]
Message-ID: <c4589769-90ac-dbe7-7f54-84700793a77e@redhat.com> (raw)
In-Reply-To: <20180329120745.11154-1-berto@igalia.com>

On 03/29/2018 07:07 AM, Alberto Garcia wrote:
> L2 entries for compressed clusters have a field that indicates the
> number of sectors used to store the data in the image.
> 
> That's however not the size of the compressed data itself, just the
> number of sectors where that data is located. The actual data size is
> usually not a multiple of the sector size, and therefore cannot be
> represented with this field.
> 

> 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.

Is it still worth trying to add such a check in 2.12?  (Probably not - 
the bug has been there "since the beginning", so it's not a regression 
and thus not a showstopper if it stays there for another release)

> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
> v5: Use 'write -c' instead of 'write' followed by 'convert' [Max]
>      Add TODO comment explaining that the size of compressed clusters
>      should also be corrected when it's too large in order to avoid
>      referencing other unrelated clusters.

Thanks, those changes look good.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org