Re: [Qemu-devel] [PATCH v2] qemu-img: let 'qemu-img convert' flush data

[Paolo Bonzini]

Il 11/04/2012 16:46, Liu Yuan ha scritto:
> From: Liu Yuan <address@hidden>
> 
> The 'qemu-img convert -h' advertise that the default cache mode is
> 'writeback', while in fact it is 'unsafe'.
> 
> This patch 1) fix the help manual and 2) let bdrv_close() call bdrv_flush()
> 3) explicitly calls bdrv_close() to flush the dirty bits.
> 
> 3) is needed because some backend storage doesn't have a self-flush
> mechanism(for e.g., sheepdog), so we need to call bdrv_close() to make
> sure the image is really writen to the storage instead of hanging around
> writeback cache forever.
> 
> Signed-off-by: Liu Yuan <address@hidden>
> ---
>  block.c    |    1 +
>  qemu-img.c |    7 +++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index c0c90f0..1ee2bf0 100644
> --- a/block.c
> +++ b/block.c
> @@ -812,6 +812,7 @@ unlink_and_fail:
>  
>  void bdrv_close(BlockDriverState *bs)
>  {
> +    bdrv_flush(bs);
>      if (bs->drv) {
>          if (bs->job) {
>              block_job_cancel_sync(bs->job);
> diff --git a/qemu-img.c b/qemu-img.c
> index 6a61ca8..846f707 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -66,8 +66,8 @@ static void help(void)
>             "  'filename' is a disk image filename\n"
>             "  'fmt' is the disk image format. It is guessed automatically in 
> most cases\n"
>             "  'cache' is the cache mode used to write the output disk image, 
> the valid\n"
> -           "    options are: 'none', 'writeback' (default), 'writethrough', 
> 'directsync'\n"
> -           "    and 'unsafe'\n"
> +           "    options are: 'none', 'writeback', 'writethrough', 
> 'directsync'\n"
> +           "    and 'unsafe' (default)\n"
>             "  'size' is the disk image size in bytes. Optional suffixes\n"
>             "    'k' or 'K' (kilobyte, 1024), 'M' (megabyte, 1024k), 'G' 
> (gigabyte, 1024M)\n"
>             "    and T (terabyte, 1024G) are supported. 'b' is ignored.\n"
> @@ -1036,6 +1036,9 @@ out:
>      free_option_parameters(param);
>      qemu_vfree(buf);
>      if (out_bs) {
> +        if (ret == 0) {
> +            bdrv_close(out_bs);
> +     }

bdrv_delete already does this.

Paolo

>          bdrv_delete(out_bs);
>      }
>      if (bs) {