Re: [Qemu-devel] [PATCH v7 6/9] qcow2: add bdrv_measure() support

[Stefan Hajnoczi]

On Tue, Jun 13, 2017 at 05:07:13PM +0200, Alberto Garcia wrote:
> On Tue 13 Jun 2017 03:33:26 PM CEST, Stefan Hajnoczi <address@hidden> wrote:
> > Use qcow2_calc_prealloc_size() to get the required file size.
> >
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > Reviewed-by: Alberto Garcia <address@hidden>
> 
> You kept my R-b here but one of the changes was in this patch:

Sorry, I forgot to remove it when making the change.

> > +    info = g_new(BlockMeasureInfo, 1);
> > +    info->fully_allocated =
> > +        qcow2_calc_prealloc_size(virtual_size, cluster_size,
> > +                                 ctz32(refcount_bits));
> > +    if (DIV_ROUND_UP(info->fully_allocated, cluster_size) > INT_MAX) {
> > +        g_free(info);
> > +        error_setg(&local_err, "The image size is too large "
> > +                               "(try using a larger cluster size)");
> > +        goto err;
> > +    }
> 
> This has the opposite problem than the previous version: valid image
> sizes are now rejected by the 'measure' command.
> 
> $ qemu-img create -f qcow2 img.qcow2 1P
> Formatting 'img.qcow2', fmt=qcow2 size=1125899906842624 encryption=off 
> cluster_size=65536 lazy_refcounts=off refcount_bits=16
> 
> $ build/qemu-img measure -O qcow2 --size 1P
> qemu-img: The image size is too large (try using a larger cluster size)

Hmm...if host file size (not virtual disk size) is 1P then qemu-img
check fails:

int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                          BdrvCheckMode fix)
{
    int64_t size, highest_cluster, nb_clusters;
    ...

    size = bdrv_getlength(bs->file->bs);
    if (size < 0) {
        res->check_errors++;
        return size;
    }

    nb_clusters = size_to_clusters(s, size);
    if (nb_clusters > INT_MAX) {
        res->check_errors++;
        return -EFBIG;
    }

This is also where I got the limit from.  It was introduced in:

commit 0abe740f1de899737242bcba1fb4a9857f7a3087
Author: Kevin Wolf <address@hidden>
Date:   Wed Mar 26 13:05:52 2014 +0100

    qcow2: Protect against some integer overflows in bdrv_check

At that point the code used ints so the overflow check was necessary.
Now the code uses 64-bit types so INT_MAX is obsolete.

I will send a separate patch to fix qemu-img check.

> The actual limit is:
> 
> #define QCOW_MAX_L1_SIZE 0x2000000
> 
> That's 4194304 entries, each one can address cluster_size^2 / 8 bytes
> 
> So using that formula, here is the maximum virtual size depending on the
> cluster size:
> 
> |--------------+------------------|
> | Cluster size | Max virtual size |
> |--------------+------------------|
> | 512 bytes    | 128 GB           |
> | 1 KB         | 512 GB           |
> | 2 KB         | 2 TB             |
> | 4 KB         | 8 TB             |
> | 8 KB         | 32 TB            |
> | 16 KB        | 128 TB           |
> | 32 KB        | 512 TB           |
> | 64 KB        | 2 PB             |
> | 128 KB       | 8 PB             |
> | 256 KB       | 32 PB            |
> | 512 KB       | 128 PB           |
> | 1 MB         | 512 PB           |
> | 2 MB         | 2 EB             |
> |--------------+------------------|
> 
> I just created a 2 EB image and it works fine, Linux can detect it
> without problems, I can create a file system, etc.
> 
> If you specify a larger size, qcow2_grow_l1_table() fails with -EFIB.

Thanks, will fix!

Stefan

signature.asc
Description: PGP signature