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

2012-04-13 Thread Kevin Wolf
Am 13.04.2012 04:18, schrieb Liu Yuan:
 On 04/11/2012 11:21 PM, Liu Yuan wrote:
 
 On 04/11/2012 11:01 PM, Paolo Bonzini wrote:

 bdrv_delete already does this.

 Paolo


 Ah yes. Thanks pointing out.

 
 
 Hi Kevin,
If no further comment, would you pick up following patch?

Can you resend as a proper top-level v3 patch with git send-email? The
patch in this thread is line-wrapped.

Kevin



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

2012-04-12 Thread Liu Yuan
On 04/11/2012 11:21 PM, Liu Yuan wrote:

 On 04/11/2012 11:01 PM, Paolo Bonzini wrote:
 
 bdrv_delete already does this.

 Paolo
 
 
 Ah yes. Thanks pointing out.
 


Hi Kevin,
   If no further comment, would you pick up following patch?

Yuan

 From 459414f677f6449482f9cfcb7917ff0e242ad490 Mon Sep 17 00:00:00 2001
 From: Liu Yuan tailai...@taobao.com
 Date: Wed, 11 Apr 2012 23:19:54 +0800
 Subject: [PATCH v3] qemu-img: let 'qemu-img convert' flush data
 
 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()
 
 2) is needed because some backend storage doesn't have a self-flush
 mechanism(for e.g., sheepdog), so we need to call bdrv_flush() to make
 sure the image is really writen to the storage instead of hanging around
 writeback cache forever.
 
 Signed-off-by: Liu Yuan tailai...@taobao.com
 ---
  block.c|1 +
  qemu-img.c |4 ++--
  2 files changed, 3 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..6e54db3 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





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

2012-04-11 Thread Liu Yuan
From: Liu Yuan tailai...@taobao.com

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 tailai...@taobao.com
---
 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(out_bs);
 }
 if (bs) {
-- 
1.7.8.2




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

2012-04-11 Thread Paolo Bonzini
Il 11/04/2012 16:46, Liu Yuan ha scritto:
 From: Liu Yuan tailai...@taobao.com
 
 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 tailai...@taobao.com
 ---
  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) {





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

2012-04-11 Thread Liu Yuan
On 04/11/2012 11:01 PM, Paolo Bonzini wrote:

 bdrv_delete already does this.
 
 Paolo


Ah yes. Thanks pointing out.

From 459414f677f6449482f9cfcb7917ff0e242ad490 Mon Sep 17 00:00:00 2001
From: Liu Yuan tailai...@taobao.com
Date: Wed, 11 Apr 2012 23:19:54 +0800
Subject: [PATCH v3] qemu-img: let 'qemu-img convert' flush data

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()

2) is needed because some backend storage doesn't have a self-flush
mechanism(for e.g., sheepdog), so we need to call bdrv_flush() to make
sure the image is really writen to the storage instead of hanging around
writeback cache forever.

Signed-off-by: Liu Yuan tailai...@taobao.com
---
 block.c|1 +
 qemu-img.c |4 ++--
 2 files changed, 3 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..6e54db3 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
-- 
1.7.8.2