From abb06c5ac1c86e747bbe08bf7b5b69723ad69832 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Mon, 21 Mar 2016 14:11:42 +0000
Subject: block: add flag to indicate that no I/O will be performed

When opening an image it is useful to know whether the caller
intends to perform I/O on the image or not. In the case of
encrypted images this will allow the block driver to avoid
having to prompt for decryption keys when we merely want to
query header metadata about the image. eg qemu-img info

This flag is enforced at the top level only, since even if
we don't want todo I/O on the 'qcow2' file payload, the
underlying 'file' driver will still need todo I/O to read
the qcow2 header, for example.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

(limited to 'qemu-img.c')

diff --git a/qemu-img.c b/qemu-img.c
index bd93d0a774..9e3ac9c1a8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -225,13 +225,13 @@ static int print_block_option_help(const char *filename, const char *fmt)
 
 
 static int img_open_password(BlockBackend *blk, const char *filename,
-                             bool require_io, bool quiet)
+                             int flags, bool quiet)
 {
     BlockDriverState *bs;
     char password[256];
 
     bs = blk_bs(blk);
-    if (bdrv_is_encrypted(bs) && require_io) {
+    if (bdrv_is_encrypted(bs) && !(flags & BDRV_O_NO_IO)) {
         qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
         if (qemu_read_password(password, sizeof(password)) < 0) {
             error_report("No password given");
@@ -248,7 +248,7 @@ static int img_open_password(BlockBackend *blk, const char *filename,
 
 static BlockBackend *img_open_opts(const char *optstr,
                                    QemuOpts *opts, int flags,
-                                   bool require_io, bool quiet)
+                                   bool quiet)
 {
     QDict *options;
     Error *local_err = NULL;
@@ -260,7 +260,7 @@ static BlockBackend *img_open_opts(const char *optstr,
         return NULL;
     }
 
-    if (img_open_password(blk, optstr, require_io, quiet) < 0) {
+    if (img_open_password(blk, optstr, flags, quiet) < 0) {
         blk_unref(blk);
         return NULL;
     }
@@ -269,7 +269,7 @@ 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 quiet)
 {
     BlockBackend *blk;
     Error *local_err = NULL;
@@ -286,7 +286,7 @@ static BlockBackend *img_open_file(const char *filename,
         return NULL;
     }
 
-    if (img_open_password(blk, filename, require_io, quiet) < 0) {
+    if (img_open_password(blk, filename, flags, quiet) < 0) {
         blk_unref(blk);
         return NULL;
     }
@@ -297,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,
-                              bool require_io, bool quiet)
+                              bool quiet)
 {
     BlockBackend *blk;
     if (image_opts) {
@@ -311,9 +311,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, quiet);
     } else {
-        blk = img_open_file(filename, fmt, flags, true, quiet);
+        blk = img_open_file(filename, fmt, flags, quiet);
     }
     return blk;
 }
@@ -685,7 +685,7 @@ static int img_check(int argc, char **argv)
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+    blk = img_open(image_opts, filename, fmt, flags, quiet);
     if (!blk) {
         return 1;
     }
@@ -877,7 +877,7 @@ static int img_commit(int argc, char **argv)
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+    blk = img_open(image_opts, filename, fmt, flags, quiet);
     if (!blk) {
         return 1;
     }
@@ -1211,13 +1211,13 @@ static int img_compare(int argc, char **argv)
         goto out3;
     }
 
-    blk1 = img_open(image_opts, filename1, fmt1, flags, true, quiet);
+    blk1 = img_open(image_opts, filename1, fmt1, flags, quiet);
     if (!blk1) {
         ret = 2;
         goto out3;
     }
 
-    blk2 = img_open(image_opts, filename2, fmt2, flags, true, quiet);
+    blk2 = img_open(image_opts, filename2, fmt2, flags, quiet);
     if (!blk2) {
         ret = 2;
         goto out2;
@@ -1899,7 +1899,7 @@ static int img_convert(int argc, char **argv)
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
         blk[bs_i] = img_open(image_opts, argv[optind + bs_i],
-                             fmt, src_flags, true, quiet);
+                             fmt, src_flags, quiet);
         if (!blk[bs_i]) {
             ret = -1;
             goto out;
@@ -2044,7 +2044,7 @@ static int img_convert(int argc, char **argv)
      * the bdrv_create() call which takes different params.
      * Not critical right now, so fix can wait...
      */
-    out_blk = img_open_file(out_filename, out_fmt, flags, true, quiet);
+    out_blk = img_open_file(out_filename, out_fmt, flags, quiet);
     if (!out_blk) {
         ret = -1;
         goto out;
@@ -2236,8 +2236,8 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
         g_hash_table_insert(filenames, (gpointer)filename, NULL);
 
         blk = img_open(image_opts, filename, fmt,
-                       BDRV_O_FLAGS | BDRV_O_NO_BACKING,
-                       false, false);
+                       BDRV_O_FLAGS | BDRV_O_NO_BACKING | BDRV_O_NO_IO,
+                       false);
         if (!blk) {
             goto err;
         }
@@ -2567,7 +2567,7 @@ static int img_map(int argc, char **argv)
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, BDRV_O_FLAGS, true, false);
+    blk = img_open(image_opts, filename, fmt, BDRV_O_FLAGS, false);
     if (!blk) {
         return 1;
     }
@@ -2712,7 +2712,7 @@ static int img_snapshot(int argc, char **argv)
     }
 
     /* Open the image */
-    blk = img_open(image_opts, filename, NULL, bdrv_oflags, true, quiet);
+    blk = img_open(image_opts, filename, NULL, bdrv_oflags, quiet);
     if (!blk) {
         return 1;
     }
@@ -2883,7 +2883,7 @@ static int img_rebase(int argc, char **argv)
      * 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, quiet);
     if (!blk) {
         ret = -1;
         goto out;
@@ -3221,7 +3221,7 @@ static int img_resize(int argc, char **argv)
     qemu_opts_del(param);
 
     blk = img_open(image_opts, filename, fmt,
-                   BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
+                   BDRV_O_FLAGS | BDRV_O_RDWR, quiet);
     if (!blk) {
         ret = -1;
         goto out;
@@ -3380,7 +3380,7 @@ static int img_amend(int argc, char **argv)
         goto out;
     }
 
-    blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+    blk = img_open(image_opts, filename, fmt, flags, quiet);
     if (!blk) {
         ret = -1;
         goto out;
-- 
cgit 1.4.1


From 4ef130fca87b7a8c77e1af9ca967f28b683811d7 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Mon, 21 Mar 2016 14:11:43 +0000
Subject: qemu-img/qemu-io: don't prompt for passwords if not required

The qemu-img/qemu-io tools prompt for disk encryption passwords
regardless of whether any are actually required. Adding a check
on bdrv_key_required() avoids this prompt for disk formats which
have been converted to the QCryptoSecret APIs.

This is just a temporary hack to ensure the block I/O tests
continue to work after each patch, since the last patch will
completely delete all the password prompting code.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 3 ++-
 qemu-io.c  | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

(limited to 'qemu-img.c')

diff --git a/qemu-img.c b/qemu-img.c
index 9e3ac9c1a8..c57898ee51 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -231,7 +231,8 @@ static int img_open_password(BlockBackend *blk, const char *filename,
     char password[256];
 
     bs = blk_bs(blk);
-    if (bdrv_is_encrypted(bs) && !(flags & BDRV_O_NO_IO)) {
+    if (bdrv_is_encrypted(bs) && bdrv_key_required(bs) &&
+        !(flags & BDRV_O_NO_IO)) {
         qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
         if (qemu_read_password(password, sizeof(password)) < 0) {
             error_report("No password given");
diff --git a/qemu-io.c b/qemu-io.c
index bc129536e4..c08b495a05 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -70,7 +70,7 @@ static int openfile(char *name, int flags, QDict *opts)
     }
 
     bs = blk_bs(qemuio_blk);
-    if (bdrv_is_encrypted(bs)) {
+    if (bdrv_is_encrypted(bs) && bdrv_key_required(bs)) {
         char password[256];
         printf("Disk image '%s' is encrypted.\n", name);
         if (qemu_read_password(password, sizeof(password)) < 0) {
-- 
cgit 1.4.1


From e699614341cd9ed5676fe24c4f73f20497c28059 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Tue, 15 Mar 2016 13:03:11 +0100
Subject: qemu-img: Expand all BDRV_O_FLAGS uses

It always only set the BDRV_O_CACHE_WB flag, which is going to go away.
In order to make the next changes more local for better reviewability
this patches expands the macro.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

(limited to 'qemu-img.c')

diff --git a/qemu-img.c b/qemu-img.c
index c57898ee51..7ef90af88c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -59,8 +59,7 @@ typedef enum OutputFormat {
     OFORMAT_HUMAN,
 } OutputFormat;
 
-/* Default to cache=writeback as data integrity is not important for qemu-tcg. */
-#define BDRV_O_FLAGS BDRV_O_CACHE_WB
+/* Default to cache=writeback as data integrity is not important for qemu-img */
 #define BDRV_DEFAULT_CACHE "writeback"
 
 static void format_print(void *opaque, const char *name)
@@ -462,7 +461,7 @@ static int img_create(int argc, char **argv)
     }
 
     bdrv_img_create(filename, fmt, base_filename, base_fmt,
-                    options, img_size, BDRV_O_FLAGS, &local_err, quiet);
+                    options, img_size, BDRV_O_CACHE_WB, &local_err, quiet);
     if (local_err) {
         error_reportf_err(local_err, "%s: ", filename);
         goto fail;
@@ -592,7 +591,7 @@ static int img_check(int argc, char **argv)
     BlockBackend *blk;
     BlockDriverState *bs;
     int fix = 0;
-    int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
+    int flags = BDRV_O_CACHE_WB | BDRV_O_CHECK;
     ImageCheck *check;
     bool quiet = false;
     Error *local_err = NULL;
@@ -1204,7 +1203,7 @@ static int img_compare(int argc, char **argv)
     /* Initialize before goto out */
     qemu_progress_init(progress, 2.0);
 
-    flags = BDRV_O_FLAGS;
+    flags = BDRV_O_CACHE_WB;
     ret = bdrv_parse_cache_flags(cache, &flags);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", cache);
@@ -1884,7 +1883,7 @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    src_flags = BDRV_O_FLAGS;
+    src_flags = BDRV_O_CACHE_WB;
     ret = bdrv_parse_cache_flags(src_cache, &src_flags);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", src_cache);
@@ -2237,7 +2236,7 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
         g_hash_table_insert(filenames, (gpointer)filename, NULL);
 
         blk = img_open(image_opts, filename, fmt,
-                       BDRV_O_FLAGS | BDRV_O_NO_BACKING | BDRV_O_NO_IO,
+                       BDRV_O_CACHE_WB | BDRV_O_NO_BACKING | BDRV_O_NO_IO,
                        false);
         if (!blk) {
             goto err;
@@ -2568,7 +2567,7 @@ static int img_map(int argc, char **argv)
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, BDRV_O_FLAGS, false);
+    blk = img_open(image_opts, filename, fmt, BDRV_O_CACHE_WB, false);
     if (!blk) {
         return 1;
     }
@@ -2632,7 +2631,7 @@ static int img_snapshot(int argc, char **argv)
     Error *err = NULL;
     bool image_opts = false;
 
-    bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
+    bdrv_oflags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
     /* Parse commandline parameters */
     for(;;) {
         static const struct option long_options[] = {
@@ -2871,7 +2870,7 @@ static int img_rebase(int argc, char **argv)
         goto out;
     }
 
-    src_flags = BDRV_O_FLAGS;
+    src_flags = BDRV_O_CACHE_WB;
     ret = bdrv_parse_cache_flags(src_cache, &src_flags);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", src_cache);
@@ -3222,7 +3221,7 @@ static int img_resize(int argc, char **argv)
     qemu_opts_del(param);
 
     blk = img_open(image_opts, filename, fmt,
-                   BDRV_O_FLAGS | BDRV_O_RDWR, quiet);
+                   BDRV_O_CACHE_WB | BDRV_O_RDWR, quiet);
     if (!blk) {
         ret = -1;
         goto out;
@@ -3374,7 +3373,7 @@ static int img_amend(int argc, char **argv)
         goto out;
     }
 
-    flags = BDRV_O_FLAGS | BDRV_O_RDWR;
+    flags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
     ret = bdrv_parse_cache_flags(cache, &flags);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
-- 
cgit 1.4.1


From ce09954720d4d59b682b45d622e62ec13c332070 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Tue, 15 Mar 2016 13:01:04 +0100
Subject: qemu-img: Call blk_set_enable_write_cache() explicitly

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 76 ++++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 44 insertions(+), 32 deletions(-)

(limited to 'qemu-img.c')

diff --git a/qemu-img.c b/qemu-img.c
index 7ef90af88c..9131416f60 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -247,7 +247,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 quiet)
 {
     QDict *options;
@@ -259,6 +259,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, flags, quiet) < 0) {
         blk_unref(blk);
@@ -269,7 +270,7 @@ static BlockBackend *img_open_opts(const char *optstr,
 
 static BlockBackend *img_open_file(const char *filename,
                                    const char *fmt, int flags,
-                                   bool quiet)
+                                   bool writethrough, bool quiet)
 {
     BlockBackend *blk;
     Error *local_err = NULL;
@@ -285,6 +286,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, flags, quiet) < 0) {
         blk_unref(blk);
@@ -296,7 +298,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 quiet)
 {
     BlockBackend *blk;
@@ -311,9 +313,9 @@ static BlockBackend *img_open(bool image_opts,
         if (!opts) {
             return NULL;
         }
-        blk = img_open_opts(filename, opts, flags, quiet);
+        blk = img_open_opts(filename, opts, flags, writethrough, quiet);
     } else {
-        blk = img_open_file(filename, fmt, flags, quiet);
+        blk = img_open_file(filename, fmt, flags, writethrough, quiet);
     }
     return blk;
 }
@@ -591,7 +593,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;
@@ -600,6 +603,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[] = {
@@ -679,13 +683,13 @@ static int img_check(int argc, char **argv)
         return 1;
     }
 
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", cache);
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, flags, quiet);
+    blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
     if (!blk) {
         return 1;
     }
@@ -795,6 +799,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;
@@ -871,13 +876,13 @@ static int img_commit(int argc, char **argv)
     }
 
     flags = BDRV_O_RDWR | BDRV_O_UNMAP;
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, flags, quiet);
+    blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
     if (!blk) {
         return 1;
     }
@@ -1121,6 +1126,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;
@@ -1203,21 +1209,21 @@ static int img_compare(int argc, char **argv)
     /* Initialize before goto out */
     qemu_progress_init(progress, 2.0);
 
-    flags = BDRV_O_CACHE_WB;
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    flags = 0;
+    ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", cache);
         ret = 2;
         goto out3;
     }
 
-    blk1 = img_open(image_opts, filename1, fmt1, flags, quiet);
+    blk1 = img_open(image_opts, filename1, fmt1, flags, writethrough, quiet);
     if (!blk1) {
         ret = 2;
         goto out3;
     }
 
-    blk2 = img_open(image_opts, filename2, fmt2, flags, quiet);
+    blk2 = img_open(image_opts, filename2, fmt2, flags, writethrough, quiet);
     if (!blk2) {
         ret = 2;
         goto out2;
@@ -1711,6 +1717,7 @@ static int img_convert(int argc, char **argv)
     int c, bs_n, bs_i, compress, cluster_sectors, skip_create;
     int64_t ret = 0;
     int progress = 0, flags, src_flags;
+    bool writethrough, src_writethrough;
     const char *fmt, *out_fmt, *cache, *src_cache, *out_baseimg, *out_filename;
     BlockDriver *drv, *proto_drv;
     BlockBackend **blk = NULL, *out_blk = NULL;
@@ -1883,8 +1890,8 @@ static int img_convert(int argc, char **argv)
         goto out;
     }
 
-    src_flags = BDRV_O_CACHE_WB;
-    ret = bdrv_parse_cache_flags(src_cache, &src_flags);
+    src_flags = 0;
+    ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_writethrough);
     if (ret < 0) {
         error_report("Invalid source cache option: %s", src_cache);
         goto out;
@@ -1899,7 +1906,7 @@ static int img_convert(int argc, char **argv)
     total_sectors = 0;
     for (bs_i = 0; bs_i < bs_n; bs_i++) {
         blk[bs_i] = img_open(image_opts, argv[optind + bs_i],
-                             fmt, src_flags, quiet);
+                             fmt, src_flags, src_writethrough, quiet);
         if (!blk[bs_i]) {
             ret = -1;
             goto out;
@@ -2033,7 +2040,7 @@ static int img_convert(int argc, char **argv)
     }
 
     flags = min_sparse ? (BDRV_O_RDWR | BDRV_O_UNMAP) : BDRV_O_RDWR;
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
         goto out;
@@ -2044,7 +2051,7 @@ static int img_convert(int argc, char **argv)
      * the bdrv_create() call which takes different params.
      * Not critical right now, so fix can wait...
      */
-    out_blk = img_open_file(out_filename, out_fmt, flags, quiet);
+    out_blk = img_open_file(out_filename, out_fmt, flags, writethrough, quiet);
     if (!out_blk) {
         ret = -1;
         goto out;
@@ -2236,8 +2243,7 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
         g_hash_table_insert(filenames, (gpointer)filename, NULL);
 
         blk = img_open(image_opts, filename, fmt,
-                       BDRV_O_CACHE_WB | BDRV_O_NO_BACKING | BDRV_O_NO_IO,
-                       false);
+                       BDRV_O_NO_BACKING | BDRV_O_NO_IO, false, false);
         if (!blk) {
             goto err;
         }
@@ -2567,7 +2573,7 @@ static int img_map(int argc, char **argv)
         return 1;
     }
 
-    blk = img_open(image_opts, filename, fmt, BDRV_O_CACHE_WB, false);
+    blk = img_open(image_opts, filename, fmt, 0, false, false);
     if (!blk) {
         return 1;
     }
@@ -2631,7 +2637,7 @@ static int img_snapshot(int argc, char **argv)
     Error *err = NULL;
     bool image_opts = false;
 
-    bdrv_oflags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
+    bdrv_oflags = BDRV_O_RDWR;
     /* Parse commandline parameters */
     for(;;) {
         static const struct option long_options[] = {
@@ -2712,7 +2718,7 @@ static int img_snapshot(int argc, char **argv)
     }
 
     /* Open the image */
-    blk = img_open(image_opts, filename, NULL, bdrv_oflags, quiet);
+    blk = img_open(image_opts, filename, NULL, bdrv_oflags, false, quiet);
     if (!blk) {
         return 1;
     }
@@ -2774,6 +2780,7 @@ static int img_rebase(int argc, char **argv)
     char *filename;
     const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
     int c, flags, src_flags, ret;
+    bool writethrough, src_writethrough;
     int unsafe = 0;
     int progress = 0;
     bool quiet = false;
@@ -2864,26 +2871,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, &flags);
+    ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     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, &src_flags);
+    src_flags = 0;
+    ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_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_flags & BDRV_O_RDWR) == 0);
+    (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, quiet);
+    blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
     if (!blk) {
         ret = -1;
         goto out;
@@ -3221,7 +3232,7 @@ static int img_resize(int argc, char **argv)
     qemu_opts_del(param);
 
     blk = img_open(image_opts, filename, fmt,
-                   BDRV_O_CACHE_WB | BDRV_O_RDWR, quiet);
+                   BDRV_O_RDWR, false, quiet);
     if (!blk) {
         ret = -1;
         goto out;
@@ -3277,6 +3288,7 @@ static int img_amend(int argc, char **argv)
     QemuOpts *opts = NULL;
     const char *fmt = NULL, *filename, *cache;
     int flags;
+    bool writethrough;
     bool quiet = false, progress = false;
     BlockBackend *blk = NULL;
     BlockDriverState *bs = NULL;
@@ -3373,14 +3385,14 @@ static int img_amend(int argc, char **argv)
         goto out;
     }
 
-    flags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
-    ret = bdrv_parse_cache_flags(cache, &flags);
+    flags = BDRV_O_RDWR;
+    ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
     if (ret < 0) {
         error_report("Invalid cache option: %s", cache);
         goto out;
     }
 
-    blk = img_open(image_opts, filename, fmt, flags, quiet);
+    blk = img_open(image_opts, filename, fmt, flags, writethrough, quiet);
     if (!blk) {
         ret = -1;
         goto out;
-- 
cgit 1.4.1


From 61de4c680846167e01d7ba42bf787f8d1d80bf5e Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Fri, 18 Mar 2016 17:46:45 +0100
Subject: block: Remove BDRV_O_CACHE_WB

The previous patches have successively made blk->enable_write_cache the
true source for the information whether a writethrough mode must be
implemented. The corresponding BDRV_O_CACHE_WB is only useless baggage
we're carrying around, so now's the time to remove it.

At the same time, we remove the 'cache.writeback' option parsing on the
BDS level as the only effect was setting the BDRV_O_CACHE_WB flag.

This change requires test cases that explicitly enabled the option to
drop it. Other than that and the change of the error message when
writethrough is enabled on the BDS level (from "Can't set writethrough
mode" to "doesn't support the option"), there should be no change in
behaviour.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c                       | 48 ++-----------------------------------------
 block/block-backend.c         | 11 ----------
 block/vvfat.c                 |  3 +--
 blockdev.c                    | 21 ++-----------------
 include/block/block.h         |  3 +--
 qemu-img.c                    |  2 +-
 qemu-io-cmds.c                |  1 -
 tests/qemu-iotests/051        |  2 +-
 tests/qemu-iotests/051.pc.out | 10 ++++-----
 tests/qemu-iotests/142        |  6 +++---
 tests/qemu-iotests/142.out    | 36 ++++++++++++++++----------------
 11 files changed, 34 insertions(+), 109 deletions(-)

(limited to 'qemu-img.c')

diff --git a/block.c b/block.c
index eae597e756..7ff4fcb7f5 100644
--- a/block.c
+++ b/block.c
@@ -680,7 +680,6 @@ static void bdrv_temp_snapshot_options(int *child_flags, QDict *child_options,
     *child_flags = (parent_flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY;
 
     /* For temporary files, unconditional cache=unsafe is fine */
-    qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
     qdict_set_default_str(child_options, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(child_options, BDRV_OPT_CACHE_NO_FLUSH, "on");
 }
@@ -705,7 +704,6 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
     /* Our block drivers take care to send flushes and respect unmap policy,
      * so we can default to enable both on lower layers regardless of the
      * corresponding parent options. */
-    qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
     flags |= BDRV_O_UNMAP;
 
     /* Clear flags that only apply to the top layer */
@@ -748,7 +746,6 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
 
     /* The cache mode is inherited unmodified for backing files; except WCE,
      * which is only applied on the top level (BlockBackend) */
-    qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
     qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
 
@@ -767,7 +764,7 @@ static const BdrvChildRole child_backing = {
 
 static int bdrv_open_flags(BlockDriverState *bs, int flags)
 {
-    int open_flags = flags | BDRV_O_CACHE_WB;
+    int open_flags = flags;
 
     /*
      * Clear flags that are internal to the block layer before opening the
@@ -789,11 +786,6 @@ static void update_flags_from_options(int *flags, QemuOpts *opts)
 {
     *flags &= ~BDRV_O_CACHE_MASK;
 
-    assert(qemu_opt_find(opts, BDRV_OPT_CACHE_WB));
-    if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, false)) {
-        *flags |= BDRV_O_CACHE_WB;
-    }
-
     assert(qemu_opt_find(opts, BDRV_OPT_CACHE_NO_FLUSH));
     if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) {
         *flags |= BDRV_O_NO_FLUSH;
@@ -807,10 +799,6 @@ static void update_flags_from_options(int *flags, QemuOpts *opts)
 
 static void update_options_from_flags(QDict *options, int flags)
 {
-    if (!qdict_haskey(options, BDRV_OPT_CACHE_WB)) {
-        qdict_put(options, BDRV_OPT_CACHE_WB,
-                  qbool_from_bool(flags & BDRV_O_CACHE_WB));
-    }
     if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
         qdict_put(options, BDRV_OPT_CACHE_DIRECT,
                   qbool_from_bool(flags & BDRV_O_NOCACHE));
@@ -872,11 +860,6 @@ static QemuOptsList bdrv_runtime_opts = {
             .type = QEMU_OPT_STRING,
             .help = "Block driver to use for the node",
         },
-        {
-            .name = BDRV_OPT_CACHE_WB,
-            .type = QEMU_OPT_BOOL,
-            .help = "Enable writeback mode",
-        },
         {
             .name = BDRV_OPT_CACHE_DIRECT,
             .type = QEMU_OPT_BOOL,
@@ -984,14 +967,6 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
     /* Apply cache mode options */
     update_flags_from_options(&bs->open_flags, opts);
 
-    if (!bs->blk && (bs->open_flags & BDRV_O_CACHE_WB) == 0) {
-        error_setg(errp, "Can't set writethrough mode except for the root");
-        ret = -EINVAL;
-        goto free_and_fail;
-    }
-
-    bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB);
-
     /* Open the image, either directly or using a protocol */
     open_flags = bdrv_open_flags(bs, bs->open_flags);
     if (drv->bdrv_file_open) {
@@ -2013,16 +1988,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
 
     update_flags_from_options(&reopen_state->flags, opts);
 
-    /* WCE is a BlockBackend level option, can't change it */
-    bool old_wce = bdrv_enable_write_cache(reopen_state->bs);
-    bool new_wce = (reopen_state->flags & BDRV_O_CACHE_WB);
-
-    if (old_wce != new_wce) {
-        error_setg(errp, "Cannot change cache.writeback");
-        ret = -EINVAL;
-        goto error;
-    }
-
     /* node-name and driver must be unchanged. Put them back into the QDict, so
      * that they are checked at the end of this function. */
     value = qemu_opt_get(opts, "node-name");
@@ -2124,8 +2089,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
     reopen_state->bs->open_flags         = reopen_state->flags;
     reopen_state->bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
 
-    bdrv_set_enable_write_cache(reopen_state->bs,
-                                !!(reopen_state->flags & BDRV_O_CACHE_WB));
     bdrv_refresh_limits(reopen_state->bs, NULL);
 }
 
@@ -2746,13 +2709,6 @@ void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce)
     if (bs->blk) {
         blk_set_enable_write_cache(bs->blk, wce);
     }
-
-    /* so a reopen() will preserve wce */
-    if (wce) {
-        bs->open_flags |= BDRV_O_CACHE_WB;
-    } else {
-        bs->open_flags &= ~BDRV_O_CACHE_WB;
-    }
 }
 
 int bdrv_is_encrypted(BlockDriverState *bs)
@@ -3605,7 +3561,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
             }
 
             /* backing files always opened read-only */
-            back_flags = flags | BDRV_O_CACHE_WB;
+            back_flags = flags;
             back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
 
             if (backing_fmt) {
diff --git a/block/block-backend.c b/block/block-backend.c
index a263636a58..d74f6701b5 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -150,8 +150,6 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
     BlockBackend *blk;
     int ret;
 
-    assert((flags & BDRV_O_CACHE_WB) == 0);
-
     blk = blk_new_with_bs(errp);
     if (!blk) {
         QDECREF(options);
@@ -1224,15 +1222,6 @@ int blk_enable_write_cache(BlockBackend *blk)
 void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
 {
     blk->enable_write_cache = wce;
-
-    /* TODO Remove this when BDRV_O_CACHE_WB isn't used any more */
-    if (blk->root) {
-        if (wce) {
-            blk->root->bs->open_flags |= BDRV_O_CACHE_WB;
-        } else {
-            blk->root->bs->open_flags &= ~BDRV_O_CACHE_WB;
-        }
-    }
 }
 
 void blk_invalidate_cache(BlockBackend *blk, Error **errp)
diff --git a/block/vvfat.c b/block/vvfat.c
index eb1126cbad..6b853146f0 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2957,8 +2957,7 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)
     options = qdict_new();
     qdict_put(options, "driver", qstring_from_str("qcow"));
     ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, options,
-                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
-                    errp);
+                    BDRV_O_RDWR | BDRV_O_NO_FLUSH, errp);
     if (ret < 0) {
         goto err;
     }
diff --git a/blockdev.c b/blockdev.c
index 7a73726a10..e50e8eabd0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -595,7 +595,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
         /* bdrv_open() defaults to the values in bdrv_flags (for compatibility
          * with other callers) rather than what we want as the real defaults.
          * Apply the defaults here instead. */
-        qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, writethrough ? "off" : "on");
         qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
         qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
         assert((bdrv_flags & BDRV_O_CACHE_MASK) == 0);
