Re: [Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting

[Maxim Levitsky <mlevitsk@redhat.com>]

On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> There is no reason why the format drivers need to truncate the protocol
> node when formatting it.  When using the old .bdrv_co_create_ops()
> interface, the file will be created with no size option anyway, which
> generally gives it a size of 0.  (Exceptions are block devices, which
> cannot be truncated anyway.)
> 
> When using blockdev-create, the user must have given the file node some
> size anyway, so there is no reason why we should override that.
> 
> qed is an exception, it needs the file to start completely empty (as
> explained by c743849bee7333c7ef256b7e12e34ed6f907064f).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/parallels.c | 5 -----
>  block/qcow.c      | 5 -----
>  block/qcow2.c     | 6 ------
>  3 files changed, 16 deletions(-)
> 
> diff --git a/block/parallels.c b/block/parallels.c
> index 7cd2714b69..905cac35c6 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -563,11 +563,6 @@ static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
>      blk_set_allow_write_beyond_eof(blk, true);
>  
>      /* Create image format */
> -    ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
> -    if (ret < 0) {
> -        goto out;
> -    }
> -
>      bat_entries = DIV_ROUND_UP(total_size, cl_size);
>      bat_sectors = DIV_ROUND_UP(bat_entry_off(bat_entries), cl_size);
>      bat_sectors = (bat_sectors *  cl_size) >> BDRV_SECTOR_BITS;
> diff --git a/block/qcow.c b/block/qcow.c
> index 5bdf72ba33..17705015ca 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -858,11 +858,6 @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
>      blk_set_allow_write_beyond_eof(qcow_blk, true);
>  
>      /* Create image format */
> -    ret = blk_truncate(qcow_blk, 0, PREALLOC_MODE_OFF, errp);
> -    if (ret < 0) {
> -        goto exit;
> -    }
> -
>      memset(&header, 0, sizeof(header));
>      header.magic = cpu_to_be32(QCOW_MAGIC);
>      header.version = cpu_to_be32(QCOW_VERSION);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 4d16393e61..4978ccc7be 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3186,12 +3186,6 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>      }
>      blk_set_allow_write_beyond_eof(blk, true);
>  
> -    /* Clear the protocol layer and preallocate it if necessary */
> -    ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
> -    if (ret < 0) {
> -        goto out;
> -    }
> -
>      /* Write the header */
>      QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
>      header = g_malloc0(cluster_size);

As long as bdrv_co_create_ops still creates the underlying file with bdrv_create_file or so,
I also don't see a reason to truncate it to 0 afterward.
Especially for block devices...
So,

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky