Re: [PATCH 2/3] block/vmdk: skip RGD for stream optimized disks

[Kevin Wolf <kwolf@redhat.com>]

Am 05.10.2026 um 15:33 hat Sanne Raymaekers geschrieben:
> Stream optimized disks the spec notes: "The rgdOffset should be ignored
> because bit 1 of the flags field is not set." Where bit 1 of the flags
> field indicates a redundant grain directory (RGD).[^1]
> 
> In short, stream optimized disks should have the RGD offset set to 0,
> and only write the main grain directory.
> 
> [^1] Virtual Disk Format 5.0 Technical Note.
> 
> Signed-off-by: Sanne Raymaekers <sraymaek@redhat.com>
> ---
>  block/vmdk.c | 76 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 46 insertions(+), 30 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 3cdca06fff..040f38cd1d 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -2218,7 +2218,7 @@ vmdk_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
>  
>  static int coroutine_fn GRAPH_UNLOCKED
>  vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
> -                 bool zeroed_grain, Error **errp)
> +                 bool zeroed_grain, bool stream_optimized, Error **errp)

You changed quite a few functions to add a new stream_optimized
parameter. This seems unnecessary as there is already a compress
parameter, which originates in vmdk_co_do_create() like this:

    compress = subformat == BLOCKDEV_VMDK_SUBFORMAT_STREAMOPTIMIZED;

So stream_optimized duplicates the same information.

>  {
>      int ret, i;
>      VMDK4Header header;
> @@ -2239,9 +2239,10 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
>      } else {
>          header.version = 1;
>      }
> -    header.flags = VMDK4_FLAG_RGD | VMDK4_FLAG_NL_DETECT
> +    header.flags =  VMDK4_FLAG_NL_DETECT

Minor formatting issue: There are two spaces before VMDK4_FLAG_NL_DETECT.

>                     | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0)
> -                   | (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0);
> +                   | (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0)
> +                   | (stream_optimized ? 0 : VMDK4_FLAG_RGD);
>      header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0;
>      header.capacity = filesize / BDRV_SECTOR_SIZE;
>      header.granularity = 128;
> @@ -2255,8 +2256,15 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
>  
>      header.desc_offset = 1;
>      header.desc_size = 20;
> -    header.rgd_offset = header.desc_offset + header.desc_size;
> -    header.gd_offset = header.rgd_offset + gd_sectors + (gt_size * gt_count);
> +
> +    if (stream_optimized) {
> +        header.rgd_offset = 0;
> +        header.gd_offset = header.desc_offset + header.desc_size;
> +    } else {
> +        header.rgd_offset = header.desc_offset + header.desc_size;
> +        header.gd_offset = header.rgd_offset + gd_sectors + (gt_size * gt_count);
> +    }
> +
>      header.grain_offset =
>          ROUND_UP(header.gd_offset + gd_sectors + (gt_size * gt_count),
>                   header.granularity);
> @@ -2296,21 +2304,24 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
>          goto exit;
>      }
>  
> -    /* write grain directory */
> +    /* write backup grain directory */

Heh, good point, the existing code had them reversed in the comments.

It would be good to mention this in the commit message because it came
as a surprise that you seemingly swapped them.

>      gd_buf_size = gd_sectors * BDRV_SECTOR_SIZE;
>      gd_buf = g_malloc0(gd_buf_size);
> -    for (i = 0, tmp = le64_to_cpu(header.rgd_offset) + gd_sectors;
> -         i < gt_count; i++, tmp += gt_size) {
> -        gd_buf[i] = cpu_to_le32(tmp);
> -    }
> -    ret = blk_co_pwrite(blk, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE,
> -                        gd_buf_size, gd_buf, 0);
> -    if (ret < 0) {
> -        error_setg_errno(errp, -ret, "failed to write VMDK grain directory");
> -        goto exit;
> +
> +    if (!stream_optimized) {
> +        for (i = 0, tmp = le64_to_cpu(header.rgd_offset) + gd_sectors;
> +             i < gt_count; i++, tmp += gt_size) {
> +            gd_buf[i] = cpu_to_le32(tmp);
> +        }
> +        ret = blk_co_pwrite(blk, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE,
> +                            gd_buf_size, gd_buf, 0);
> +        if (ret < 0) {
> +            error_setg_errno(errp, -ret, "failed to write VMDK grain directory");

This should probably be "failed to write VMDK backup grain directory"
(you removed the "backup" below, but didn't add it here).

> +            goto exit;
> +        }
>      }
>  
> -    /* write backup grain directory */
> +    /* write grain directory */
>      for (i = 0, tmp = le64_to_cpu(header.gd_offset) + gd_sectors;
>           i < gt_count; i++, tmp += gt_size) {
>          gd_buf[i] = cpu_to_le32(tmp);
> @@ -2319,7 +2330,7 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
>                          gd_buf_size, gd_buf, 0);
>      if (ret < 0) {
>          error_setg_errno(errp, -ret,
> -                         "failed to write VMDK backup grain directory");
> +                         "failed to write VMDK grain directory");
>      }
>  
>      ret = 0;

Kevin