@@ -691,7 +690,6 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
     /* bdrv_open() defaults to the values in bdrv_flags (for compatibility
      * with other callers) rather than what we want as the real defaults.
      * Apply the defaults here instead. */
-    qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, "on");
     qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
 
@@ -1779,12 +1777,6 @@ static void external_snapshot_prepare(BlkActionState *common,
         flags |= BDRV_O_NO_BACKING;
     }
 
-    /* There is no BB attached during bdrv_open(), so we can't set a
-     * writethrough mode. bdrv_append() will swap the WCE setting so that the
-     * backing file becomes unconditionally writeback (which is what backing
-     * files should always be) and the new overlay gets the original setting. */
-    flags |= BDRV_O_CACHE_WB;
-
     assert(state->new_bs == NULL);
     ret = bdrv_open(&state->new_bs, new_image_file, snapshot_ref, options,
                     flags, errp);
@@ -2529,7 +2521,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
     BlockBackend *blk;
     BlockDriverState *medium_bs = NULL;
     int bdrv_flags, ret;
-    bool writethrough;
     QDict *options = NULL;
     Error *err = NULL;
 
@@ -2548,12 +2539,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
     bdrv_flags &= ~(BDRV_O_TEMPORARY | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING |
         BDRV_O_PROTOCOL);
 
