Re: [Qemu-devel] [PATCH v4] block/vdi: Use bdrv_flush after metadata updates

[Max Reitz <mreitz@redhat.com>]

On 07.05.2015 17:16, Zhe Qiu wrote:
> In reference to b0ad5a45...078a458e, metadata writes to
> qcow2/cow/qcow/vpc/vmdk are all synced prior to succeeding writes.
>
> Only when write is successful that bdrv_flush is called.
>
> Signed-off-by: Zhe Qiu <phoeagon@gmail.com>
> ---
>   block/vdi.c | 3 +++
>   1 file changed, 3 insertions(+)

I missed Kevin's arguments before, but I think that adding this is more 
correct than not having it; and when thinking about speed, this is vdi, 
a format supported for compatibility. Writing isn't our most important 
concern anyway (especially considering that it's even to disable write 
support for vdi at compile time).

So if we wanted to optimize it, we'd probably have to cache multiple 
allocations, do them at once and then flush afterwards (like the 
metadata cache we have in qcow2?), but that is complicated (like the 
metadata cache in qcow2), certainly too complicated for a format 
supported for compatibility (unless someone really wants to implement it).

Reviewed-by: Max Reitz <mreitz@redhat.com>

>
> diff --git a/block/vdi.c b/block/vdi.c
> index 7642ef3..dfe8ade 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -713,6 +713,9 @@ static int vdi_co_write(BlockDriverState *bs,
>           logout("will write %u block map sectors starting from entry %u\n",
>                  n_sectors, bmap_first);
>           ret = bdrv_write(bs->file, offset, base, n_sectors);
> +        if (ret >= 0) {
> +            ret = bdrv_flush(bs->file);
> +        }
>       }
>   
>       return ret;