Re: [Qemu-devel] [PATCH v4 1/4] block/vhdx.c: Don't blindly update the header

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1683 bytes --]

On 2017-11-07 14:10, Jeff Cody wrote:
> The VHDX specification requires that before user data modification of
> the vhdx image, the VHDX header file and data GUIDs need to be updated.
> In vhdx_open(), if the image is set to RDWR, we go ahead and update the
> header.
> 
> However, just because the image is set to RDWR does not mean we can go
> ahead and write at this point - specifically, if the QEMU run state is
> INMIGRATE, the underlying file BS may be set to inactive via the BDS
> open flag of BDRV_O_INACTIVE.  Attempting to write under this condition
> will cause an assert in bdrv_co_pwritev().
> 
> We can alternatively latch the first time the image is written.  And lo
> and behold, we do just that, via vhdx_user_visible_write() in
> vhdx_co_writev().  This means the call to vhdx_update_headers() in
> vhdx_open() is likely just vestigial, and can be removed.
> 
> Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/vhdx.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 7ae4589..9956933 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1008,13 +1008,6 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
>          goto fail;
>      }
>  
> -    if (flags & BDRV_O_RDWR) {
> -        ret = vhdx_update_headers(bs, s, false, NULL);

And this doesn't even update the data GUID...

Max

> -        if (ret < 0) {
> -            goto fail;
> -        }
> -    }
> -
>      /* TODO: differencing files */
>  
>      return 0;
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]