-    /* Must open the image in writeback mode as long as no BlockBackend is
-     * attached. The right mode can be set as the final step after changing the
-     * medium. */
-    writethrough = !(bdrv_flags & BDRV_O_CACHE_WB);
-    bdrv_flags |= BDRV_O_CACHE_WB;
-
     if (!has_read_only) {
         read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;
     }
@@ -2611,8 +2596,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
         goto fail;
     }
 
-    bdrv_set_enable_write_cache(medium_bs, !writethrough);
-
     qmp_blockdev_close_tray(device, errp);
 
 fail:
@@ -3238,7 +3221,7 @@ static void do_drive_backup(const char *device, const char *target,
         goto out;
     }
 
-    flags = bs->open_flags | BDRV_O_CACHE_WB | BDRV_O_RDWR;
+    flags = bs->open_flags | BDRV_O_RDWR;
 
     /* See if we have a backing HD we can use to create our new image
      * on top of. */
@@ -3533,7 +3516,7 @@ void qmp_drive_mirror(const char *device, const char *target,
         format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
     }
 
-    flags = bs->open_flags | BDRV_O_CACHE_WB | BDRV_O_RDWR;
+    flags = bs->open_flags | BDRV_O_RDWR;
     source = backing_bs(bs);
     if (!source && sync == MIRROR_SYNC_MODE_TOP) {
         sync = MIRROR_SYNC_MODE_FULL;
diff --git a/include/block/block.h b/include/block/block.h
index 738eeb6ffa..7bacd556b8 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -82,7 +82,6 @@ typedef struct HDGeometry {
 #define BDRV_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
 #define BDRV_O_TEMPORARY   0x0010 /* delete the file after use */
 #define BDRV_O_NOCACHE     0x0020 /* do not use the host page cache */
-#define BDRV_O_CACHE_WB    0x0040 /* use write-back caching */
 #define BDRV_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
 #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
 #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
@@ -96,7 +95,7 @@ typedef struct HDGeometry {
                                       ignoring the format layer */
 #define BDRV_O_NO_IO       0x10000 /* don't initialize for I/O */
 
-#define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
+#define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
 
 
 /* Option names of options parsed by the block layer */
diff --git a/qemu-img.c b/qemu-img.c
index 9131416f60..55f76e7b00 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -463,7 +463,7 @@ static int img_create(int argc, char **argv)
     }
 
     bdrv_img_create(filename, fmt, base_filename, base_fmt,
-                    options, img_size, BDRV_O_CACHE_WB, &local_err, quiet);
+                    options, img_size, 0, &local_err, quiet);
     if (local_err) {
         error_reportf_err(local_err, "%s: ", filename);
         goto fail;
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 932e367a17..382faa8a2a 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2151,7 +2151,6 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
     opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
     qemu_opts_reset(&reopen_opts);
 
-    flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;
     brq = bdrv_reopen_queue(NULL, bs, opts, flags);
     bdrv_reopen_multiple(brq, &local_err);
     if (local_err) {
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 7bfe9fffe1..88b3d91da8 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -218,7 +218,7 @@ run_qemu -drive driver=null-co,cache=invalid_value
 
 for cache in writeback writethrough unsafe invalid_value; do
     echo -e "info block\ninfo block file\ninfo block backing\ninfo block backing-file" | \
-    run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
+    run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
 done
 
 echo
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 73cc15adeb..ec6d22229c 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -239,7 +239,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 Testing: -drive driver=null-co,cache=invalid_value
 QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
 
-Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) iininfinfoinfo info binfo blinfo bloinfo blocinfo block
 drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -259,7 +259,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
     Cache mode:       writeback, ignore flushes
 (qemu) qququiquit
 
-Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) iininfinfoinfo info binfo blinfo bloinfo blocinfo block
 drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -279,7 +279,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
     Cache mode:       writeback, ignore flushes
 (qemu) qququiquit
 
-Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) iininfinfoinfo info binfo blinfo bloinfo blocinfo block
 drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -299,8 +299,8 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
     Cache mode:       writeback, ignore flushes
 (qemu) qququiquit
 
-Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
+Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
 
 
 === Specifying the protocol layer ===
diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
index a035747904..3828c23b7b 100755
--- a/tests/qemu-iotests/142
+++ b/tests/qemu-iotests/142
@@ -110,11 +110,11 @@ function check_cache_all()
     echo -e "\n\ncache.writeback=off on none0"
     echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
     echo -e "\ncache.writeback=off on file"
-    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.writeback=off | grep -e "doesn't" -e "does not"
     echo -e "\ncache.writeback=off on backing"
-    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.writeback=off | grep -e "doesn't" -e "does not"
     echo -e "\ncache.writeback=off on backing-file"
-    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
+    echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.writeback=off | grep -e "doesn't" -e "does not"
 
     # cache.no-flush is supposed to be inherited by both bs->file and bs->backing
 
diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out
index 3d5ef5fe8d..600beca8fb 100644
--- a/tests/qemu-iotests/142.out
+++ b/tests/qemu-iotests/142.out
@@ -71,13 +71,13 @@ cache.writeback=off on none0
     Cache mode:       writeback
 
 cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
 
 cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 
 cache.no-flush=on on none0
@@ -147,13 +147,13 @@ cache.writeback=off on none0
     Cache mode:       writeback
 
 cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
 
 cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 
 cache.no-flush=on on none0
