Re: [PATCH] vmdk:truncate more one sector in init extent

[Kevin Wolf <kwolf@redhat.com>]

Am 22.08.2024 um 12:52 hat luzhipeng geschrieben:
> issue:https://gitlab.com/qemu-project/qemu/-/issues/1357
> empty vmdk only contains metadata, ovftool failed.
> So it allocates more one sector for empty disk. the ovftool
> command line: ovftool input.ovf output.ova
> 
> Signed-off-by: luzhipeng <luzhipeng@cestc.cn>

I think this commit message needs more of the information from the bug
report, otherwise it seems unexplainable why adding an empty sector
should make a difference.

> diff --git a/block/vmdk.c b/block/vmdk.c
> index 78f6433607..283dee9b49 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -2286,7 +2286,7 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
>          goto exit;
>      }
>  
> -    ret = blk_co_truncate(blk, le64_to_cpu(header.grain_offset) << 9, false,
> +    ret = blk_co_truncate(blk, (le64_to_cpu(header.grain_offset) << 9) + BDRV_SECTOR_SIZE,
> +                          false, PREALLOC_MODE_OFF, 0, errp);
>      if (ret < 0) {
>          goto exit;

This is not a good fix. It means that we will always leave an empty
sector after the header, even if more data follows.

Does the problem really only happen with empty images? I think we don't
necessarily add an end-of-stream marker for other images either, we just
align the image size to full sectors at the end of 'qemu-img convert'.

I wonder if vmdk_co_pwritev_compressed() should be changed to write both
a footer and an explicit end-of-stream marker for streamOptimized images
in the bytes == 0 case.

Kevin