Re: [Qemu-devel] [PATCH v12 6/6] qcow2: Add falloc and full preallocation option

[Kevin Wolf <kwolf@redhat.com>]

Am 11.07.2014 um 08:10 hat Hu Tao geschrieben:
> This adds preallocation=falloc and preallocation=full mode to qcow2
> image creation.
> 
> preallocation=full allocates disk space by writing zeros to disk to
> ensure disk space in any cases.
> 
> preallocation=falloc likes preallocation=full, but allocates disk space
> by posix_fallocate().
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  block/qcow2.c              | 31 ++++++++++++++++++++++++--
>  tests/qemu-iotests/082.out | 54 +++++++++++++++++++++++-----------------------
>  2 files changed, 56 insertions(+), 29 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index cfba93b..f48e915 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1593,6 +1593,9 @@ static int preallocate(BlockDriverState *bs)
>      return 0;
>  }
>  
> +static uint64_t minimal_blob_size(uint64_t ts, int cb, int spcb,
> +                                  uint64_t overhead);
> +
>  static int qcow2_create2(const char *filename, int64_t total_size,
>                           const char *backing_file, const char *backing_format,
>                           int flags, size_t cluster_size, PreallocMode prealloc,
> @@ -1628,6 +1631,29 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>      Error *local_err = NULL;
>      int ret;
>  
> +    if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
> +        int64_t meta_size = 0;
> +        uint64_t nl2e;
> +
> +        total_size = align_offset(total_size, cluster_size);

I don't think it's a good idea to let the virtual disk size depend on
whether preallocation is enabled or not. You should always get the same
rounding (which is rounding up to the next sector boundary).

Do you need full clusters for your calculations below or what is this
good for? If so, please use a local variable and leave the value used
for the bdrv_truncate() call unmodified.

> +        /* total size of L2 tables */
> +        nl2e = total_size >> cluster_bits;
> +        nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
> +        uint64_t l2_clusters = nl2e * sizeof(uint64_t) >> cluster_bits;
> +
> +        meta_size =
> +            (1 +
> +             minimal_blob_size(total_size >> BDRV_SECTOR_BITS,
> +                               cluster_bits, cluster_bits - BDRV_SECTOR_BITS,
> +                               1 + l2_clusters +
> +                               (total_size >> cluster_bits)) +
> +             l2_clusters) << cluster_bits;
> +
> +        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, total_size + meta_size);
> +        qemu_opt_set(opts, BLOCK_OPT_PREALLOC, PreallocMode_lookup[prealloc]);
> +    }
> +
>      ret = bdrv_create_file(filename, opts, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);

Kevin