@@ -223,13 +223,13 @@ cache.writeback=off on none0
     Cache mode:       writeback, direct
 
 cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
 
 cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 
 cache.no-flush=on on none0
@@ -299,13 +299,13 @@ cache.writeback=off on none0
     Cache mode:       writeback, direct
 
 cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
 
 cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 
 cache.no-flush=on on none0
@@ -375,13 +375,13 @@ cache.writeback=off on none0
     Cache mode:       writeback
 
 cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
 
 cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 
 cache.no-flush=on on none0
@@ -704,13 +704,13 @@ cache.writeback=off on none0
     Cache mode:       writeback, direct
 
 cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
 
 cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
 
 
 cache.no-flush=on on none0
-- 
cgit 1.4.1


From aad15de4275d2fc90acdf6101493dfee4e39b803 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Thu, 24 Mar 2016 23:33:57 +0100
Subject: qemu-img: Fix preallocation with -S 0 for convert

When passing -S 0 to qemu-img convert, the target image is supposed to
be fully allocated. Right now, this is not the case if the source image
contains areas which bdrv_get_block_status() reports as being zero.

This patch changes a zeroed area's status from BLK_ZERO to BLK_DATA
before invoking convert_write() if -S 0 has been specified. In addition,
the check whether convert_read() actually needs to do anything
(basically only if the current area is a BLK_DATA area) is pulled out of
that function to the caller.

