Re: [Qemu-devel] [PATCH v7 11/19] block: vhdx write support

[Stefan Hajnoczi]

On Fri, Oct 11, 2013 at 02:17:31PM -0400, Jeff Cody wrote:
> +            /* check the payload block state */
> +            switch (s->bat[sinfo.bat_idx] & VHDX_BAT_STATE_BIT_MASK) {
> +            case PAYLOAD_BLOCK_ZERO:
> +                /* in this case, we need to preserve zero writes for
> +                 * data that is not part of this write, so we must pad
> +                 * the rest of the buffer to zeroes */
> +
> +                /* if we are on a posix system with ftruncate() that extends
> +                 * a file, then it is zero-filled for us.  On Win32, the raw
> +                 * layer uses SetFilePointer and SetFileEnd, which does not
> +                 * zero fill AFAIK */
> +
> +                /* Queue another write of zero buffers if the underlying file
> +                 * does not zero-fill on file extension */
> +
> +                if (bdrv_has_zero_init(bs->file) == 0) {

Maybe this should be done differently once Peter Lieven's thin
provisioning patches are merged.  I think bdrv_has_zero_init() only
tells you whether the file is zeroed after creation.  It doesn't
guarantee zeroes for other operations like growing the file.

I'm not 100% sure right now, will follow-up on this point later when
I've reviewed Peter's series and thought more about it.

> diff --git a/block/vhdx.h b/block/vhdx.h
> index 6abbf50..f331548 100644
> --- a/block/vhdx.h
> +++ b/block/vhdx.h
> @@ -217,7 +217,7 @@ typedef struct QEMU_PACKED VHDXLogDataSector {
>  #define PAYLOAD_BLOCK_UNDEFINED         1
>  #define PAYLOAD_BLOCK_ZERO              2
>  #define PAYLOAD_BLOCK_UNMAPPED          5
> -#define PAYLOAD_BLOCK_FULL_PRESENT      6
> +#define PAYLOAD_BLOCK_FULLY_PRESENT     6
>  #define PAYLOAD_BLOCK_PARTIALLY_PRESENT 7

This change should be in a separate patch.