Re: [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly

[Max Reitz]

On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  qemu-img.c | 79 
> ++
>  1 file changed, 48 insertions(+), 31 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 839e05b..96b51d4 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c

[...]

> @@ -2862,26 +2874,30 @@ static int img_rebase(int argc, char **argv)
>  qemu_progress_print(0, 100);
>  
>  flags = BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
> -ret = bdrv_parse_cache_flags(cache, );
> +ret = bdrv_parse_cache_mode(cache, , );
>  if (ret < 0) {
>  error_report("Invalid cache option: %s", cache);
>  goto out;
>  }
>  
> -src_flags = BDRV_O_CACHE_WB;
> -ret = bdrv_parse_cache_flags(src_cache, _flags);
> +src_flags = 0;
> +ret = bdrv_parse_cache_mode(src_cache, _flags, _writethrough);
>  if (ret < 0) {
>  error_report("Invalid source cache option: %s", src_cache);
>  goto out;
>  }
>  
> +/* The source files are opened read-only, don't care about WCE */
> +assert((src_writethrough & BDRV_O_RDWR) == 0);

Well, yeah, that is a trivial assertion to make because BDRV_O_RDWR is 2.

I guess you meant s/src_writethrough/src_flags/.

With that fixed:

Reviewed-by: Max Reitz 

> +(void) src_writethrough;
> +
>  /*
>   * Open the images.
>   *
>   * Ignore the old backing file for unsafe rebase in case we want to 
> correct
>   * the reference to a renamed or moved backing file.
>   */
> -blk = img_open(image_opts, filename, fmt, flags, true, quiet);
> +blk = img_open(image_opts, filename, fmt, flags, true, writethrough, 
> quiet);
>  if (!blk) {
>  ret = -1;
>  goto out;



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly

[Kevin Wolf]

Signed-off-by: Kevin Wolf 
---
 qemu-img.c | 79 ++
 1 file changed, 48 insertions(+), 31 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 839e05b..96b51d4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -245,7 +245,7 @@ static int img_open_password(BlockBackend *blk, const char 
*filename,
 
 
 static BlockBackend *img_open_opts(const char *optstr,
-   QemuOpts *opts, int flags,
+   QemuOpts *opts, int flags, bool 
writethrough,
bool require_io, bool quiet)
 {
 QDict *options;
@@ -257,6 +257,7 @@ static BlockBackend *img_open_opts(const char *optstr,
 error_reportf_err(local_err, "Could not open '%s'", optstr);
 return NULL;
 }
+blk_set_enable_write_cache(blk, !writethrough);
 
 if (img_open_password(blk, optstr, require_io, quiet) < 0) {
 blk_unref(blk);
@@ -267,7 +268,8 @@ static BlockBackend *img_open_opts(const char *optstr,
 
 static BlockBackend *img_open_file(const char *filename,
const char *fmt, int flags,
-   bool require_io, bool quiet)
+   bool writethrough, bool require_io,
+   bool quiet)
 {
 BlockBackend *blk;
 Error *local_err = NULL;
@@ -283,6 +285,7 @@ static BlockBackend *img_open_file(const char *filename,
 error_reportf_err(local_err, "Could not open '%s': ", filename);
 return NULL;
 }
+blk_set_enable_write_cache(blk, !writethrough);
 
 if (img_open_password(blk, filename, require_io, quiet) < 0) {
 blk_unref(blk);
@@ -294,7 +297,7 @@ static BlockBackend *img_open_file(const char *filename,
 
 static BlockBackend *img_open(bool image_opts,
   const char *filename,
-  const char *fmt, int flags,
+  const char *fmt, int flags, bool writethrough,
   bool require_io, bool quiet)
 {
 BlockBackend *blk;
@@ -309,9 +312,9 @@ static BlockBackend *img_open(bool image_opts,
 if (!opts) {
 return NULL;
 }
-blk = img_open_opts(filename, opts, flags, true, quiet);
+blk = img_open_opts(filename, opts, flags, writethrough, true, quiet);
 } else {
-blk = img_open_file(filename, fmt, flags, true, quiet);
+blk = img_open_file(filename, fmt, flags, writethrough, true, quiet);
 }
 return blk;
 }
@@ -589,7 +592,8 @@ static int img_check(int argc, char **argv)
 BlockBackend *blk;
 BlockDriverState *bs;
 int fix = 0;
-int flags = BDRV_O_CACHE_WB | BDRV_O_CHECK;
+int flags = BDRV_O_CHECK;
+bool writethrough;
 ImageCheck *check;
 bool quiet = false;
 Error *local_err = NULL;
@@ -598,6 +602,7 @@ static int img_check(int argc, char **argv)
 fmt = NULL;
 output = NULL;
 cache = BDRV_DEFAULT_CACHE;
+
 for(;;) {
 int option_index = 0;
 static const struct option long_options[] = {
@@ -677,13 +682,13 @@ static int img_check(int argc, char **argv)
 return 1;
 }
 
-ret = bdrv_parse_cache_flags(cache, );
+ret = bdrv_parse_cache_mode(cache, , );
 if (ret < 0) {
 error_report("Invalid source cache option: %s", cache);
 return 1;
 }
 
-blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+blk = img_open(image_opts, filename, fmt, flags, writethrough, true, 
quiet);
 if (!blk) {
 return 1;
 }
@@ -793,6 +798,7 @@ static int img_commit(int argc, char **argv)
 BlockBackend *blk;
 BlockDriverState *bs, *base_bs;
 bool progress = false, quiet = false, drop = false;
+bool writethrough;
 Error *local_err = NULL;
 CommonBlockJobCBInfo cbi;
 bool image_opts = false;
@@ -869,13 +875,13 @@ static int img_commit(int argc, char **argv)
 }
 
 flags = BDRV_O_RDWR | BDRV_O_UNMAP;
-ret = bdrv_parse_cache_flags(cache, );
+ret = bdrv_parse_cache_mode(cache, , );
 if (ret < 0) {
 error_report("Invalid cache option: %s", cache);
 return 1;
 }
 
-blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+blk = img_open(image_opts, filename, fmt, flags, writethrough, true, 
quiet);
 if (!blk) {
 return 1;
 }
@@ -1119,6 +1125,7 @@ static int img_compare(int argc, char **argv)
 int ret = 0; /* return value - 0 Ident, 1 Different, >1 Error */
 bool progress = false, quiet = false, strict = false;
 int flags;
+bool writethrough;
 int64_t total_sectors;
 int64_t sector_num = 0;
 int64_t nb_sectors;
@@ -1201,21 +1208,23 @@ static int img_compare(int argc, char **argv)
 /* Initialize before goto out */
 qemu_progress_init(progress, 2.0);
 
-flags = BDRV_O_CACHE_WB;
-ret =