Re: [Qemu-devel] [PATCH v7 11/25] qcow2: refcount_order parameter for qcow2_create2

[Eric Blake]

On 02/18/2015 03:40 PM, Max Reitz wrote:
> Add a refcount_order parameter to qcow2_create2(), use that value for
> the image header and for calculating the size required for
> preallocation.
> 
> For now, always pass 4.
> 
> This addition requires changes to the calculation of the file size for
> the "full" and "falloc" preallocation modes. That in turn is a nice
> opportunity to add a comment about that calculation not necessarily
> being exact (and that being intentional).
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/qcow2.c | 47 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 36 insertions(+), 11 deletions(-)
> 

> @@ -2010,6 +2022,8 @@ static int qcow2_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>      size_t cluster_size = DEFAULT_CLUSTER_SIZE;
>      PreallocMode prealloc;
>      int version = 3;
> +    uint64_t refcount_bits = 16;
> +    int refcount_order;
>      Error *local_err = NULL;
>      int ret;
>  
> @@ -2064,8 +2078,19 @@ static int qcow2_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>          goto finish;
>      }
>  
> +    if (version < 3 && refcount_bits != 16) {
> +        error_setg(errp, "Different refcount widths than 16 bits require "
> +                   "compatibility level 1.1 or above (use compat=1.1 or "
> +                   "greater)");
> +        ret = -EINVAL;
> +        goto finish;
> +    }
> +
> +    refcount_order = ffs(refcount_bits) - 1;

ffs() doesn't work on uint64_t (it gives the wrong answer for
0x100000000, for example); you want to use ffsll().  But ffsll() isn't
portable.  But we have include/qemu/host-utils.h that gives us ctz64()
which is what we want (where ctz==ffs-1 other than for the special case
of 0).  So this should be:

refcount_order = ctz64(refcount_bits);

With that change,
Reviewed-by: Eric Blake <address@hidden>

(Hmm, that means that at least qcow2.c:qcow2_create2() has a bug for
calling ffs(size_t), and we probably ought to eradicate other uses of
ffs from the tree - but as a separate followup).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature