Re: [Qemu-devel] [PATCH v2 3/3] qcow2: Avoid memory over-allocation on compressed images

[Eric Blake]

On 02/22/2018 04:50 AM, Alberto Garcia wrote:

On Thu 22 Feb 2018 12:39:53 AM CET, Eric Blake wrote:

+assert(!!s->cluster_data == !!s->cluster_cache);
+assert(csize < 2 * s->cluster_size + 512);
  if (!s->cluster_data) {
-/* one more sector for decompressed data alignment */
-s->cluster_data = qemu_try_blockalign(bs->file->bs,
-QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
+s->cluster_data = g_try_malloc(2 * s->cluster_size + 512);
  if (!s->cluster_data) {
  return -ENOMEM;
  }


Why the "+ 512" ?


I was thinking "number of sectors is up to two clusters, and we add one, 
PLUS we must read from the initial sector containing the offset".  But I 
was obviously not careful enough - the maximum value (all 1s) is 512 
bytes short of 2 full clusters, and we add at most 511 more bytes for 
the initial sector containing the offset (our +1 covers the leading 
sector).  So you are right, I can tighten this down for a slightly 
smaller allocation (and a nice power-of-2 allocation may be slightly 
more efficient as well).  v3 coming up.


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

Re: [Qemu-devel] [PATCH v2 3/3] qcow2: Avoid memory over-allocation on compressed images

[Alberto Garcia]

On Thu 22 Feb 2018 12:39:53 AM CET, Eric Blake wrote:
> +assert(!!s->cluster_data == !!s->cluster_cache);
> +assert(csize < 2 * s->cluster_size + 512);
>  if (!s->cluster_data) {
> -/* one more sector for decompressed data alignment */
> -s->cluster_data = qemu_try_blockalign(bs->file->bs,
> -QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
> +s->cluster_data = g_try_malloc(2 * s->cluster_size + 512);
>  if (!s->cluster_data) {
>  return -ENOMEM;
>  }

Why the "+ 512" ?

nb_csectors is guaranteed to be at most twice the cluster size, you can
even assert that:

int max_csize = (s->csize_mask + 1) * 512;
assert(max_csize == s->cluster_size * 2);
s->cluster_data = qemu_try_blockalign(bs->file->bs, max_csize);

And csize is at most (max_csize - sector_offset), so you can change your
assertion to this:

   assert(csize <= 2 * s->cluster_size);

Berto