If -S 0 has been specified, zeroed areas need to be written as data to
the output, thus they then have to be accounted when calculating the
progress made.

This patch changes the reference output for iotest 122; contrary to what
it assumed, -S 0 really should allocate everything in the output, not
just areas that are filled with zeros (as opposed to being zeroed).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c                 | 26 +++++++++++++++-----------
 tests/qemu-iotests/122.out |  6 ++----
 2 files changed, 17 insertions(+), 15 deletions(-)

(limited to 'qemu-img.c')

diff --git a/qemu-img.c b/qemu-img.c
index 55f76e7b00..06264d9128 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1514,10 +1514,6 @@ static int convert_read(ImgConvertState *s, int64_t sector_num, int nb_sectors,
     int n;
     int ret;
 
-    if (s->status == BLK_ZERO || s->status == BLK_BACKING_FILE) {
-        return 0;
-    }
-
     assert(nb_sectors <= s->buf_sectors);
     while (nb_sectors > 0) {
         BlockBackend *blk;
@@ -1655,7 +1651,8 @@ static int convert_do_copy(ImgConvertState *s)
             ret = n;
             goto fail;
         }
-        if (s->status == BLK_DATA) {
+        if (s->status == BLK_DATA || (!s->min_sparse && s->status == BLK_ZERO))
+        {
             s->allocated_sectors += n;
         }
         sector_num += n;
@@ -1675,17 +1672,24 @@ static int convert_do_copy(ImgConvertState *s)
             ret = n;
             goto fail;
         }
-        if (s->status == BLK_DATA) {
+        if (s->status == BLK_DATA || (!s->min_sparse && s->status == BLK_ZERO))
+        {
             allocated_done += n;
             qemu_progress_print(100.0 * allocated_done / s->allocated_sectors,
                                 0);
         }
 
-        ret = convert_read(s, sector_num, n, buf);
-        if (ret < 0) {
-            error_report("error while reading sector %" PRId64
-                         ": %s", sector_num, strerror(-ret));
-            goto fail;
+        if (s->status == BLK_DATA) {
+            ret = convert_read(s, sector_num, n, buf);
+            if (ret < 0) {
+                error_report("error while reading sector %" PRId64
+                             ": %s", sector_num, strerror(-ret));
+                goto fail;
+            }
+        } else if (!s->min_sparse && s->status == BLK_ZERO) {
+            n = MIN(n, s->buf_sectors);
+            memset(buf, 0, n * BDRV_SECTOR_SIZE);
+            s->status = BLK_DATA;
         }
 
         ret = convert_write(s, sector_num, n, buf);
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index 0068e96741..98814de5d6 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -112,16 +112,14 @@ read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true, "offset": 327680},
-{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data": false}]
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true, "offset": 327680}]
 
 convert -c -S 0:
 read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true},
-{ "start": 6291456, "length": 60817408, "depth": 0, "zero": true, "data": false}]
+[{ "start": 0, "length": 67108864, "depth": 0, "zero": false, "data": true}]
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
 wrote 33554432/33554432 bytes at offset 0
 32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-- 
cgit 1.4.1