From 670df5e3b4b5ef830a7c3c970170dbfa11cbb8d2 Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Fri, 6 Sep 2013 12:18:47 +0200
Subject: [PATCH 01/33] qcow2: Pass discard type to qcow2_discard_clusters()

The function will be used internally instead of only being called for
guest discard requests.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-cluster.c | 8 ++++----
 block/qcow2.c         | 2 +-
 block/qcow2.h         | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 2d5aa92962..b0d688eb99 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1338,7 +1338,7 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
  * clusters.
  */
 static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
-    unsigned int nb_clusters)
+    unsigned int nb_clusters, enum qcow2_discard_type type)
 {
     BDRVQcowState *s = bs->opaque;
     uint64_t *l2_table;
@@ -1367,7 +1367,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
         l2_table[l2_index + i] = cpu_to_be64(0);
 
         /* Then decrease the refcount */
-        qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
+        qcow2_free_any_clusters(bs, old_offset, 1, type);
     }
 
     ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
@@ -1379,7 +1379,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
 }
 
 int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
-    int nb_sectors)
+    int nb_sectors, enum qcow2_discard_type type)
 {
     BDRVQcowState *s = bs->opaque;
     uint64_t end_offset;
@@ -1402,7 +1402,7 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
 
     /* Each L2 table is handled by its own loop iteration */
     while (nb_clusters > 0) {
-        ret = discard_single_l2(bs, offset, nb_clusters);
+        ret = discard_single_l2(bs, offset, nb_clusters, type);
         if (ret < 0) {
             goto fail;
         }
diff --git a/block/qcow2.c b/block/qcow2.c
index 578792f0a3..147822e8fd 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1582,7 +1582,7 @@ static coroutine_fn int qcow2_co_discard(BlockDriverState *bs,
 
     qemu_co_mutex_lock(&s->lock);
     ret = qcow2_discard_clusters(bs, sector_num << BDRV_SECTOR_BITS,
-        nb_sectors);
+        nb_sectors, QCOW2_DISCARD_REQUEST);
     qemu_co_mutex_unlock(&s->lock);
     return ret;
 }
diff --git a/block/qcow2.h b/block/qcow2.h
index 1000239e4c..9c33b98457 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -450,7 +450,7 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
 
 int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
 int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
-    int nb_sectors);
+    int nb_sectors, enum qcow2_discard_type type);
 int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors);
 
 /* qcow2-snapshot.c functions */

From 1ebf561c11302f4fbe4afdd82758fe053cf1d5fc Mon Sep 17 00:00:00 2001
From: Kevin Wolf <kwolf@redhat.com>
Date: Fri, 6 Sep 2013 12:20:08 +0200
Subject: [PATCH 02/33] qcow2: Discard VM state in active L1 after creating
 snapshot

During savevm, the VM state is written to the active L1 of the image and
then a snapshot is taken. After that, the VM state isn't needed any more
in the active L1 and should be discarded. This is implemented by this
patch.

The impact of not discarding the VM state is that a snapshot can never
become smaller than any previous snapshot (because it would be padded
with old VM state), and more importantly that future savevm operations
cause unnecessary COWs (with associated flushes), which makes subsequent
snapshots much slower.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-snapshot.c | 7 +++++++
 block/qcow2.c          | 5 -----
 block/qcow2.h          | 5 +++++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index e7e601301a..ffead08ca2 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -416,6 +416,13 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
 
     g_free(old_snapshot_list);
 
+    /* The VM state isn't needed any more in the active L1 table; in fact, it
+     * hurts by causing expensive COW for the next snapshot. */
+    qcow2_discard_clusters(bs, qcow2_vm_state_offset(s),
+                           align_offset(sn->vm_state_size, s->cluster_size)
+                                >> BDRV_SECTOR_BITS,
+                           QCOW2_DISCARD_NEVER);
+
 #ifdef DEBUG_ALLOC
     {
       BdrvCheckResult result = {0};
diff --git a/block/qcow2.c b/block/qcow2.c
index 147822e8fd..c9e266e22e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1757,11 +1757,6 @@ static coroutine_fn int qcow2_co_flush_to_os(BlockDriverState *bs)
     return 0;
 }
 
-static int64_t qcow2_vm_state_offset(BDRVQcowState *s)
-{
-	return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
-}
-
 static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
     BDRVQcowState *s = bs->opaque;
diff --git a/block/qcow2.h b/block/qcow2.h
index 9c33b98457..49eed828c5 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -361,6 +361,11 @@ static inline int64_t align_offset(int64_t offset, int n)
     return offset;
 }
 
+static inline int64_t qcow2_vm_state_offset(BDRVQcowState *s)
+{
+    return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
+}
+
 static inline int qcow2_get_cluster_type(uint64_t l2_entry)
 {
     if (l2_entry & QCOW_OFLAG_COMPRESSED) {

From 56e023af805215260c71d44f5b5a98087f4920d2 Mon Sep 17 00:00:00 2001
From: Tal Kain <tal.kain@ravellosystems.com>
Date: Mon, 9 Sep 2013 11:14:55 +0200
Subject: [PATCH 03/33] raw-win32.c: Fix incorrect handling behaviour of small
 block files

It is a valid case that the read data's size is smaller than the
requested size since there could be files that are smaller than
the minimum block size (For ex. when a VMDK disk descriptor file)

Signed-off-by: Tal Kain <tal.kain@ravellosystems.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/raw-win32.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index d2d2d9f4d4..ff3c5ea0d7 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -85,6 +85,7 @@ static size_t handle_aiocb_rw(RawWin32AIOData *aiocb)
             ret_count = 0;
         }
         if (ret_count != len) {
+            offset += ret_count;
             break;
         }
         offset += len;

From 6f176b48f9f98820ed192a1355cc1c7c08ddf46b Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Tue, 3 Sep 2013 10:09:50 +0200
Subject: [PATCH 04/33] block: Image file option amendment

This patch adds the "amend" option to qemu-img which allows changing
image options on existing image files. It also adds the generic bdrv
implementation which is basically just a wrapper for the image format
specific function.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   |  8 ++++
 include/block/block.h     |  2 +
 include/block/block_int.h |  3 ++
 qemu-img-cmds.hx          |  6 +++
 qemu-img.c                | 84 +++++++++++++++++++++++++++++++++++++++
 qemu-img.texi             |  5 +++
 6 files changed, 108 insertions(+)

diff --git a/block.c b/block.c
index a325efcb21..b81d1e210a 100644
--- a/block.c
+++ b/block.c
@@ -4579,3 +4579,11 @@ void bdrv_add_before_write_notifier(BlockDriverState *bs,
 {
     notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
 }
+
+int bdrv_amend_options(BlockDriverState *bs, QEMUOptionParameter *options)
+{
+    if (bs->drv->bdrv_amend_options == NULL) {
+        return -ENOTSUP;
+    }
+    return bs->drv->bdrv_amend_options(bs, options);
+}
diff --git a/include/block/block.h b/include/block/block.h
index 728ec1aebf..1c5f939d04 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -241,6 +241,8 @@ typedef enum {
 
 int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
 
+int bdrv_amend_options(BlockDriverState *bs_new, QEMUOptionParameter *options);
+
 /* async block I/O */
 typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
                                      int sector_num);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7c35198ad7..1541777d42 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -188,6 +188,9 @@ struct BlockDriver {
     int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result,
         BdrvCheckMode fix);
 
+    int (*bdrv_amend_options)(BlockDriverState *bs,
+        QEMUOptionParameter *options);
+
     void (*bdrv_debug_event)(BlockDriverState *bs, BlkDebugEvent event);
 
     /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 0c36e5968f..da1d965f3e 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -67,5 +67,11 @@ DEF("resize", img_resize,
     "resize [-q] filename [+ | -]size")
 STEXI
 @item resize [-q] @var{filename} [+ | -]@var{size}
+ETEXI
+
+DEF("amend", img_amend,
+    "amend [-q] [-f fmt] -o options filename")
+STEXI
+@item amend [-q] [-f @var{fmt}] -o @var{options} @var{filename}
 @end table
 ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index 3e5e388d1c..0cf6be2280 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2525,6 +2525,90 @@ out:
     return 0;
 }
 
+static int img_amend(int argc, char **argv)
+{
+    int c, ret = 0;
+    char *options = NULL;
+    QEMUOptionParameter *create_options = NULL, *options_param = NULL;
+    const char *fmt = NULL, *filename;
+    bool quiet = false;
+    BlockDriverState *bs = NULL;
+
+    for (;;) {
+        c = getopt(argc, argv, "hqf:o:");
+        if (c == -1) {
+            break;
+        }
+
+        switch (c) {
+            case 'h':
+            case '?':
+                help();
+                break;
+            case 'o':
+                options = optarg;
+                break;
+            case 'f':
+                fmt = optarg;
+                break;
+            case 'q':
+                quiet = true;
+                break;
+        }
+    }
+
+    if (optind != argc - 1) {
+        help();
+    }
+
+    if (!options) {
+        help();
+    }
+
+    filename = argv[argc - 1];
+
+    bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
+    if (!bs) {
+        error_report("Could not open image '%s'", filename);
+        ret = -1;
+        goto out;
+    }
+
+    fmt = bs->drv->format_name;
+
+    if (is_help_option(options)) {
+        ret = print_block_option_help(filename, fmt);
+        goto out;
+    }
+
+    create_options = append_option_parameters(create_options,
+            bs->drv->create_options);
+    options_param = parse_option_parameters(options, create_options,
+            options_param);
+    if (options_param == NULL) {
+        error_report("Invalid options for file format '%s'", fmt);
+        ret = -1;
+        goto out;
+    }
+
+    ret = bdrv_amend_options(bs, options_param);
+    if (ret < 0) {
+        error_report("Error while amending options: %s", strerror(-ret));
+        goto out;
+    }
+
+out:
+    if (bs) {
+        bdrv_unref(bs);
+    }
+    free_option_parameters(create_options);
+    free_option_parameters(options_param);
+    if (ret) {
+        return 1;
+    }
+    return 0;
+}
+
 static const img_cmd_t img_cmds[] = {
 #define DEF(option, callback, arg_string)        \
     { option, callback },
diff --git a/qemu-img.texi b/qemu-img.texi
index 43ee4eb5c4..768054e900 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -350,6 +350,11 @@ sizes accordingly.  Failure to do so will result in data loss!
 After using this command to grow a disk image, you must use file system and
 partitioning tools inside the VM to actually begin using the new space on the
 device.
+
+@item amend [-f @var{fmt}] -o @var{options} @var{filename}
+
+Amends the image format specific @var{options} for the image file
+@var{filename}. Not all file formats support this operation.
 @end table
 @c man end
 

From e7108feaace8e02b3a4bf010448fc2744f753381 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Tue, 3 Sep 2013 10:09:51 +0200
Subject: [PATCH 05/33] qcow2-cache: Empty cache

Add a function for emptying a cache, i.e., flushing it and marking all
elements invalid.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c | 18 ++++++++++++++++++
 block/qcow2.h       |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 7bcae09a69..40a5a3fc39 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -202,6 +202,24 @@ void qcow2_cache_depends_on_flush(Qcow2Cache *c)
     c->depends_on_flush = true;
 }
 
+int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c)
+{
+    int ret, i;
+
+    ret = qcow2_cache_flush(bs, c);
+    if (ret < 0) {
+        return ret;
+    }
+
+    for (i = 0; i < c->size; i++) {
+        assert(c->entries[i].ref == 0);
+        c->entries[i].offset = 0;
+        c->entries[i].cache_hits = 0;
+    }
+
+    return 0;
+}
+
 static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c)
 {
     int i;
diff --git a/block/qcow2.h b/block/qcow2.h
index 49eed828c5..35c822b8a6 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -478,6 +478,8 @@ int qcow2_cache_set_dependency(BlockDriverState *bs, Qcow2Cache *c,
     Qcow2Cache *dependency);
 void qcow2_cache_depends_on_flush(Qcow2Cache *c);
 
+int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c);
+
 int qcow2_cache_get(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
     void **table);
 int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,

From 32b6444d23d0ff618d73e5b766600cd258066169 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Tue, 3 Sep 2013 10:09:52 +0200
Subject: [PATCH 06/33] qcow2-cluster: Expand zero clusters

Add functionality for expanding zero clusters. This is necessary for
downgrading the image version to one without zero cluster support.

For non-backed images, this function may also just discard zero clusters
instead of truly expanding them.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c  | 233 +++++++++++++++++++++++++++++++++++++++++
 block/qcow2-refcount.c |  29 ++---
 block/qcow2.h          |   5 +
 3 files changed, 253 insertions(+), 14 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index b0d688eb99..738ff73c1d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1497,3 +1497,236 @@ fail:
 
     return ret;
 }
+
+/*
+ * Expands all zero clusters in a specific L1 table (or deallocates them, for
+ * non-backed non-pre-allocated zero clusters).
+ *
+ * expanded_clusters is a bitmap where every bit corresponds to one cluster in
+ * the image file; a bit gets set if the corresponding cluster has been used for
+ * zero expansion (i.e., has been filled with zeroes and is referenced from an
+ * L2 table). nb_clusters contains the total cluster count of the image file,
+ * i.e., the number of bits in expanded_clusters.
+ */
+static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
+                                      int l1_size, uint8_t *expanded_clusters,
+                                      uint64_t nb_clusters)
+{
+    BDRVQcowState *s = bs->opaque;
+    bool is_active_l1 = (l1_table == s->l1_table);
+    uint64_t *l2_table = NULL;
+    int ret;
+    int i, j;
+
+    if (!is_active_l1) {
+        /* inactive L2 tables require a buffer to be stored in when loading
+         * them from disk */
+        l2_table = qemu_blockalign(bs, s->cluster_size);
+    }
+
+    for (i = 0; i < l1_size; i++) {
+        uint64_t l2_offset = l1_table[i] & L1E_OFFSET_MASK;
+        bool l2_dirty = false;
+
+        if (!l2_offset) {
+            /* unallocated */
+            continue;
+        }
+
+        if (is_active_l1) {
+            /* get active L2 tables from cache */
+            ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
+                    (void **)&l2_table);
+        } else {
+            /* load inactive L2 tables from disk */
+            ret = bdrv_read(bs->file, l2_offset / BDRV_SECTOR_SIZE,
+                    (void *)l2_table, s->cluster_sectors);
+        }
+        if (ret < 0) {
+            goto fail;
+        }
+
+        for (j = 0; j < s->l2_size; j++) {
+            uint64_t l2_entry = be64_to_cpu(l2_table[j]);
+            int64_t offset = l2_entry & L2E_OFFSET_MASK, cluster_index;
+            int cluster_type = qcow2_get_cluster_type(l2_entry);
+
+            if (cluster_type == QCOW2_CLUSTER_NORMAL) {
+                cluster_index = offset >> s->cluster_bits;
+                assert((cluster_index >= 0) && (cluster_index < nb_clusters));
+                if (expanded_clusters[cluster_index / 8] &
+                    (1 << (cluster_index % 8))) {
+                    /* Probably a shared L2 table; this cluster was a zero
+                     * cluster which has been expanded, its refcount
+                     * therefore most likely requires an update. */
+                    ret = qcow2_update_cluster_refcount(bs, cluster_index, 1,
+                                                        QCOW2_DISCARD_NEVER);
+                    if (ret < 0) {
+                        goto fail;
+                    }
+                    /* Since we just increased the refcount, the COPIED flag may
+                     * no longer be set. */
+                    l2_table[j] = cpu_to_be64(l2_entry & ~QCOW_OFLAG_COPIED);
+                    l2_dirty = true;
+                }
+                continue;
+            }
+            else if (qcow2_get_cluster_type(l2_entry) != QCOW2_CLUSTER_ZERO) {
+                continue;
+            }
+
+            if (!offset) {
+                /* not preallocated */
+                if (!bs->backing_hd) {
+                    /* not backed; therefore we can simply deallocate the
+                     * cluster */
+                    l2_table[j] = 0;
+                    l2_dirty = true;
+                    continue;
+                }
+
+                offset = qcow2_alloc_clusters(bs, s->cluster_size);
+                if (offset < 0) {
+                    ret = offset;
+                    goto fail;
+                }
+            }
+
+            ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
+                                                offset, s->cluster_size);
+            if (ret < 0) {
+                qcow2_free_clusters(bs, offset, s->cluster_size,
+                        QCOW2_DISCARD_ALWAYS);
+                goto fail;
+            }
+
+            ret = bdrv_write_zeroes(bs->file, offset / BDRV_SECTOR_SIZE,
+                                    s->cluster_sectors);
+            if (ret < 0) {
+                qcow2_free_clusters(bs, offset, s->cluster_size,
+                        QCOW2_DISCARD_ALWAYS);
+                goto fail;
+            }
+
+            l2_table[j] = cpu_to_be64(offset | QCOW_OFLAG_COPIED);
+            l2_dirty = true;
+
+            cluster_index = offset >> s->cluster_bits;
+            assert((cluster_index >= 0) && (cluster_index < nb_clusters));
+            expanded_clusters[cluster_index / 8] |= 1 << (cluster_index % 8);
+        }
+
+        if (is_active_l1) {
+            if (l2_dirty) {
+                qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+                qcow2_cache_depends_on_flush(s->l2_table_cache);
+            }
+            ret = qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table);
+            if (ret < 0) {
+                l2_table = NULL;
+                goto fail;
+            }
+        } else {
+            if (l2_dirty) {
+                ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT &
+                        ~(QCOW2_OL_INACTIVE_L2 | QCOW2_OL_ACTIVE_L2), l2_offset,
+                        s->cluster_size);
+                if (ret < 0) {
+                    goto fail;
+                }
+
+                ret = bdrv_write(bs->file, l2_offset / BDRV_SECTOR_SIZE,
+                        (void *)l2_table, s->cluster_sectors);
+                if (ret < 0) {
+                    goto fail;
+                }
+            }
+        }
+    }
+
+    ret = 0;
+
+fail:
+    if (l2_table) {
+        if (!is_active_l1) {
+            qemu_vfree(l2_table);
+        } else {
+            if (ret < 0) {
+                qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table);
+            } else {
+                ret = qcow2_cache_put(bs, s->l2_table_cache,
+                        (void **)&l2_table);
+            }
+        }
+    }
+    return ret;
+}
+
+/*
+ * For backed images, expands all zero clusters on the image. For non-backed
+ * images, deallocates all non-pre-allocated zero clusters (and claims the
+ * allocation for pre-allocated ones). This is important for downgrading to a
+ * qcow2 version which doesn't yet support metadata zero clusters.
+ */
+int qcow2_expand_zero_clusters(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+    uint64_t *l1_table = NULL;
+    int cluster_to_sector_bits = s->cluster_bits - BDRV_SECTOR_BITS;
+    uint64_t nb_clusters;
+    uint8_t *expanded_clusters;
+    int ret;
+    int i, j;
+
+    nb_clusters = (bs->total_sectors + (1 << cluster_to_sector_bits) - 1)
+            >> cluster_to_sector_bits;
+    expanded_clusters = g_malloc0((nb_clusters + 7) / 8);
+
+    ret = expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size,
+                                     expanded_clusters, nb_clusters);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    /* Inactive L1 tables may point to active L2 tables - therefore it is
+     * necessary to flush the L2 table cache before trying to access the L2
+     * tables pointed to by inactive L1 entries (else we might try to expand
+     * zero clusters that have already been expanded); furthermore, it is also
+     * necessary to empty the L2 table cache, since it may contain tables which
+     * are now going to be modified directly on disk, bypassing the cache.
+     * qcow2_cache_empty() does both for us. */
+    ret = qcow2_cache_empty(bs, s->l2_table_cache);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    for (i = 0; i < s->nb_snapshots; i++) {
+        int l1_sectors = (s->snapshots[i].l1_size * sizeof(uint64_t) +
+                BDRV_SECTOR_SIZE - 1) / BDRV_SECTOR_SIZE;
+
+        l1_table = g_realloc(l1_table, l1_sectors * BDRV_SECTOR_SIZE);
+
+        ret = bdrv_read(bs->file, s->snapshots[i].l1_table_offset /
+                BDRV_SECTOR_SIZE, (void *)l1_table, l1_sectors);
+        if (ret < 0) {
+            goto fail;
+        }
+
+        for (j = 0; j < s->snapshots[i].l1_size; j++) {
+            be64_to_cpus(&l1_table[j]);
+        }
+
+        ret = expand_zero_clusters_in_l1(bs, l1_table, s->snapshots[i].l1_size,
+                                         expanded_clusters, nb_clusters);
+        if (ret < 0) {
+            goto fail;
+        }
+    }
+
+    ret = 0;
+
+fail:
+    g_free(expanded_clusters);
+    g_free(l1_table);
+    return ret;
+}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ba129de478..4264148142 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -601,10 +601,10 @@ fail:
  * If the return value is non-negative, it is the new refcount of the cluster.
  * If it is negative, it is -errno and indicates an error.
  */
-static int update_cluster_refcount(BlockDriverState *bs,
-                                   int64_t cluster_index,
-                                   int addend,
-                                   enum qcow2_discard_type type)
+int qcow2_update_cluster_refcount(BlockDriverState *bs,
+                                  int64_t cluster_index,
+                                  int addend,
+                                  enum qcow2_discard_type type)
 {
     BDRVQcowState *s = bs->opaque;
     int ret;
@@ -733,8 +733,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
         if (free_in_cluster == 0)
             s->free_byte_offset = 0;
         if ((offset & (s->cluster_size - 1)) != 0)
-            update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
-                                    QCOW2_DISCARD_NEVER);
+            qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+                                          QCOW2_DISCARD_NEVER);
     } else {
         offset = qcow2_alloc_clusters(bs, s->cluster_size);
         if (offset < 0) {
@@ -744,8 +744,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
         if ((cluster_offset + s->cluster_size) == offset) {
             /* we are lucky: contiguous data */
             offset = s->free_byte_offset;
-            update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
-                                    QCOW2_DISCARD_NEVER);
+            qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
+                                          QCOW2_DISCARD_NEVER);
             s->free_byte_offset += size;
         } else {
             s->free_byte_offset = offset;
@@ -754,8 +754,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
     }
 
     /* The cluster refcount was incremented, either by qcow2_alloc_clusters()
-     * or explicitly by update_cluster_refcount().  Refcount blocks must be
-     * flushed before the caller's L2 table updates.
+     * or explicitly by qcow2_update_cluster_refcount().  Refcount blocks must
+     * be flushed before the caller's L2 table updates.
      */
     qcow2_cache_set_dependency(bs, s->l2_table_cache, s->refcount_block_cache);
     return offset;
@@ -896,8 +896,9 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
                             break;
                         }
                         if (addend != 0) {
-                            refcount = update_cluster_refcount(bs, cluster_index, addend,
-                                                               QCOW2_DISCARD_SNAPSHOT);
+                            refcount = qcow2_update_cluster_refcount(bs,
+                                    cluster_index, addend,
+                                    QCOW2_DISCARD_SNAPSHOT);
                         } else {
                             refcount = get_refcount(bs, cluster_index);
                         }
@@ -936,8 +937,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 
 
             if (addend != 0) {
-                refcount = update_cluster_refcount(bs, l2_offset >> s->cluster_bits, addend,
-                                                   QCOW2_DISCARD_SNAPSHOT);
+                refcount = qcow2_update_cluster_refcount(bs, l2_offset >>
+                        s->cluster_bits, addend, QCOW2_DISCARD_SNAPSHOT);
             } else {
                 refcount = get_refcount(bs, l2_offset >> s->cluster_bits);
             }
diff --git a/block/qcow2.h b/block/qcow2.h
index 35c822b8a6..48080fdc03 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -411,6 +411,9 @@ int qcow2_update_header(BlockDriverState *bs);
 int qcow2_refcount_init(BlockDriverState *bs);
 void qcow2_refcount_close(BlockDriverState *bs);
 
+int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index,
+                                  int addend, enum qcow2_discard_type type);
+
 int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size);
 int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
     int nb_clusters);
@@ -458,6 +461,8 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
     int nb_sectors, enum qcow2_discard_type type);
 int qcow2_zero_clusters(BlockDriverState *bs, uint64_t offset, int nb_sectors);
 
+int qcow2_expand_zero_clusters(BlockDriverState *bs);
+
 /* qcow2-snapshot.c functions */
 int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
 int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id);

From b6481f376bc65894910dd98db3f299d698817106 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Tue, 3 Sep 2013 10:09:53 +0200
Subject: [PATCH 07/33] qcow2: Save refcount order in BDRVQcowState

Save the image refcount order in BDRVQcowState. This will be relevant
for future code supporting different refcount orders than four and also
for code that needs to verify a certain refcount order for an opened
image.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 3 ++-
 block/qcow2.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index c9e266e22e..27203f8f07 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -455,6 +455,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
         ret = -ENOTSUP;
         goto fail;
     }
+    s->refcount_order = header.refcount_order;
 
     if (header.cluster_bits < MIN_CLUSTER_BITS ||
         header.cluster_bits > MAX_CLUSTER_BITS) {
@@ -1143,7 +1144,7 @@ int qcow2_update_header(BlockDriverState *bs)
         .incompatible_features  = cpu_to_be64(s->incompatible_features),
         .compatible_features    = cpu_to_be64(s->compatible_features),
         .autoclear_features     = cpu_to_be64(s->autoclear_features),
-        .refcount_order         = cpu_to_be32(3 + REFCOUNT_SHIFT),
+        .refcount_order         = cpu_to_be32(s->refcount_order),
         .header_length          = cpu_to_be32(header_length),
     };
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 48080fdc03..bea6ddb43a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -199,6 +199,7 @@ typedef struct BDRVQcowState {
     int flags;
     int qcow_version;
     bool use_lazy_refcounts;
+    int refcount_order;
 
     bool discard_passthrough[QCOW2_DISCARD_MAX];
 

From 9296b3ed7050cc6e0645fbc3b0aea74406d7eeb2 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Tue, 3 Sep 2013 10:09:54 +0200
Subject: [PATCH 08/33] qcow2: Implement bdrv_amend_options

Implement bdrv_amend_options for compat, size, backing_file, backing_fmt
and lazy_refcounts.

Downgrading images from compat=1.1 to compat=0.10 is achieved through
handling all incompatible flags accordingly, clearing all compatible and
autoclear flags and expanding all zero clusters.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 194 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 27203f8f07..7c9354cadf 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1820,6 +1820,199 @@ static int qcow2_load_vmstate(BlockDriverState *bs, uint8_t *buf,
     return ret;
 }
 
+/*
+ * Downgrades an image's version. To achieve this, any incompatible features
+ * have to be removed.
+ */
+static int qcow2_downgrade(BlockDriverState *bs, int target_version)
+{
+    BDRVQcowState *s = bs->opaque;
+    int current_version = s->qcow_version;
+    int ret;
+
+    if (target_version == current_version) {
+        return 0;
+    } else if (target_version > current_version) {
+        return -EINVAL;
+    } else if (target_version != 2) {
+        return -EINVAL;
+    }
+
+    if (s->refcount_order != 4) {
+        /* we would have to convert the image to a refcount_order == 4 image
+         * here; however, since qemu (at the time of writing this) does not
+         * support anything different than 4 anyway, there is no point in doing
+         * so right now; however, we should error out (if qemu supports this in
+         * the future and this code has not been adapted) */
+        error_report("qcow2_downgrade: Image refcount orders other than 4 are"
+                     "currently not supported.");
+        return -ENOTSUP;
+    }
+
+    /* clear incompatible features */
+    if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) {
+        ret = qcow2_mark_clean(bs);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    /* with QCOW2_INCOMPAT_CORRUPT, it is pretty much impossible to get here in
+     * the first place; if that happens nonetheless, returning -ENOTSUP is the
+     * best thing to do anyway */
+
+    if (s->incompatible_features) {
+        return -ENOTSUP;
+    }
+
+    /* since we can ignore compatible features, we can set them to 0 as well */
+    s->compatible_features = 0;
+    /* if lazy refcounts have been used, they have already been fixed through
+     * clearing the dirty flag */
+
+    /* clearing autoclear features is trivial */
+    s->autoclear_features = 0;
+
+    ret = qcow2_expand_zero_clusters(bs);
+    if (ret < 0) {
+        return ret;
+    }
+
+    s->qcow_version = target_version;
+    ret = qcow2_update_header(bs);
+    if (ret < 0) {
+        s->qcow_version = current_version;
+        return ret;
+    }
+    return 0;
+}
+
+static int qcow2_amend_options(BlockDriverState *bs,
+                               QEMUOptionParameter *options)
+{
+    BDRVQcowState *s = bs->opaque;
+    int old_version = s->qcow_version, new_version = old_version;
+    uint64_t new_size = 0;
+    const char *backing_file = NULL, *backing_format = NULL;
+    bool lazy_refcounts = s->use_lazy_refcounts;
+    int ret;
+    int i;
+
+    for (i = 0; options[i].name; i++)
+    {
+        if (!options[i].assigned) {
+            /* only change explicitly defined options */
+            continue;
+        }
+
+        if (!strcmp(options[i].name, "compat")) {
+            if (!options[i].value.s) {
+                /* preserve default */
+            } else if (!strcmp(options[i].value.s, "0.10")) {
+                new_version = 2;
+            } else if (!strcmp(options[i].value.s, "1.1")) {
+                new_version = 3;
+            } else {
+                fprintf(stderr, "Unknown compatibility level %s.\n",
+                        options[i].value.s);
+                return -EINVAL;
+            }
+        } else if (!strcmp(options[i].name, "preallocation")) {
+            fprintf(stderr, "Cannot change preallocation mode.\n");
+            return -ENOTSUP;
+        } else if (!strcmp(options[i].name, "size")) {
+            new_size = options[i].value.n;
+        } else if (!strcmp(options[i].name, "backing_file")) {
+            backing_file = options[i].value.s;
+        } else if (!strcmp(options[i].name, "backing_fmt")) {
+            backing_format = options[i].value.s;
+        } else if (!strcmp(options[i].name, "encryption")) {
+            if ((options[i].value.n != !!s->crypt_method)) {
+                fprintf(stderr, "Changing the encryption flag is not "
+                        "supported.\n");
+                return -ENOTSUP;
+            }
+        } else if (!strcmp(options[i].name, "cluster_size")) {
+            if (options[i].value.n != s->cluster_size) {
+                fprintf(stderr, "Changing the cluster size is not "
+                        "supported.\n");
+                return -ENOTSUP;
+            }
+        } else if (!strcmp(options[i].name, "lazy_refcounts")) {
+            lazy_refcounts = options[i].value.n;
+        } else {
+            /* if this assertion fails, this probably means a new option was
+             * added without having it covered here */
+            assert(false);
+        }
+    }
+
+    if (new_version != old_version) {
+        if (new_version > old_version) {
+            /* Upgrade */
+            s->qcow_version = new_version;
+            ret = qcow2_update_header(bs);
+            if (ret < 0) {
+                s->qcow_version = old_version;
+                return ret;
+            }
+        } else {
+            ret = qcow2_downgrade(bs, new_version);
+            if (ret < 0) {
+                return ret;
+            }
+        }
+    }
+
+    if (backing_file || backing_format) {
+        ret = qcow2_change_backing_file(bs, backing_file ?: bs->backing_file,
+                                        backing_format ?: bs->backing_format);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    if (s->use_lazy_refcounts != lazy_refcounts) {
+        if (lazy_refcounts) {
+            if (s->qcow_version < 3) {
+                fprintf(stderr, "Lazy refcounts only supported with compatibility "
+                        "level 1.1 and above (use compat=1.1 or greater)\n");
+                return -EINVAL;
+            }
+            s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
+            ret = qcow2_update_header(bs);
+            if (ret < 0) {
+                s->compatible_features &= ~QCOW2_COMPAT_LAZY_REFCOUNTS;
+                return ret;
+            }
+            s->use_lazy_refcounts = true;
+        } else {
+            /* make image clean first */
+            ret = qcow2_mark_clean(bs);
+            if (ret < 0) {
+                return ret;
+            }
+            /* now disallow lazy refcounts */
+            s->compatible_features &= ~QCOW2_COMPAT_LAZY_REFCOUNTS;
+            ret = qcow2_update_header(bs);
+            if (ret < 0) {
+                s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
+                return ret;
+            }
+            s->use_lazy_refcounts = false;
+        }
+    }
+
+    if (new_size) {
+        ret = bdrv_truncate(bs, new_size);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
 static QEMUOptionParameter qcow2_create_options[] = {
     {
         .name = BLOCK_OPT_SIZE,
@@ -1903,6 +2096,7 @@ static BlockDriver bdrv_qcow2 = {
 
     .create_options = qcow2_create_options,
     .bdrv_check = qcow2_check,
+    .bdrv_amend_options = qcow2_amend_options,
 };
 
 static void bdrv_qcow2_init(void)

From a8110c3d327cabff8dc258c5c8705903b56c1513 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Tue, 3 Sep 2013 10:09:55 +0200
Subject: [PATCH 09/33] qemu-iotest: qcow2 image option amendment

Add tests for qemu-img amend on qcow2 image files.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/061     | 178 +++++++++++++++++++
 tests/qemu-iotests/061.out | 349 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 528 insertions(+)
 create mode 100755 tests/qemu-iotests/061
 create mode 100644 tests/qemu-iotests/061.out

diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
new file mode 100755
index 0000000000..86404e6a51
--- /dev/null
+++ b/tests/qemu-iotests/061
@@ -0,0 +1,178 @@
+#!/bin/bash
+#
+# Test case for image option amendment in qcow2.
+#
+# Copyright (C) 2013 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qocw2-specific low-level functionality
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+echo
+echo "=== Testing version downgrade with zero expansion ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing dirty version downgrade ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing version downgrade with unknown compat/autoclear flags ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+./qcow2.py "$TEST_IMG" set-feature-bit compatible 42
+./qcow2.py "$TEST_IMG" set-feature-bit autoclear 42
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+_check_test_img
+
+echo
+echo "=== Testing version upgrade and resize ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "compat=1.1,lazy_refcounts=on,size=128M" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing dirty lazy_refcounts=off ==="
+echo
+IMGOPTS="compat=1.1,lazy_refcounts=on" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" -c flush -c abort "$TEST_IMG" | _filter_qemu_io
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IMG amend -o "lazy_refcounts=off" "$TEST_IMG"
+./qcow2.py "$TEST_IMG" dump-header
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing backing file ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "backing_file=$TEST_IMG.base,backing_fmt=qcow2" "$TEST_IMG"
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "=== Testing invalid configurations ==="
+echo
+IMGOPTS="compat=0.10" _make_test_img 64M
+$QEMU_IMG amend -o "lazy_refcounts=on" "$TEST_IMG"
+$QEMU_IMG amend -o "compat=1.1" "$TEST_IMG" # actually valid
+$QEMU_IMG amend -o "compat=0.10,lazy_refcounts=on" "$TEST_IMG"
+$QEMU_IMG amend -o "compat=0.42" "$TEST_IMG"
+$QEMU_IMG amend -o "foo=bar" "$TEST_IMG"
+$QEMU_IMG amend -o "cluster_size=1k" "$TEST_IMG"
+$QEMU_IMG amend -o "encryption=on" "$TEST_IMG"
+$QEMU_IMG amend -o "preallocation=on" "$TEST_IMG"
+
+echo
+echo "=== Testing correct handling of unset value ==="
+echo
+IMGOPTS="compat=1.1,cluster_size=1k" _make_test_img 64M
+echo "Should work:"
+$QEMU_IMG amend -o "lazy_refcounts=on" "$TEST_IMG"
+echo "Should not work:" # Just to know which of these tests actually fails
+$QEMU_IMG amend -o "cluster_size=64k" "$TEST_IMG"
+
+echo
+echo "=== Testing zero expansion on inactive clusters ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "=== Testing zero expansion on backed image ==="
+echo
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+IMGOPTS="compat=1.1,backing_file=$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "read -P 0x2a 0 128k" -c "write -z 0 64k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 64k" -c "read -P 0x2a 64k 64k" "$TEST_IMG" | _filter_qemu_io
+
+echo
+echo "=== Testing zero expansion on backed inactive clusters ==="
+echo
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+IMGOPTS="compat=1.1,backing_file=$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -z 0 64k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IO -c "write -P 0x42 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0x42 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 64k" -c "read -P 0x2a 64k 64k" "$TEST_IMG" | _filter_qemu_io
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
new file mode 100644
index 0000000000..05bd1d5ff1
--- /dev/null
+++ b/tests/qemu-iotests/061.out
@@ -0,0 +1,349 @@
+QA output created by 061
+
+=== Testing version downgrade with zero expansion ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x1
+autoclear_features        0x0
+refcount_order            4
+header_length             104
+
+magic                     0x514649fb
+version                   2
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x0
+autoclear_features        0x0
+refcount_order            4
+header_length             72
+
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing dirty version downgrade ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x1
+compatible_features       0x1
+autoclear_features        0x0
+refcount_order            4
+header_length             104
+
+Repairing cluster 5 refcount=0 reference=1
+Repairing cluster 6 refcount=0 reference=1
+magic                     0x514649fb
+version                   2
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x0
+autoclear_features        0x0
+refcount_order            4
+header_length             72
+
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing version downgrade with unknown compat/autoclear flags ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x40000000000
+autoclear_features        0x40000000000
+refcount_order            4
+header_length             104
+
+magic                     0x514649fb
+version                   2
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x0
+autoclear_features        0x0
+refcount_order            4
+header_length             72
+
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
+No errors were found on the image.
+
+=== Testing version upgrade and resize ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 65536/65536 bytes at offset 44040192
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic                     0x514649fb
+version                   2
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x0
+autoclear_features        0x0
+refcount_order            4
+header_length             72
+
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      134217728
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x1
+autoclear_features        0x0
+refcount_order            4
+header_length             104
+
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
+read 65536/65536 bytes at offset 44040192
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing dirty lazy_refcounts=off ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x1
+compatible_features       0x1
+autoclear_features        0x0
+refcount_order            4
+header_length             104
+
+Repairing cluster 5 refcount=0 reference=1
+Repairing cluster 6 refcount=0 reference=1
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x0
+compatible_features       0x0
+autoclear_features        0x0
+refcount_order            4
+header_length             104
+
+Header extension:
+magic                     0x6803f857
+length                    144
+data                      <binary>
+
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+=== Testing invalid configurations ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
+qemu-img: Error while amending options: Invalid argument
+Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
+qemu-img: Error while amending options: Invalid argument
+Unknown compatibility level 0.42.
+qemu-img: Error while amending options: Invalid argument
+Unknown option 'foo'
+qemu-img: Invalid options for file format 'qcow2'
+Changing the cluster size is not supported.
+qemu-img: Error while amending options: Operation not supported
+Changing the encryption flag is not supported.
+qemu-img: Error while amending options: Operation not supported
+Cannot change preallocation mode.
+qemu-img: Error while amending options: Operation not supported
+
+=== Testing correct handling of unset value ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Should work:
+Should not work:
+Changing the cluster size is not supported.
+qemu-img: Error while amending options: Operation not supported
+
+=== Testing zero expansion on inactive clusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing zero expansion on backed image ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.base' 
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing zero expansion on backed inactive clusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.base' 
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 316b1dd75c..8012828a17 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -65,5 +65,6 @@
 056 rw auto backing
 059 rw auto
 060 rw auto
+061 rw auto
 062 rw auto
 063 rw auto

From f93296eaffcb3753f680f2dcffea2637f14f2092 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Fri, 6 Sep 2013 11:24:32 +0800
Subject: [PATCH 10/33] qemu-iotests: add unix socket help program

This program can do a sendmsg call to transfer fd with unix
socket, which is not supported in python2.

The built binary will not be deleted in clean, but it is a
existing issue in ./tests, which should be solved in another
patch.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 configure                              |   2 +-
 tests/Makefile                         |   3 +-
 tests/qemu-iotests/socket_scm_helper.c | 135 +++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 2 deletions(-)
 create mode 100644 tests/qemu-iotests/socket_scm_helper.c

diff --git a/configure b/configure
index 2b83936e8e..b49902841e 100755
--- a/configure
+++ b/configure
@@ -4651,7 +4651,7 @@ if [ "$dtc_internal" = "yes" ]; then
 fi
 
 # build tree in object directory in case the source is not in the current directory
-DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa"
+DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
 DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
 DIRS="$DIRS roms/seabios roms/vgabios"
 DIRS="$DIRS qapi-generated"
diff --git a/tests/Makefile b/tests/Makefile
index c13fefc314..994fef1839 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -174,6 +174,7 @@ tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
 tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
+tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
 
 # QTest rules
 
@@ -252,7 +253,7 @@ check-report.html: check-report.xml
 # Other tests
 
 .PHONY: check-tests/qemu-iotests-quick.sh
-check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
+check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF) tests/qemu-iotests/socket_scm_helper$(EXESUF)
 	$<
 
 .PHONY: check-tests/test-qapi.py
diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
new file mode 100644
index 0000000000..0e2b2859af
--- /dev/null
+++ b/tests/qemu-iotests/socket_scm_helper.c
@@ -0,0 +1,135 @@
+/*
+ * SCM_RIGHTS with unix socket help program for test
+ *
+ * Copyright IBM, Inc. 2013
+ *
+ * Authors:
+ *  Wenchao Xia    <xiawenc@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include <stdio.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+/* #define SOCKET_SCM_DEBUG */
+
+/*
+ * @fd and @fd_to_send will not be checked for validation in this function,
+ * a blank will be sent as iov data to notify qemu.
+ */
+static int send_fd(int fd, int fd_to_send)
+{
+    struct msghdr msg;
+    struct iovec iov[1];
+    int ret;
+    char control[CMSG_SPACE(sizeof(int))];
+    struct cmsghdr *cmsg;
+
+    memset(&msg, 0, sizeof(msg));
+    memset(control, 0, sizeof(control));
+
+    /* Send a blank to notify qemu */
+    iov[0].iov_base = (void *)" ";
+    iov[0].iov_len = 1;
+
+    msg.msg_iov = iov;
+    msg.msg_iovlen = 1;
+
+    msg.msg_control = control;
+    msg.msg_controllen = sizeof(control);
+
+    cmsg = CMSG_FIRSTHDR(&msg);
+
+    cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+    cmsg->cmsg_level = SOL_SOCKET;
+    cmsg->cmsg_type = SCM_RIGHTS;
+    memcpy(CMSG_DATA(cmsg), &fd, sizeof(int));
+
+    do {
+        ret = sendmsg(fd, &msg, 0);
+    } while (ret < 0 && errno == EINTR);
+
+    if (ret < 0) {
+        fprintf(stderr, "Failed to send msg, reason: %s\n", strerror(errno));
+    }
+
+    return ret;
+}
+
+/* Convert string to fd number. */
+static int get_fd_num(const char *fd_str)
+{
+    int sock;
+    char *err;
+
+    errno = 0;
+    sock = strtol(fd_str, &err, 10);
+    if (errno) {
+        fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
+                strerror(errno));
+        return -1;
+    }
+    if (!*fd_str || *err || sock < 0) {
+        fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str);
+        return -1;
+    }
+
+    return sock;
+}
+
+/*
+ * To make things simple, the caller needs to specify:
+ * 1. socket fd.
+ * 2. path of the file to be sent.
+ */
+int main(int argc, char **argv, char **envp)
+{
+    int sock, fd, ret;
+
+#ifdef SOCKET_SCM_DEBUG
+    int i;
+    for (i = 0; i < argc; i++) {
+        fprintf(stderr, "Parameter %d: %s\n", i, argv[i]);
+    }
+#endif
+
+    if (argc != 3) {
+        fprintf(stderr,
+                "Usage: %s < socket-fd > < file-path >\n",
+                argv[0]);
+        return EXIT_FAILURE;
+    }
+
+
+    sock = get_fd_num(argv[1]);
+    if (sock < 0) {
+        return EXIT_FAILURE;
+    }
+
+    /* Now only open a file in readonly mode for test purpose. If more precise
+       control is needed, use python script in file operation, which is
+       supposed to fork and exec this program. */
+    fd = open(argv[2], O_RDONLY);
+    if (fd < 0) {
+        fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
+        return EXIT_FAILURE;
+    }
+
+    ret = send_fd(sock, fd);
+    if (ret < 0) {
+        close(fd);
+        return EXIT_FAILURE;
+    }
+
+    close(fd);
+    return EXIT_SUCCESS;
+}

From 30b005d9d75af6388899fad2f462efb8af2b25b3 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Fri, 6 Sep 2013 11:24:33 +0800
Subject: [PATCH 11/33] qemu-iotests: add infrastructure of fd passing via SCM

This patch make use of the compiled scm helper program to transfer
fd via unix socket at runtime.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 QMP/qmp.py                    |  6 ++++++
 tests/qemu-iotests/check      |  1 +
 tests/qemu-iotests/iotests.py | 23 +++++++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index c551df1ed7..074f09a063 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -188,3 +188,9 @@ class QEMUMonitorProtocol:
 
     def settimeout(self, timeout):
         self.__sock.settimeout(timeout)
+
+    def get_sock_fd(self):
+        return self.__sock.fileno()
+
+    def is_scm_available(self):
+        return self.__sock.family == socket.AF_UNIX
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 4ecf497d8e..f5f328f5f5 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -164,6 +164,7 @@ QEMU_IO       -- $QEMU_IO
 IMGFMT        -- $FULL_IMGFMT_DETAILS
 IMGPROTO      -- $FULL_IMGPROTO_DETAILS
 PLATFORM      -- $FULL_HOST_DETAILS
+SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
 
 EOF
 #MKFS_OPTIONS  -- $FULL_MKFS_OPTIONS
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 33ad0ecb92..87b4a3a880 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -38,6 +38,8 @@ imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
 test_dir = os.environ.get('TEST_DIR', '/var/tmp')
 
+socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
+
 def qemu_img(*args):
     '''Run qemu-img and return the exit code'''
     devnull = open('/dev/null', 'r+')
@@ -80,6 +82,12 @@ class VM(object):
                      '-display', 'none', '-vga', 'none']
         self._num_drives = 0
 
+    # This can be used to add an unused monitor instance.
+    def add_monitor_telnet(self, ip, port):
+        args = 'tcp:%s:%d,server,nowait,telnet' % (ip, port)
+        self._args.append('-monitor')
+        self._args.append(args)
+
     def add_drive(self, path, opts=''):
         '''Add a virtio-blk drive to the VM'''
         options = ['if=virtio',
@@ -112,6 +120,21 @@ class VM(object):
         self._args.append(','.join(options))
         return self
 
+    def send_fd_scm(self, fd_file_path):
+        # In iotest.py, the qmp should always use unix socket.
+        assert self._qmp.is_scm_available()
+        bin = socket_scm_helper
+        if os.path.exists(bin) == False:
+            print "Scm help program does not present, path '%s'." % bin
+            return -1
+        fd_param = ["%s" % bin,
+                    "%d" % self._qmp.get_sock_fd(),
+                    "%s" % fd_file_path]
+        devnull = open('/dev/null', 'rb')
+        p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
+                             stderr=sys.stderr)
+        return p.wait()
+
     def launch(self):
         '''Launch the VM and establish a QMP connection'''
         devnull = open('/dev/null', 'rb')

From fd9c577b24818e0051e93c4f1e3db7262869f162 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Fri, 6 Sep 2013 11:24:34 +0800
Subject: [PATCH 12/33] qemu-iotests: add tests for runtime fd passing via SCM
 rights

This case will test whether the monitor can receive fd at runtime.
To verify better, additional monitor is created to see if qemu
can handler two monitor instances correctly.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/045     | 51 +++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/045.out |  4 +--
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/045 b/tests/qemu-iotests/045
index 2b6f1af27a..6be8fc4912 100755
--- a/tests/qemu-iotests/045
+++ b/tests/qemu-iotests/045
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 #
-# Tests for fdsets.
+# Tests for fdsets and getfd.
 #
 # Copyright (C) 2012 IBM Corp.
 #
@@ -125,5 +125,54 @@ class TestFdSets(iotests.QMPTestCase):
                 'No file descriptor supplied via SCM_RIGHTS')
         self.vm.shutdown()
 
+# Add fd at runtime, there are two ways: monitor related or fdset related
+class TestSCMFd(iotests.QMPTestCase):
+    def setUp(self):
+        self.vm = iotests.VM()
+        qemu_img('create', '-f', iotests.imgfmt, image0, '128K')
+        # Add an unused monitor, to verify it works fine when two monitor
+        # instances present
+        self.vm.add_monitor_telnet("0",4445)
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(image0)
+
+    def _send_fd_by_SCM(self):
+        ret = self.vm.send_fd_scm(image0)
+        self.assertEqual(ret, 0, 'Failed to send fd with UNIX SCM')
+
+    def test_add_fd(self):
+        self._send_fd_by_SCM()
+        result = self.vm.qmp('add-fd', fdset_id=2, opaque='image0:r')
+        self.assert_qmp(result, 'return/fdset-id', 2)
+
+    def test_getfd(self):
+        self._send_fd_by_SCM()
+        result = self.vm.qmp('getfd', fdname='image0:r')
+        self.assert_qmp(result, 'return', {})
+
+    def test_getfd_invalid_fdname(self):
+        self._send_fd_by_SCM()
+        result = self.vm.qmp('getfd', fdname='0image0:r')
+        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "Parameter 'fdname' expects a name not starting with a digit")
+
+    def test_closefd(self):
+        self._send_fd_by_SCM()
+        result = self.vm.qmp('getfd', fdname='image0:r')
+        self.assert_qmp(result, 'return', {})
+        result = self.vm.qmp('closefd', fdname='image0:r')
+        self.assert_qmp(result, 'return', {})
+
+    def test_closefd_fd_not_found(self):
+        fdname = 'image0:r'
+        result = self.vm.qmp('closefd', fdname=fdname)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+        self.assert_qmp(result, 'error/desc',
+            "File descriptor named '%s' not found" % fdname)
+
 if __name__ == '__main__':
     iotests.main(supported_fmts=['raw'])
diff --git a/tests/qemu-iotests/045.out b/tests/qemu-iotests/045.out
index 3f8a935a08..e56cae021b 100644
--- a/tests/qemu-iotests/045.out
+++ b/tests/qemu-iotests/045.out
@@ -1,5 +1,5 @@
-......
+...........
 ----------------------------------------------------------------------
-Ran 6 tests
+Ran 11 tests
 
 OK

From d982919d3897f36d79e215c46e3bc27fd6e27bf8 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Thu, 5 Sep 2013 10:55:54 +0200
Subject: [PATCH 13/33] qemu-iotests: New test case in 061

Add one test case for zero cluster expansion on qcow2 version downgrade
in shared L2 tables (i.e., L2 tables with a refcount > 1) and one for
zero expansion on backed clusters in shared L2 tables.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/061     | 28 ++++++++++++++++++++++++++++
 tests/qemu-iotests/061.out | 27 +++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index 86404e6a51..5f04bfa851 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -145,6 +145,19 @@ $QEMU_IMG snapshot -a foo "$TEST_IMG"
 _check_test_img
 $QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
 
+echo
+echo "=== Testing zero expansion on shared L2 table ==="
+echo
+IMGOPTS="compat=1.1" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+
 echo
 echo "=== Testing zero expansion on backed image ==="
 echo
@@ -172,6 +185,21 @@ $QEMU_IMG snapshot -a foo "$TEST_IMG"
 _check_test_img
 $QEMU_IO -c "read -P 0 0 64k" -c "read -P 0x2a 64k 64k" "$TEST_IMG" | _filter_qemu_io
 
+echo
+echo "=== Testing zero expansion on backed image with shared L2 table ==="
+echo
+IMGOPTS="compat=1.1" TEST_IMG="$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io
+IMGOPTS="compat=1.1,backing_file=$TEST_IMG.base" _make_test_img 64M
+$QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -a foo "$TEST_IMG"
+_check_test_img
+$QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index 05bd1d5ff1..d42127fb6f 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -312,6 +312,18 @@ No errors were found on the image.
 read 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+=== Testing zero expansion on shared L2 table ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
 === Testing zero expansion on backed image ===
 
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
@@ -346,4 +358,19 @@ read 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 65536/65536 bytes at offset 65536
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing zero expansion on backed image with shared L2 table ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.base' 
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+read 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done

From 2ea1dd758c45f8ff12c67ed7934c3ce021eeaf12 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:32 +0800
Subject: [PATCH 14/33] snapshot: new function
 bdrv_snapshot_find_by_id_and_name()

To make it clear about id and name in searching, add this API
to distinguish them. Caller can choose to search by id or name,
*errp will be set only for exception.

Some code are modified based on Pavel's patch.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/snapshot.c         | 73 ++++++++++++++++++++++++++++++++++++++++
 include/block/snapshot.h |  6 ++++
 2 files changed, 79 insertions(+)

diff --git a/block/snapshot.c b/block/snapshot.c
index 8f61cc0745..a923b386b6 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -48,6 +48,79 @@ int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
     return ret;
 }
 
+/**
+ * Look up an internal snapshot by @id and @name.
+ * @bs: block device to search
+ * @id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ * @sn_info: location to store information on the snapshot found
+ * @errp: location to store error, will be set only for exception
+ *
+ * This function will traverse snapshot list in @bs to search the matching
+ * one, @id and @name are the matching condition:
+ * If both @id and @name are specified, find the first one with id @id and
+ * name @name.
+ * If only @id is specified, find the first one with id @id.
+ * If only @name is specified, find the first one with name @name.
+ * if none is specified, abort().
+ *
+ * Returns: true when a snapshot is found and @sn_info will be filled, false
+ * when error or not found. If all operation succeed but no matching one is
+ * found, @errp will NOT be set.
+ */
+bool bdrv_snapshot_find_by_id_and_name(BlockDriverState *bs,
+                                       const char *id,
+                                       const char *name,
+                                       QEMUSnapshotInfo *sn_info,
+                                       Error **errp)
+{
+    QEMUSnapshotInfo *sn_tab, *sn;
+    int nb_sns, i;
+    bool ret = false;
+
+    assert(id || name);
+
+    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+    if (nb_sns < 0) {
+        error_setg_errno(errp, -nb_sns, "Failed to get a snapshot list");
+        return false;
+    } else if (nb_sns == 0) {
+        return false;
+    }
+
+    if (id && name) {
+        for (i = 0; i < nb_sns; i++) {
+            sn = &sn_tab[i];
+            if (!strcmp(sn->id_str, id) && !strcmp(sn->name, name)) {
+                *sn_info = *sn;
+                ret = true;
+                break;
+            }
+        }
+    } else if (id) {
+        for (i = 0; i < nb_sns; i++) {
+            sn = &sn_tab[i];
+            if (!strcmp(sn->id_str, id)) {
+                *sn_info = *sn;
+                ret = true;
+                break;
+            }
+        }
+    } else if (name) {
+        for (i = 0; i < nb_sns; i++) {
+            sn = &sn_tab[i];
+            if (!strcmp(sn->name, name)) {
+                *sn_info = *sn;
+                ret = true;
+                break;
+            }
+        }
+    }
+
+    g_free(sn_tab);
+    return ret;
+}
+
 int bdrv_can_snapshot(BlockDriverState *bs)
 {
     BlockDriver *drv = bs->drv;
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index eaf61f0326..9d06dc17a6 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -26,6 +26,7 @@
 #define SNAPSHOT_H
 
 #include "qemu-common.h"
+#include "qapi/error.h"
 
 typedef struct QEMUSnapshotInfo {
     char id_str[128]; /* unique snapshot id */
@@ -40,6 +41,11 @@ typedef struct QEMUSnapshotInfo {
 
 int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
                        const char *name);
+bool bdrv_snapshot_find_by_id_and_name(BlockDriverState *bs,
+                                       const char *id,
+                                       const char *name,
+                                       QEMUSnapshotInfo *sn_info,
+                                       Error **errp);
 int bdrv_can_snapshot(BlockDriverState *bs);
 int bdrv_snapshot_create(BlockDriverState *bs,
                          QEMUSnapshotInfo *sn_info);

From a89d89d3e65800fa4a8e00de7af0ea8272bef779 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:33 +0800
Subject: [PATCH 15/33] snapshot: distinguish id and name in snapshot delete

Snapshot creation actually already distinguish id and name since it take
a structured parameter *sn, but delete can't. Later an accurate delete
is needed in qmp_transaction abort and blockdev-snapshot-delete-sync,
so change its prototype. Also *errp is added to tip error, but return
value is kepted to let caller check what kind of error happens. Existing
caller for it are savevm, delvm and qemu-img, they are not impacted by
introducing a new function bdrv_snapshot_delete_by_id_or_name(), which
check the return value and do the operation again.

Before this patch:
  For qcow2, it search id first then name to find the one to delete.
  For rbd, it search name.
  For sheepdog, it does nothing.

After this patch:
  For qcow2, logic is the same by call it twice in caller.
  For rbd, it always fails in delete with id, but still search for name
in second try, no change to user.

Some code for *errp is based on Pavel's patch.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-snapshot.c    | 55 ++++++++++++++++++++++++++-----------
 block/qcow2.h             |  5 +++-
 block/rbd.c               | 21 +++++++++++++-
 block/sheepdog.c          |  5 +++-
 block/snapshot.c          | 58 +++++++++++++++++++++++++++++++++++++--
 include/block/block_int.h |  5 +++-
 include/block/snapshot.h  |  8 +++++-
 qemu-img.c                | 11 +++++---
 savevm.c                  | 32 +++++++++++----------
 9 files changed, 157 insertions(+), 43 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index ffead08ca2..7d144205c3 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -297,31 +297,47 @@ static void find_new_snapshot_id(BlockDriverState *bs,
     snprintf(id_str, id_str_size, "%d", id_max + 1);
 }
 
-static int find_snapshot_by_id(BlockDriverState *bs, const char *id_str)
+static int find_snapshot_by_id_and_name(BlockDriverState *bs,
+                                        const char *id,
+                                        const char *name)
 {
     BDRVQcowState *s = bs->opaque;
     int i;
 
-    for(i = 0; i < s->nb_snapshots; i++) {
-        if (!strcmp(s->snapshots[i].id_str, id_str))
-            return i;
+    if (id && name) {
+        for (i = 0; i < s->nb_snapshots; i++) {
+            if (!strcmp(s->snapshots[i].id_str, id) &&
+                !strcmp(s->snapshots[i].name, name)) {
+                return i;
+            }
+        }
+    } else if (id) {
+        for (i = 0; i < s->nb_snapshots; i++) {
+            if (!strcmp(s->snapshots[i].id_str, id)) {
+                return i;
+            }
+        }
+    } else if (name) {
+        for (i = 0; i < s->nb_snapshots; i++) {
+            if (!strcmp(s->snapshots[i].name, name)) {
+                return i;
+            }
+        }
     }
+
     return -1;
 }
 
-static int find_snapshot_by_id_or_name(BlockDriverState *bs, const char *name)
+static int find_snapshot_by_id_or_name(BlockDriverState *bs,
+                                       const char *id_or_name)
 {
-    BDRVQcowState *s = bs->opaque;
-    int i, ret;
+    int ret;
 
-    ret = find_snapshot_by_id(bs, name);
-    if (ret >= 0)
+    ret = find_snapshot_by_id_and_name(bs, id_or_name, NULL);
+    if (ret >= 0) {
         return ret;
-    for(i = 0; i < s->nb_snapshots; i++) {
-        if (!strcmp(s->snapshots[i].name, name))
-            return i;
     }
-    return -1;
+    return find_snapshot_by_id_and_name(bs, NULL, id_or_name);
 }
 
 /* if no id is provided, a new one is constructed */
@@ -343,7 +359,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
     }
 
     /* Check that the ID is unique */
-    if (find_snapshot_by_id(bs, sn_info->id_str) >= 0) {
+    if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
         return -EEXIST;
     }
 
@@ -560,15 +576,19 @@ fail:
     return ret;
 }
 
-int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+int qcow2_snapshot_delete(BlockDriverState *bs,
+                          const char *snapshot_id,
+                          const char *name,
+                          Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
     QCowSnapshot sn;
     int snapshot_index, ret;
 
     /* Search the snapshot */
-    snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
+    snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
     if (snapshot_index < 0) {
+        error_setg(errp, "Can't find the snapshot");
         return -ENOENT;
     }
     sn = s->snapshots[snapshot_index];
@@ -580,6 +600,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     s->nb_snapshots--;
     ret = qcow2_write_snapshots(bs);
     if (ret < 0) {
+        error_setg(errp, "Failed to remove snapshot from snapshot list");
         return ret;
     }
 
@@ -597,6 +618,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     ret = qcow2_update_snapshot_refcount(bs, sn.l1_table_offset,
                                          sn.l1_size, -1);
     if (ret < 0) {
+        error_setg(errp, "Failed to free the cluster and L1 table");
         return ret;
     }
     qcow2_free_clusters(bs, sn.l1_table_offset, sn.l1_size * sizeof(uint64_t),
@@ -605,6 +627,7 @@ int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     /* must update the copied flag on the current cluster offsets */
     ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 0);
     if (ret < 0) {
+        error_setg(errp, "Failed to update snapshot status in disk");
         return ret;
     }
 
diff --git a/block/qcow2.h b/block/qcow2.h
index bea6ddb43a..c90e5d6c6e 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -467,7 +467,10 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs);
 /* qcow2-snapshot.c functions */
 int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
 int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id);
-int qcow2_snapshot_delete(BlockDriverState *bs, const char *snapshot_id);
+int qcow2_snapshot_delete(BlockDriverState *bs,
+                          const char *snapshot_id,
+                          const char *name,
+                          Error **errp);
 int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab);
 int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name);
 
diff --git a/block/rbd.c b/block/rbd.c
index e798e19f81..b1ab80ce19 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -891,12 +891,31 @@ static int qemu_rbd_snap_create(BlockDriverState *bs,
 }
 
 static int qemu_rbd_snap_remove(BlockDriverState *bs,
-                                const char *snapshot_name)
+                                const char *snapshot_id,
+                                const char *snapshot_name,
+                                Error **errp)
 {
     BDRVRBDState *s = bs->opaque;
     int r;
 
+    if (!snapshot_name) {
+        error_setg(errp, "rbd need a valid snapshot name");
+        return -EINVAL;
+    }
+
+    /* If snapshot_id is specified, it must be equal to name, see
+       qemu_rbd_snap_list() */
+    if (snapshot_id && strcmp(snapshot_id, snapshot_name)) {
+        error_setg(errp,
+                   "rbd do not support snapshot id, it should be NULL or "
+                   "equal to snapshot name");
+        return -EINVAL;
+    }
+
     r = rbd_snap_remove(s->image, snapshot_name);
+    if (r < 0) {
+        error_setg_errno(errp, -r, "Failed to remove the snapshot");
+    }
     return r;
 }
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index f9988d35ba..fe438e0fa2 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2072,7 +2072,10 @@ out:
     return ret;
 }
 
-static int sd_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+static int sd_snapshot_delete(BlockDriverState *bs,
+                              const char *snapshot_id,
+                              const char *name,
+                              Error **errp)
 {
     /* FIXME: Delete specified snapshot id.  */
     return 0;
diff --git a/block/snapshot.c b/block/snapshot.c
index a923b386b6..82e602fccf 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -182,21 +182,73 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     return -ENOTSUP;
 }
 
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
+/**
+ * Delete an internal snapshot by @snapshot_id and @name.
+ * @bs: block device used in the operation
+ * @snapshot_id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ * @errp: location to store error
+ *
+ * If both @snapshot_id and @name are specified, delete the first one with
+ * id @snapshot_id and name @name.
+ * If only @snapshot_id is specified, delete the first one with id
+ * @snapshot_id.
+ * If only @name is specified, delete the first one with name @name.
+ * if none is specified, return -ENINVAL.
+ *
+ * Returns: 0 on success, -errno on failure. If @bs is not inserted, return
+ * -ENOMEDIUM. If @snapshot_id and @name are both NULL, return -EINVAL. If @bs
+ * does not support internal snapshot deletion, return -ENOTSUP. If @bs does
+ * not support parameter @snapshot_id or @name, or one of them is not correctly
+ * specified, return -EINVAL. If @bs can't find one matching @id and @name,
+ * return -ENOENT. If @errp != NULL, it will always be filled with error
+ * message on failure.
+ */
+int bdrv_snapshot_delete(BlockDriverState *bs,
+                         const char *snapshot_id,
+                         const char *name,
+                         Error **errp)
 {
     BlockDriver *drv = bs->drv;
     if (!drv) {
+        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
         return -ENOMEDIUM;
     }
+    if (!snapshot_id && !name) {
+        error_setg(errp, "snapshot_id and name are both NULL");
+        return -EINVAL;
+    }
     if (drv->bdrv_snapshot_delete) {
-        return drv->bdrv_snapshot_delete(bs, snapshot_id);
+        return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
     }
     if (bs->file) {
-        return bdrv_snapshot_delete(bs->file, snapshot_id);
+        return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
     }
+    error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+              drv->format_name, bdrv_get_device_name(bs),
+              "internal snapshot deletion");
     return -ENOTSUP;
 }
 
+void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+                                        const char *id_or_name,
+                                        Error **errp)
+{
+    int ret;
+    Error *local_err = NULL;
+
+    ret = bdrv_snapshot_delete(bs, id_or_name, NULL, &local_err);
+    if (ret == -ENOENT || ret == -EINVAL) {
+        error_free(local_err);
+        local_err = NULL;
+        ret = bdrv_snapshot_delete(bs, NULL, id_or_name, &local_err);
+    }
+
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+    }
+}
+
 int bdrv_snapshot_list(BlockDriverState *bs,
                        QEMUSnapshotInfo **psn_info)
 {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1541777d42..e7d1766ae0 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -150,7 +150,10 @@ struct BlockDriver {
                                 QEMUSnapshotInfo *sn_info);
     int (*bdrv_snapshot_goto)(BlockDriverState *bs,
                               const char *snapshot_id);
-    int (*bdrv_snapshot_delete)(BlockDriverState *bs, const char *snapshot_id);
+    int (*bdrv_snapshot_delete)(BlockDriverState *bs,
+                                const char *snapshot_id,
+                                const char *name,
+                                Error **errp);
     int (*bdrv_snapshot_list)(BlockDriverState *bs,
                               QEMUSnapshotInfo **psn_info);
     int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 9d06dc17a6..012bf226d3 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -51,7 +51,13 @@ int bdrv_snapshot_create(BlockDriverState *bs,
                          QEMUSnapshotInfo *sn_info);
 int bdrv_snapshot_goto(BlockDriverState *bs,
                        const char *snapshot_id);
-int bdrv_snapshot_delete(BlockDriverState *bs, const char *snapshot_id);
+int bdrv_snapshot_delete(BlockDriverState *bs,
+                         const char *snapshot_id,
+                         const char *name,
+                         Error **errp);
+void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
+                                        const char *id_or_name,
+                                        Error **errp);
 int bdrv_snapshot_list(BlockDriverState *bs,
                        QEMUSnapshotInfo **psn_info);
 int bdrv_snapshot_load_tmp(BlockDriverState *bs,
diff --git a/qemu-img.c b/qemu-img.c
index 0cf6be2280..139526f348 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2006,6 +2006,7 @@ static int img_snapshot(int argc, char **argv)
     int action = 0;
     qemu_timeval tv;
     bool quiet = false;
+    Error *err = NULL;
 
     bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
     /* Parse commandline parameters */
@@ -2098,10 +2099,12 @@ static int img_snapshot(int argc, char **argv)
         break;
 
     case SNAPSHOT_DELETE:
-        ret = bdrv_snapshot_delete(bs, snapshot_name);
-        if (ret) {
-            error_report("Could not delete snapshot '%s': %d (%s)",
-                snapshot_name, ret, strerror(-ret));
+        bdrv_snapshot_delete_by_id_or_name(bs, snapshot_name, &err);
+        if (error_is_set(&err)) {
+            error_report("Could not delete snapshot '%s': (%s)",
+                         snapshot_name, error_get_pretty(err));
+            error_free(err);
+            ret = 1;
         }
         break;
     }
diff --git a/savevm.c b/savevm.c
index c536aa4986..4a3c819fcd 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2325,18 +2325,21 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
 {
     BlockDriverState *bs;
     QEMUSnapshotInfo sn1, *snapshot = &sn1;
-    int ret;
+    Error *err = NULL;
 
     bs = NULL;
     while ((bs = bdrv_next(bs))) {
         if (bdrv_can_snapshot(bs) &&
             bdrv_snapshot_find(bs, snapshot, name) >= 0)
         {
-            ret = bdrv_snapshot_delete(bs, name);
-            if (ret < 0) {
+            bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
+            if (error_is_set(&err)) {
                 monitor_printf(mon,
-                               "Error while deleting snapshot on '%s'\n",
-                               bdrv_get_device_name(bs));
+                               "Error while deleting snapshot on device '%s':"
+                               " %s\n",
+                               bdrv_get_device_name(bs),
+                               error_get_pretty(err));
+                error_free(err);
                 return -1;
             }
         }
@@ -2550,7 +2553,7 @@ int load_vmstate(const char *name)
 void do_delvm(Monitor *mon, const QDict *qdict)
 {
     BlockDriverState *bs, *bs1;
-    int ret;
+    Error *err = NULL;
     const char *name = qdict_get_str(qdict, "name");
 
     bs = find_vmstate_bs();
@@ -2562,15 +2565,14 @@ void do_delvm(Monitor *mon, const QDict *qdict)
     bs1 = NULL;
     while ((bs1 = bdrv_next(bs1))) {
         if (bdrv_can_snapshot(bs1)) {
-            ret = bdrv_snapshot_delete(bs1, name);
-            if (ret < 0) {
-                if (ret == -ENOTSUP)
-                    monitor_printf(mon,
-                                   "Snapshots not supported on device '%s'\n",
-                                   bdrv_get_device_name(bs1));
-                else
-                    monitor_printf(mon, "Error %d while deleting snapshot on "
-                                   "'%s'\n", ret, bdrv_get_device_name(bs1));
+            bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
+            if (error_is_set(&err)) {
+                monitor_printf(mon,
+                               "Error while deleting snapshot on device '%s':"
+                               " %s\n",
+                               bdrv_get_device_name(bs),
+                               error_get_pretty(err));
+                error_free(err);
             }
         }
     }

From bbe860104f0544d7863296606e042cc62bf7ab4b Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:34 +0800
Subject: [PATCH 16/33] qmp: add internal snapshot support in qmp_transaction

Unlike savevm, the qmp_transaction interface will not generate
snapshot name automatically, saving trouble to return information
of the new created snapshot.

Although qcow2 support storing multiple snapshots with same name
but different ID, here it will fail when an snapshot with that name
already exist before the operation. Format such as rbd do not support
ID at all, and in most case, it means trouble to user when he faces
multiple snapshots with same name, so ban that case. Request with
empty name will be rejected.

Snapshot ID can't be specified in this interface.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c       | 116 +++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |  19 +++++++-
 qmp-commands.hx  |  34 ++++++++++----
 3 files changed, 160 insertions(+), 9 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 07dac05a2c..0fd30e2d8d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -889,6 +889,117 @@ struct BlkTransactionState {
     QSIMPLEQ_ENTRY(BlkTransactionState) entry;
 };
 
+/* internal snapshot private data */
+typedef struct InternalSnapshotState {
+    BlkTransactionState common;
+    BlockDriverState *bs;
+    QEMUSnapshotInfo sn;
+} InternalSnapshotState;
+
+static void internal_snapshot_prepare(BlkTransactionState *common,
+                                      Error **errp)
+{
+    const char *device;
+    const char *name;
+    BlockDriverState *bs;
+    QEMUSnapshotInfo old_sn, *sn;
+    bool ret;
+    qemu_timeval tv;
+    BlockdevSnapshotInternal *internal;
+    InternalSnapshotState *state;
+    int ret1;
+
+    g_assert(common->action->kind ==
+             TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC);
+    internal = common->action->blockdev_snapshot_internal_sync;
+    state = DO_UPCAST(InternalSnapshotState, common, common);
+
+    /* 1. parse input */
+    device = internal->device;
+    name = internal->name;
+
+    /* 2. check for validation */
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (!bdrv_is_inserted(bs)) {
+        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+        return;
+    }
+
+    if (bdrv_is_read_only(bs)) {
+        error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
+        return;
+    }
+
+    if (!bdrv_can_snapshot(bs)) {
+        error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+                  bs->drv->format_name, device, "internal snapshot");
+        return;
+    }
+
+    if (!strlen(name)) {
+        error_setg(errp, "Name is empty");
+        return;
+    }
+
+    /* check whether a snapshot with name exist */
+    ret = bdrv_snapshot_find_by_id_and_name(bs, NULL, name, &old_sn, errp);
+    if (error_is_set(errp)) {
+        return;
+    } else if (ret) {
+        error_setg(errp,
+                   "Snapshot with name '%s' already exists on device '%s'",
+                   name, device);
+        return;
+    }
+
+    /* 3. take the snapshot */
+    sn = &state->sn;
+    pstrcpy(sn->name, sizeof(sn->name), name);
+    qemu_gettimeofday(&tv);
+    sn->date_sec = tv.tv_sec;
+    sn->date_nsec = tv.tv_usec * 1000;
+    sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+
+    ret1 = bdrv_snapshot_create(bs, sn);
+    if (ret1 < 0) {
+        error_setg_errno(errp, -ret1,
+                         "Failed to create snapshot '%s' on device '%s'",
+                         name, device);
+        return;
+    }
+
+    /* 4. succeed, mark a snapshot is created */
+    state->bs = bs;
+}
+
+static void internal_snapshot_abort(BlkTransactionState *common)
+{
+    InternalSnapshotState *state =
+                             DO_UPCAST(InternalSnapshotState, common, common);
+    BlockDriverState *bs = state->bs;
+    QEMUSnapshotInfo *sn = &state->sn;
+    Error *local_error = NULL;
+
+    if (!bs) {
+        return;
+    }
+
+    if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
+        error_report("Failed to delete snapshot with id '%s' and name '%s' on "
+                     "device '%s' in abort: %s",
+                     sn->id_str,
+                     sn->name,
+                     bdrv_get_device_name(bs),
+                     error_get_pretty(local_error));
+        error_free(local_error);
+    }
+}
+
 /* external snapshot private data */
 typedef struct ExternalSnapshotState {
     BlkTransactionState common;
@@ -1072,6 +1183,11 @@ static const BdrvActionOps actions[] = {
         .prepare = abort_prepare,
         .commit = abort_commit,
     },
+    [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC] = {
+        .instance_size = sizeof(InternalSnapshotState),
+        .prepare  = internal_snapshot_prepare,
+        .abort = internal_snapshot_abort,
+    },
 };
 
 /*
diff --git a/qapi-schema.json b/qapi-schema.json
index 2b2c8bce07..77bbbf59cf 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1685,6 +1685,22 @@
   'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
             '*mode': 'NewImageMode' } }
 
+##
+# @BlockdevSnapshotInternal
+#
+# @device: the name of the device to generate the snapshot from
+#
+# @name: the name of the internal snapshot to be created
+#
+# Notes: In transaction, if @name is empty, or any snapshot matching @name
+#        exists, the operation will fail. Only some image formats support it,
+#        for example, qcow2, rbd, and sheepdog.
+#
+# Since: 1.7
+##
+{ 'type': 'BlockdevSnapshotInternal',
+  'data': { 'device': 'str', 'name': 'str' } }
+
 ##
 # @DriveBackup
 #
@@ -1747,7 +1763,8 @@
   'data': {
        'blockdev-snapshot-sync': 'BlockdevSnapshot',
        'drive-backup': 'DriveBackup',
-       'abort': 'Abort'
+       'abort': 'Abort',
+       'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
    } }
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 008cad95a2..66701927af 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1001,14 +1001,15 @@ SQMP
 transaction
 -----------
 
-Atomically operate on one or more block devices.  The only supported
-operation for now is snapshotting.  If there is any failure performing
-any of the operations, all snapshots for the group are abandoned, and
-the original disks pre-snapshot attempt are used.
+Atomically operate on one or more block devices.  The only supported operations
+for now are drive-backup, internal and external snapshotting.  A list of
+dictionaries is accepted, that contains the actions to be performed.
+If there is any failure performing any of the operations, all operations
+for the group are abandoned.
 
-A list of dictionaries is accepted, that contains the actions to be performed.
-For snapshots this is the device, the file to use for the new snapshot,
-and the format.  The default format, if not specified, is qcow2.
+For external snapshots, the dictionary contains the device, the file to use for
+the new snapshot, and the format.  The default format, if not specified, is
+qcow2.
 
 Each new snapshot defaults to being created by QEMU (wiping any
 contents if the file already exists), but it is also possible to reuse
@@ -1017,6 +1018,17 @@ the new image file has the same contents as the current one; QEMU cannot
 perform any meaningful check.  Typically this is achieved by using the
 current image file as the backing file for the new image.
 
+On failure, the original disks pre-snapshot attempt will be used.
+
+For internal snapshots, the dictionary contains the device and the snapshot's
+name.  If an internal snapshot matching name already exists, the request will
+be rejected.  Only some image formats support it, for example, qcow2, rbd,
+and sheepdog.
+
+On failure, qemu will try delete the newly created internal snapshot in the
+transaction.  When an I/O error occurs during deletion, the user needs to fix
+it later with qemu-img or other command.
+
 Arguments:
 
 actions array:
@@ -1029,6 +1041,9 @@ actions array:
       - "format": format of new image (json-string, optional)
       - "mode": whether and how QEMU should create the snapshot file
         (NewImageMode, optional, default "absolute-paths")
+      When "type" is "blockdev-snapshot-internal-sync":
+      - "device": device name to snapshot (json-string)
+      - "name": name of the new snapshot (json-string)
 
 Example:
 
@@ -1040,7 +1055,10 @@ Example:
          { 'type': 'blockdev-snapshot-sync', 'data' : { "device": "ide-hd1",
                                          "snapshot-file": "/some/place/my-image2",
                                          "mode": "existing",
-                                         "format": "qcow2" } } ] } }
+                                         "format": "qcow2" } },
+         { 'type': 'blockdev-snapshot-internal-sync', 'data' : {
+                                         "device": "ide-hd2",
+                                         "name": "snapshot0" } } ] } }
 <- { "return": {} }
 
 EQMP

From f323bc9e8b3b46ad28402995a9dcaaeff3eb5e03 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:35 +0800
Subject: [PATCH 17/33] qmp: add interface blockdev-snapshot-internal-sync

Snapshot ID can't be specified in this interface.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c       | 13 +++++++++++++
 qapi-schema.json | 20 ++++++++++++++++++++
 qmp-commands.hx  | 29 +++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 0fd30e2d8d..0f4a7b5d85 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -858,6 +858,19 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file,
                        &snapshot, errp);
 }
 
+void qmp_blockdev_snapshot_internal_sync(const char *device,
+                                         const char *name,
+                                         Error **errp)
+{
+    BlockdevSnapshotInternal snapshot = {
+        .device = (char *) device,
+        .name = (char *) name
+    };
+
+    blockdev_do_action(TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_INTERNAL_SYNC,
+                       &snapshot, errp);
+}
+
 
 /* New and old BlockDriverState structs for group snapshots */
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 77bbbf59cf..43faf91410 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1804,6 +1804,26 @@
 { 'command': 'blockdev-snapshot-sync',
   'data': 'BlockdevSnapshot' }
 
+##
+# @blockdev-snapshot-internal-sync
+#
+# Synchronously take an internal snapshot of a block device, when the format
+# of the image used supports it.
+#
+# For the arguments, see the documentation of BlockdevSnapshotInternal.
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If any snapshot matching @name exists, or @name is empty,
+#          GenericError
+#          If the format of the image used does not support it,
+#          BlockFormatFeatureNotSupported
+#
+# Since 1.7
+##
+{ 'command': 'blockdev-snapshot-internal-sync',
+  'data': 'BlockdevSnapshotInternal' }
+
 ##
 # @human-monitor-command:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 66701927af..5c9ddef3a5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1095,6 +1095,35 @@ Example:
                                                         "format": "qcow2" } }
 <- { "return": {} }
 
+EQMP
+
+    {
+        .name       = "blockdev-snapshot-internal-sync",
+        .args_type  = "device:B,name:s",
+        .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_internal_sync,
+    },
+
+SQMP
+blockdev-snapshot-internal-sync
+-------------------------------
+
+Synchronously take an internal snapshot of a block device when the format of
+image used supports it.  If the name is an empty string, or a snapshot with
+name already exists, the operation will fail.
+
+Arguments:
+
+- "device": device name to snapshot (json-string)
+- "name": name of the new snapshot (json-string)
+
+Example:
+
+-> { "execute": "blockdev-snapshot-internal-sync",
+                "arguments": { "device": "ide-hd0",
+                               "name": "snapshot0" }
+   }
+<- { "return": {} }
+
 EQMP
 
     {

From 44e3e053af56c1faec79ac9fdd78ef3f0ecd467e Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:36 +0800
Subject: [PATCH 18/33] qmp: add interface
 blockdev-snapshot-delete-internal-sync

This interface use id and name as optional parameters, to handle the
case that one image contain multiple snapshots with same name which
may be '', but with different id.

Adding parameter id is for historical compatiability reason, and
that case is not possible in qemu's new interface for internal
snapshot at block device level, but still possible in qemu-img.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c            | 61 +++++++++++++++++++++++++++++++++++++++++++
 include/qemu-common.h |  3 +++
 qapi-schema.json      | 27 +++++++++++++++++++
 qmp-commands.hx       | 41 +++++++++++++++++++++++++++++
 4 files changed, 132 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 0f4a7b5d85..84d55c1adc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -871,6 +871,67 @@ void qmp_blockdev_snapshot_internal_sync(const char *device,
                        &snapshot, errp);
 }
 
+SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
+                                                         bool has_id,
+                                                         const char *id,
+                                                         bool has_name,
+                                                         const char *name,
+                                                         Error **errp)
+{
+    BlockDriverState *bs = bdrv_find(device);
+    QEMUSnapshotInfo sn;
+    Error *local_err = NULL;
+    SnapshotInfo *info = NULL;
+    int ret;
+
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return NULL;
+    }
+
+    if (!has_id) {
+        id = NULL;
+    }
+
+    if (!has_name) {
+        name = NULL;
+    }
+
+    if (!id && !name) {
+        error_setg(errp, "Name or id must be provided");
+        return NULL;
+    }
+
+    ret = bdrv_snapshot_find_by_id_and_name(bs, id, name, &sn, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+        return NULL;
+    }
+    if (!ret) {
+        error_setg(errp,
+                   "Snapshot with id '%s' and name '%s' does not exist on "
+                   "device '%s'",
+                   STR_OR_NULL(id), STR_OR_NULL(name), device);
+        return NULL;
+    }
+
+    bdrv_snapshot_delete(bs, id, name, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+        return NULL;
+    }
+
+    info = g_malloc0(sizeof(SnapshotInfo));
+    info->id = g_strdup(sn.id_str);
+    info->name = g_strdup(sn.name);
+    info->date_nsec = sn.date_nsec;
+    info->date_sec = sn.date_sec;
+    info->vm_state_size = sn.vm_state_size;
+    info->vm_clock_nsec = sn.vm_clock_nsec % 1000000000;
+    info->vm_clock_sec = sn.vm_clock_nsec / 1000000000;
+
+    return info;
+}
 
 /* New and old BlockDriverState structs for group snapshots */
 
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 6948bb9177..50548361d0 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -191,6 +191,9 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
 int64_t strtosz_suffix_unit(const char *nptr, char **end,
                             const char default_suffix, int64_t unit);
 
+/* used to print char* safely */
+#define STR_OR_NULL(str) ((str) ? (str) : "null")
+
 /* path.c */
 void init_paths(const char *prefix);
 const char *path(const char *pathname);
diff --git a/qapi-schema.json b/qapi-schema.json
index 43faf91410..145eca8855 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1824,6 +1824,33 @@
 { 'command': 'blockdev-snapshot-internal-sync',
   'data': 'BlockdevSnapshotInternal' }
 
+##
+# @blockdev-snapshot-delete-internal-sync
+#
+# Synchronously delete an internal snapshot of a block device, when the format
+# of the image used support it. The snapshot is identified by name or id or
+# both. One of the name or id is required. Return SnapshotInfo for the
+# successfully deleted snapshot.
+#
+# @device: the name of the device to delete the snapshot from
+#
+# @id: optional the snapshot's ID to be deleted
+#
+# @name: optional the snapshot's name to be deleted
+#
+# Returns: SnapshotInfo on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If snapshot not found, GenericError
+#          If the format of the image used does not support it,
+#          BlockFormatFeatureNotSupported
+#          If @id and @name are both not specified, GenericError
+#
+# Since 1.7
+##
+{ 'command': 'blockdev-snapshot-delete-internal-sync',
+  'data': { 'device': 'str', '*id': 'str', '*name': 'str'},
+  'returns': 'SnapshotInfo' }
+
 ##
 # @human-monitor-command:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5c9ddef3a5..b17c46e0b1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1124,6 +1124,47 @@ Example:
    }
 <- { "return": {} }
 
+EQMP
+
+    {
+        .name       = "blockdev-snapshot-delete-internal-sync",
+        .args_type  = "device:B,id:s?,name:s?",
+        .mhandler.cmd_new =
+                      qmp_marshal_input_blockdev_snapshot_delete_internal_sync,
+    },
+
+SQMP
+blockdev-snapshot-delete-internal-sync
+--------------------------------------
+
+Synchronously delete an internal snapshot of a block device when the format of
+image used supports it.  The snapshot is identified by name or id or both.  One
+of name or id is required.  If the snapshot is not found, the operation will
+fail.
+
+Arguments:
+
+- "device": device name (json-string)
+- "id": ID of the snapshot (json-string, optional)
+- "name": name of the snapshot (json-string, optional)
+
+Example:
+
+-> { "execute": "blockdev-snapshot-delete-internal-sync",
+                "arguments": { "device": "ide-hd0",
+                               "name": "snapshot0" }
+   }
+<- { "return": {
+                   "id": "1",
+                   "name": "snapshot0",
+                   "vm-state-size": 0,
+                   "date-sec": 1000012,
+                   "date-nsec": 10,
+                   "vm-clock-sec": 100,
+                   "vm-clock-nsec": 20
+     }
+   }
+
 EQMP
 
     {

From 775ca88e8205248929a3f141925eafe2ec1e3275 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:37 +0800
Subject: [PATCH 19/33] hmp: add interface hmp_snapshot_blkdev_internal

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hmp-commands.hx | 19 +++++++++++++++++--
 hmp.c           | 10 ++++++++++
 hmp.h           |  1 +
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 65b7f6076c..f19a914aaf 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1023,8 +1023,7 @@ ETEXI
                       "of device. If a new image file is specified, the\n\t\t\t"
                       "new image file will become the new root image.\n\t\t\t"
                       "If format is specified, the snapshot file will\n\t\t\t"
-                      "be created in that format. Otherwise the\n\t\t\t"
-                      "snapshot will be internal! (currently unsupported).\n\t\t\t"
+                      "be created in that format.\n\t\t\t"
                       "The default format is qcow2.  The -n flag requests QEMU\n\t\t\t"
                       "to reuse the image found in new-image-file, instead of\n\t\t\t"
                       "recreating it from scratch.",
@@ -1035,6 +1034,22 @@ STEXI
 @item snapshot_blkdev
 @findex snapshot_blkdev
 Snapshot device, using snapshot file as target if provided
+ETEXI
+
+    {
+        .name       = "snapshot_blkdev_internal",
+        .args_type  = "device:B,name:s",
+        .params     = "device name",
+        .help       = "take an internal snapshot of device.\n\t\t\t"
+                      "The format of the image used by device must\n\t\t\t"
+                      "support it, such as qcow2.\n\t\t\t",
+        .mhandler.cmd = hmp_snapshot_blkdev_internal,
+    },
+
+STEXI
+@item snapshot_blkdev_internal
+@findex snapshot_blkdev_internal
+Take an internal snapshot on device if it support
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index b4a6422e7a..568bb91452 100644
--- a/hmp.c
+++ b/hmp.c
@@ -978,6 +978,16 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &errp);
 }
 
+void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *name = qdict_get_str(qdict, "name");
+    Error *errp = NULL;
+
+    qmp_blockdev_snapshot_internal_sync(device, name, &errp);
+    hmp_handle_error(mon, &errp);
+}
+
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
 {
     qmp_migrate_cancel(NULL);
diff --git a/hmp.h b/hmp.h
index 6c3bdcd4c2..cc1ca58e8f 100644
--- a/hmp.h
+++ b/hmp.h
@@ -54,6 +54,7 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict);
 void hmp_balloon(Monitor *mon, const QDict *qdict);
 void hmp_block_resize(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
+void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
 void hmp_drive_backup(Monitor *mon, const QDict *qdict);
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);

From 7a4ed2ee4215eee80d2ae02097bbf63a33748fda Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:38 +0800
Subject: [PATCH 20/33] hmp: add interface hmp_snapshot_delete_blkdev_internal

It is hard to make both id and name optional in hmp console as qmp
interface, so this interface require user to specify name.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hmp-commands.hx | 18 ++++++++++++++++++
 hmp.c           | 12 ++++++++++++
 hmp.h           |  1 +
 3 files changed, 31 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f19a914aaf..caae5ad9e9 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1050,6 +1050,24 @@ STEXI
 @item snapshot_blkdev_internal
 @findex snapshot_blkdev_internal
 Take an internal snapshot on device if it support
+ETEXI
+
+    {
+        .name       = "snapshot_delete_blkdev_internal",
+        .args_type  = "device:B,name:s,id:s?",
+        .params     = "device name [id]",
+        .help       = "delete an internal snapshot of device.\n\t\t\t"
+                      "If id is specified, qemu will try delete\n\t\t\t"
+                      "the snapshot matching both id and name.\n\t\t\t"
+                      "The format of the image used by device must\n\t\t\t"
+                      "support it, such as qcow2.\n\t\t\t",
+        .mhandler.cmd = hmp_snapshot_delete_blkdev_internal,
+    },
+
+STEXI
+@item snapshot_delete_blkdev_internal
+@findex snapshot_delete_blkdev_internal
+Delete an internal snapshot on device if it support
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index 568bb91452..2a902951df 100644
--- a/hmp.c
+++ b/hmp.c
@@ -988,6 +988,18 @@ void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &errp);
 }
 
+void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    const char *name = qdict_get_str(qdict, "name");
+    const char *id = qdict_get_try_str(qdict, "id");
+    Error *errp = NULL;
+
+    qmp_blockdev_snapshot_delete_internal_sync(device, !!id, id,
+                                               true, name, &errp);
+    hmp_handle_error(mon, &errp);
+}
+
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
 {
     qmp_migrate_cancel(NULL);
diff --git a/hmp.h b/hmp.h
index cc1ca58e8f..54cf71fb94 100644
--- a/hmp.h
+++ b/hmp.h
@@ -55,6 +55,7 @@ void hmp_balloon(Monitor *mon, const QDict *qdict);
 void hmp_block_resize(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
+void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
 void hmp_drive_backup(Monitor *mon, const QDict *qdict);
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);

From 8023090be592514dea4975428543edd598c65fc3 Mon Sep 17 00:00:00 2001
From: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Date: Wed, 11 Sep 2013 14:04:39 +0800
Subject: [PATCH 21/33] qemu-iotests: add 057 internal snapshot for block
 device test case

Create in transaction and deletion in single command will be tested.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/057     | 259 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/057.out |   5 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 265 insertions(+)
 create mode 100755 tests/qemu-iotests/057
 create mode 100644 tests/qemu-iotests/057.out

diff --git a/tests/qemu-iotests/057 b/tests/qemu-iotests/057
new file mode 100755
index 0000000000..9cdd582e39
--- /dev/null
+++ b/tests/qemu-iotests/057
@@ -0,0 +1,259 @@
+#!/usr/bin/env python
+#
+# Tests for internal snapshot.
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# Based on 055.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import time
+import os
+import iotests
+from iotests import qemu_img, qemu_io
+
+test_drv_base_name = 'drive'
+
+class ImageSnapshotTestCase(iotests.QMPTestCase):
+    image_len = 120 * 1024 * 1024 # MB
+
+    def __init__(self, *args):
+        self.expect = []
+        super(ImageSnapshotTestCase, self).__init__(*args)
+
+    def _setUp(self, test_img_base_name, image_num):
+        self.vm = iotests.VM()
+        for i in range(0, image_num):
+            filename = '%s%d' % (test_img_base_name, i)
+            img = os.path.join(iotests.test_dir, filename)
+            device = '%s%d' % (test_drv_base_name, i)
+            qemu_img('create', '-f', iotests.imgfmt, img, str(self.image_len))
+            self.vm.add_drive(img)
+            self.expect.append({'image': img, 'device': device,
+                                'snapshots': [],
+                                'snapshots_name_counter': 0})
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        for dev_expect in self.expect:
+            os.remove(dev_expect['image'])
+
+    def createSnapshotInTransaction(self, snapshot_num, abort = False):
+        actions = []
+        for dev_expect in self.expect:
+            num = dev_expect['snapshots_name_counter']
+            for j in range(0, snapshot_num):
+                name = '%s_sn%d' % (dev_expect['device'], num)
+                num = num + 1
+                if abort == False:
+                    dev_expect['snapshots'].append({'name': name})
+                    dev_expect['snapshots_name_counter'] = num
+                actions.append({
+                    'type': 'blockdev-snapshot-internal-sync',
+                    'data': { 'device': dev_expect['device'],
+                              'name': name },
+                })
+
+        if abort == True:
+            actions.append({
+                'type': 'abort',
+                'data': {},
+            })
+
+        result = self.vm.qmp('transaction', actions = actions)
+
+        if abort == True:
+            self.assert_qmp(result, 'error/class', 'GenericError')
+        else:
+            self.assert_qmp(result, 'return', {})
+
+    def verifySnapshotInfo(self):
+        result = self.vm.qmp('query-block')
+
+        # Verify each expected result
+        for dev_expect in self.expect:
+            # 1. Find the returned image value and snapshot info
+            image_result = None
+            for device in result['return']:
+                if device['device'] == dev_expect['device']:
+                    image_result = device['inserted']['image']
+                    break
+            self.assertTrue(image_result != None)
+            # Do not consider zero snapshot case now
+            sn_list_result = image_result['snapshots']
+            sn_list_expect = dev_expect['snapshots']
+
+            # 2. Verify it with expect
+            self.assertTrue(len(sn_list_result) == len(sn_list_expect))
+
+            for sn_expect in sn_list_expect:
+                sn_result = None
+                for sn in sn_list_result:
+                    if sn_expect['name'] == sn['name']:
+                        sn_result = sn
+                        break
+                self.assertTrue(sn_result != None)
+                # Fill in the detail info
+                sn_expect.update(sn_result)
+
+    def deleteSnapshot(self, device, id = None, name = None):
+        sn_list_expect = None
+        sn_expect = None
+
+        self.assertTrue(id != None or name != None)
+
+        # Fill in the detail info include ID
+        self.verifySnapshotInfo()
+
+        #find the expected snapshot list
+        for dev_expect in self.expect:
+            if dev_expect['device'] == device:
+                sn_list_expect = dev_expect['snapshots']
+                break
+        self.assertTrue(sn_list_expect != None)
+
+        if id != None and name != None:
+            for sn in sn_list_expect:
+                if sn['id'] == id and sn['name'] == name:
+                    sn_expect = sn
+                    result = \
+                          self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+                                      device = device,
+                                      id = id,
+                                      name = name)
+                    break
+        elif id != None:
+            for sn in sn_list_expect:
+                if sn['id'] == id:
+                    sn_expect = sn
+                    result = \
+                          self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+                                      device = device,
+                                      id = id)
+                    break
+        else:
+            for sn in sn_list_expect:
+                if sn['name'] == name:
+                    sn_expect = sn
+                    result = \
+                          self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+                                      device = device,
+                                      name = name)
+                    break
+
+        self.assertTrue(sn_expect != None)
+
+        self.assert_qmp(result, 'return', sn_expect)
+        sn_list_expect.remove(sn_expect)
+
+class TestSingleTransaction(ImageSnapshotTestCase):
+    def setUp(self):
+        self._setUp('test_a.img', 1)
+
+    def test_create(self):
+        self.createSnapshotInTransaction(1)
+        self.verifySnapshotInfo()
+
+    def test_error_name_empty(self):
+        actions = [{'type': 'blockdev-snapshot-internal-sync',
+                    'data': { 'device': self.expect[0]['device'],
+                              'name': '' },
+                  }]
+        result = self.vm.qmp('transaction', actions = actions)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_error_device(self):
+        actions = [{'type': 'blockdev-snapshot-internal-sync',
+                    'data': { 'device': 'drive_error',
+                              'name': 'a' },
+                  }]
+        result = self.vm.qmp('transaction', actions = actions)
+        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+    def test_error_exist(self):
+        self.createSnapshotInTransaction(1)
+        self.verifySnapshotInfo()
+        actions = [{'type': 'blockdev-snapshot-internal-sync',
+                    'data': { 'device': self.expect[0]['device'],
+                              'name': self.expect[0]['snapshots'][0] },
+                  }]
+        result = self.vm.qmp('transaction', actions = actions)
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+class TestMultipleTransaction(ImageSnapshotTestCase):
+    def setUp(self):
+        self._setUp('test_b.img', 2)
+
+    def test_create(self):
+        self.createSnapshotInTransaction(3)
+        self.verifySnapshotInfo()
+
+    def test_abort(self):
+        self.createSnapshotInTransaction(2)
+        self.verifySnapshotInfo()
+        self.createSnapshotInTransaction(3, abort = True)
+        self.verifySnapshotInfo()
+
+class TestSnapshotDelete(ImageSnapshotTestCase):
+    def setUp(self):
+        self._setUp('test_c.img', 1)
+
+    def test_delete_with_id(self):
+        self.createSnapshotInTransaction(2)
+        self.verifySnapshotInfo()
+        self.deleteSnapshot(self.expect[0]['device'],
+                            id = self.expect[0]['snapshots'][0]['id'])
+        self.verifySnapshotInfo()
+
+    def test_delete_with_name(self):
+        self.createSnapshotInTransaction(3)
+        self.verifySnapshotInfo()
+        self.deleteSnapshot(self.expect[0]['device'],
+                            name = self.expect[0]['snapshots'][1]['name'])
+        self.verifySnapshotInfo()
+
+    def test_delete_with_id_and_name(self):
+        self.createSnapshotInTransaction(4)
+        self.verifySnapshotInfo()
+        self.deleteSnapshot(self.expect[0]['device'],
+                            id = self.expect[0]['snapshots'][2]['id'],
+                            name = self.expect[0]['snapshots'][2]['name'])
+        self.verifySnapshotInfo()
+
+
+    def test_error_device(self):
+        result = self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+                              device = 'drive_error',
+                              id = '0')
+        self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+    def test_error_no_id_and_name(self):
+        result = self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+                              device = self.expect[0]['device'])
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+    def test_error_snapshot_not_exist(self):
+        self.createSnapshotInTransaction(2)
+        self.verifySnapshotInfo()
+        result = self.vm.qmp('blockdev-snapshot-delete-internal-sync',
+                              device = self.expect[0]['device'],
+                              id = self.expect[0]['snapshots'][0]['id'],
+                              name = self.expect[0]['snapshots'][1]['name'])
+        self.assert_qmp(result, 'error/class', 'GenericError')
+
+if __name__ == '__main__':
+    iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/057.out b/tests/qemu-iotests/057.out
new file mode 100644
index 0000000000..281b69efea
--- /dev/null
+++ b/tests/qemu-iotests/057.out
@@ -0,0 +1,5 @@
+............
+----------------------------------------------------------------------
+Ran 12 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 8012828a17..1ad02e5a2c 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -63,6 +63,7 @@
 054 rw auto
 055 rw auto
 056 rw auto backing
+057 rw auto
 059 rw auto
 060 rw auto
 061 rw auto

From 015a1036a74ad29bb6916754911ce25587ff4db3 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Thu, 5 Sep 2013 14:22:29 +0200
Subject: [PATCH 22/33] bdrv: Use "Error" for opening images

Add an Error ** parameter to BlockDriver.bdrv_open and
BlockDriver.bdrv_file_open to allow more specific error messages.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c                   |  4 ++--
 block/blkdebug.c          |  3 ++-
 block/blkverify.c         |  3 ++-
 block/bochs.c             |  3 ++-
 block/cloop.c             |  3 ++-
 block/cow.c               |  3 ++-
 block/curl.c              |  3 ++-
 block/dmg.c               |  3 ++-
 block/gluster.c           |  2 +-
 block/iscsi.c             |  5 +++--
 block/nbd.c               |  3 ++-
 block/parallels.c         |  3 ++-
 block/qcow.c              |  3 ++-
 block/qcow2.c             |  5 +++--
 block/qed.c               |  5 +++--
 block/raw-posix.c         | 12 ++++++++----
 block/raw-win32.c         |  6 ++++--
 block/raw_bsd.c           |  3 ++-
 block/rbd.c               |  3 ++-
 block/sheepdog.c          |  3 ++-
 block/snapshot.c          |  2 +-
 block/ssh.c               |  3 ++-
 block/vdi.c               |  3 ++-
 block/vhdx.c              |  3 ++-
 block/vmdk.c              |  3 ++-
 block/vpc.c               |  3 ++-
 block/vvfat.c             |  3 ++-
 include/block/block_int.h |  6 ++++--
 28 files changed, 67 insertions(+), 37 deletions(-)

diff --git a/block.c b/block.c
index b81d1e210a..89098bc6b0 100644
--- a/block.c
+++ b/block.c
@@ -761,7 +761,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     if (drv->bdrv_file_open) {
         assert(file == NULL);
         assert(drv->bdrv_parse_filename || filename != NULL);
-        ret = drv->bdrv_file_open(bs, options, open_flags);
+        ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
     } else {
         if (file == NULL) {
             qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
@@ -771,7 +771,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
             goto free_and_fail;
         }
         bs->file = file;
-        ret = drv->bdrv_open(bs, options, open_flags);
+        ret = drv->bdrv_open(bs, options, open_flags, NULL);
     }
 
     if (ret < 0) {
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 5d33e03608..52d65ffcd4 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -350,7 +350,8 @@ static QemuOptsList runtime_opts = {
     },
 };
 
-static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags)
+static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
+                         Error **errp)
 {
     BDRVBlkdebugState *s = bs->opaque;
     QemuOpts *opts;
diff --git a/block/blkverify.c b/block/blkverify.c
index c4e961eeb1..2093391c8b 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -116,7 +116,8 @@ static QemuOptsList runtime_opts = {
     },
 };
 
-static int blkverify_open(BlockDriverState *bs, QDict *options, int flags)
+static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
+                          Error **errp)
 {
     BDRVBlkverifyState *s = bs->opaque;
     QemuOpts *opts;
diff --git a/block/bochs.c b/block/bochs.c
index d7078c0775..51d9a90577 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -108,7 +108,8 @@ static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
     return 0;
 }
 
-static int bochs_open(BlockDriverState *bs, QDict *options, int flags)
+static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
 {
     BDRVBochsState *s = bs->opaque;
     int i;
diff --git a/block/cloop.c b/block/cloop.c
index 6ea7cf4046..b907023e10 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -53,7 +53,8 @@ static int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
     return 0;
 }
 
-static int cloop_open(BlockDriverState *bs, QDict *options, int flags)
+static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
 {
     BDRVCloopState *s = bs->opaque;
     uint32_t offsets_size, max_compressed_block_size = 1, i;
diff --git a/block/cow.c b/block/cow.c
index 764b93fae0..3ae1b5cdc9 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -58,7 +58,8 @@ static int cow_probe(const uint8_t *buf, int buf_size, const char *filename)
         return 0;
 }
 
-static int cow_open(BlockDriverState *bs, QDict *options, int flags)
+static int cow_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVCowState *s = bs->opaque;
     struct cow_header_v2 cow_header;
diff --git a/block/curl.c b/block/curl.c
index ca2cedcec1..5a46f9707c 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -395,7 +395,8 @@ static QemuOptsList runtime_opts = {
     },
 };
 
-static int curl_open(BlockDriverState *bs, QDict *options, int flags)
+static int curl_open(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp)
 {
     BDRVCURLState *s = bs->opaque;
     CURLState *state = NULL;
diff --git a/block/dmg.c b/block/dmg.c
index 3141cb5b88..d5e9b1ff01 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -92,7 +92,8 @@ static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
     return 0;
 }
 
-static int dmg_open(BlockDriverState *bs, QDict *options, int flags)
+static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVDMGState *s = bs->opaque;
     uint64_t info_begin,info_end,last_in_offset,last_out_offset;
diff --git a/block/gluster.c b/block/gluster.c
index dbb03f4de5..6c7b000db6 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -288,7 +288,7 @@ static QemuOptsList runtime_opts = {
 };
 
 static int qemu_gluster_open(BlockDriverState *bs,  QDict *options,
-                             int bdrv_flags)
+                             int bdrv_flags, Error **errp)
 {
     BDRVGlusterState *s = bs->opaque;
     int open_flags = O_BINARY;
diff --git a/block/iscsi.c b/block/iscsi.c
index 813abd8fef..5f26e7379c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1046,7 +1046,8 @@ static QemuOptsList runtime_opts = {
  * We support iscsi url's on the form
  * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
  */
-static int iscsi_open(BlockDriverState *bs, QDict *options, int flags)
+static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
 {
     IscsiLun *iscsilun = bs->opaque;
     struct iscsi_context *iscsi = NULL;
@@ -1260,7 +1261,7 @@ static int iscsi_create(const char *filename, QEMUOptionParameter *options)
 
     bs_options = qdict_new();
     qdict_put(bs_options, "filename", qstring_from_str(filename));
-    ret = iscsi_open(bs, bs_options, 0);
+    ret = iscsi_open(bs, bs_options, 0, NULL);
     QDECREF(bs_options);
 
     if (ret != 0) {
diff --git a/block/nbd.c b/block/nbd.c
index 691066f726..c8deeee67f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -453,7 +453,8 @@ static void nbd_teardown_connection(BlockDriverState *bs)
     closesocket(s->sock);
 }
 
-static int nbd_open(BlockDriverState *bs, QDict *options, int flags)
+static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVNBDState *s = bs->opaque;
     int result;
diff --git a/block/parallels.c b/block/parallels.c
index 18b3ac0b28..2121e43204 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -68,7 +68,8 @@ static int parallels_probe(const uint8_t *buf, int buf_size, const char *filenam
     return 0;
 }
 
-static int parallels_open(BlockDriverState *bs, QDict *options, int flags)
+static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
+                          Error **errp)
 {
     BDRVParallelsState *s = bs->opaque;
     int i;
diff --git a/block/qcow.c b/block/qcow.c
index 93a993bb44..2a1c9c9e87 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -92,7 +92,8 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
         return 0;
 }
 
-static int qcow_open(BlockDriverState *bs, QDict *options, int flags)
+static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
     int len, i, shift, ret;
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c9354cadf..eda085cfd8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -350,7 +350,8 @@ static QemuOptsList qcow2_runtime_opts = {
     },
 };
 
-static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
+static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
     int len, i, ret = 0;
@@ -1060,7 +1061,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs)
               qbool_from_int(s->use_lazy_refcounts));
 
     memset(s, 0, sizeof(BDRVQcowState));
-    qcow2_open(bs, options, flags);
+    qcow2_open(bs, options, flags, NULL);
 
     QDECREF(options);
 
diff --git a/block/qed.c b/block/qed.c
index 49b3a37ed5..69e1834ff6 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -373,7 +373,8 @@ static void bdrv_qed_rebind(BlockDriverState *bs)
     s->bs = bs;
 }
 
-static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags)
+static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
+                         Error **errp)
 {
     BDRVQEDState *s = bs->opaque;
     QEDHeader le_header;
@@ -1547,7 +1548,7 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs)
 
     bdrv_qed_close(bs);
     memset(s, 0, sizeof(BDRVQEDState));
-    bdrv_qed_open(bs, NULL, bs->open_flags);
+    bdrv_qed_open(bs, NULL, bs->open_flags, NULL);
 }
 
 static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 1b41ea3356..dcdf45c4ac 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -335,7 +335,8 @@ fail:
     return ret;
 }
 
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVRawState *s = bs->opaque;
 
@@ -1331,7 +1332,8 @@ static int check_hdev_writable(BDRVRawState *s)
     return 0;
 }
 
-static int hdev_open(BlockDriverState *bs, QDict *options, int flags)
+static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     int ret;
@@ -1565,7 +1567,8 @@ static BlockDriver bdrv_host_device = {
 };
 
 #ifdef __linux__
-static int floppy_open(BlockDriverState *bs, QDict *options, int flags)
+static int floppy_open(BlockDriverState *bs, QDict *options, int flags,
+                       Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     int ret;
@@ -1686,7 +1689,8 @@ static BlockDriver bdrv_host_floppy = {
     .bdrv_eject         = floppy_eject,
 };
 
-static int cdrom_open(BlockDriverState *bs, QDict *options, int flags)
+static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
 {
     BDRVRawState *s = bs->opaque;
 
diff --git a/block/raw-win32.c b/block/raw-win32.c
index ff3c5ea0d7..80551b957c 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -235,7 +235,8 @@ static QemuOptsList raw_runtime_opts = {
     },
 };
 
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     int access_flags;
@@ -532,7 +533,8 @@ static int hdev_probe_device(const char *filename)
     return 0;
 }
 
-static int hdev_open(BlockDriverState *bs, QDict *options, int flags)
+static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     int access_flags, create_flags;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index a9060caec4..2418f580cc 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -135,7 +135,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
     return bdrv_create_file(filename, options);
 }
 
-static int raw_open(BlockDriverState *bs, QDict *options, int flags)
+static int raw_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     bs->sg = bs->file->sg;
     return 0;
diff --git a/block/rbd.c b/block/rbd.c
index b1ab80ce19..b77f73ca89 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -446,7 +446,8 @@ static QemuOptsList runtime_opts = {
     },
 };
 
-static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags)
+static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
+                         Error **errp)
 {
     BDRVRBDState *s = bs->opaque;
     char pool[RBD_MAX_POOL_NAME_SIZE];
diff --git a/block/sheepdog.c b/block/sheepdog.c
index fe438e0fa2..1cfc6aedd5 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1242,7 +1242,8 @@ static QemuOptsList runtime_opts = {
     },
 };
 
-static int sd_open(BlockDriverState *bs, QDict *options, int flags)
+static int sd_open(BlockDriverState *bs, QDict *options, int flags,
+                   Error **errp)
 {
     int ret, fd;
     uint32_t vid = 0;
diff --git a/block/snapshot.c b/block/snapshot.c
index 82e602fccf..a05c0c0be0 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -170,7 +170,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
     if (bs->file) {
         drv->bdrv_close(bs);
         ret = bdrv_snapshot_goto(bs->file, snapshot_id);
-        open_ret = drv->bdrv_open(bs, NULL, bs->open_flags);
+        open_ret = drv->bdrv_open(bs, NULL, bs->open_flags, NULL);
         if (open_ret < 0) {
             bdrv_unref(bs->file);
             bs->drv = NULL;
diff --git a/block/ssh.c b/block/ssh.c
index 27691b4ad5..23cd1f07e5 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -608,7 +608,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
     return ret;
 }
 
-static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags)
+static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
+                         Error **errp)
 {
     BDRVSSHState *s = bs->opaque;
     int ret;
diff --git a/block/vdi.c b/block/vdi.c
index 1bf7dc575a..dedbafb539 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -364,7 +364,8 @@ static int vdi_probe(const uint8_t *buf, int buf_size, const char *filename)
     return result;
 }
 
-static int vdi_open(BlockDriverState *bs, QDict *options, int flags)
+static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVVdiState *s = bs->opaque;
     VdiHeader header;
diff --git a/block/vhdx.c b/block/vhdx.c
index e9704b1fdc..b8aa49ce4e 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -715,7 +715,8 @@ exit:
 }
 
 
-static int vhdx_open(BlockDriverState *bs, QDict *options, int flags)
+static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp)
 {
     BDRVVHDXState *s = bs->opaque;
     int ret = 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index fb5b5297ce..d60bf0cdf8 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -806,7 +806,8 @@ exit:
     return ret;
 }
 
-static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
+static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp)
 {
     int ret;
     BDRVVmdkState *s = bs->opaque;
diff --git a/block/vpc.c b/block/vpc.c
index fe4f311d50..ed00370bad 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -155,7 +155,8 @@ static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
     return 0;
 }
 
-static int vpc_open(BlockDriverState *bs, QDict *options, int flags)
+static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
+                    Error **errp)
 {
     BDRVVPCState *s = bs->opaque;
     int i;
diff --git a/block/vvfat.c b/block/vvfat.c
index 0129195e29..2f8be7cf1a 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1065,7 +1065,8 @@ static void vvfat_parse_filename(const char *filename, QDict *options,
     qdict_put(options, "rw", qbool_from_int(rw));
 }
 
-static int vvfat_open(BlockDriverState *bs, QDict *options, int flags)
+static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
+                      Error **errp)
 {
     BDRVVVFATState *s = bs->opaque;
     int cyls, heads, secs;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index e7d1766ae0..076b40aa43 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -80,8 +80,10 @@ struct BlockDriver {
     void (*bdrv_reopen_commit)(BDRVReopenState *reopen_state);
     void (*bdrv_reopen_abort)(BDRVReopenState *reopen_state);
 
-    int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags);
-    int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags);
+    int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags,
+                     Error **errp);
+    int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
+                          Error **errp);
     int (*bdrv_read)(BlockDriverState *bs, int64_t sector_num,
                      uint8_t *buf, int nb_sectors);
     int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,

From d5124c00d80b4d948509f2c7f6b54228d9981f75 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Thu, 5 Sep 2013 14:26:05 +0200
Subject: [PATCH 23/33] bdrv: Use "Error" for creating images

Add an Error ** parameter to BlockDriver.bdrv_create to allow more
specific error messages.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c                   | 2 +-
 block/cow.c               | 3 ++-
 block/gluster.c           | 2 +-
 block/iscsi.c             | 3 ++-
 block/qcow.c              | 3 ++-
 block/qcow2.c             | 3 ++-
 block/qed.c               | 3 ++-
 block/raw-posix.c         | 6 ++++--
 block/raw-win32.c         | 3 ++-
 block/raw_bsd.c           | 3 ++-
 block/rbd.c               | 3 ++-
 block/sheepdog.c          | 3 ++-
 block/ssh.c               | 3 ++-
 block/vdi.c               | 3 ++-
 block/vmdk.c              | 3 ++-
 block/vpc.c               | 3 ++-
 include/block/block_int.h | 3 ++-
 17 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/block.c b/block.c
index 89098bc6b0..f9b7033503 100644
--- a/block.c
+++ b/block.c
@@ -401,7 +401,7 @@ static void coroutine_fn bdrv_create_co_entry(void *opaque)
     CreateCo *cco = opaque;
     assert(cco->drv);
 
-    cco->ret = cco->drv->bdrv_create(cco->filename, cco->options);
+    cco->ret = cco->drv->bdrv_create(cco->filename, cco->options, NULL);
 }
 
 int bdrv_create(BlockDriver *drv, const char* filename,
diff --git a/block/cow.c b/block/cow.c
index 3ae1b5cdc9..e252c95601 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -295,7 +295,8 @@ static void cow_close(BlockDriverState *bs)
 {
 }
 
-static int cow_create(const char *filename, QEMUOptionParameter *options)
+static int cow_create(const char *filename, QEMUOptionParameter *options,
+                      Error **errp)
 {
     struct cow_header_v2 cow_header;
     struct stat st;
diff --git a/block/gluster.c b/block/gluster.c
index 6c7b000db6..256de10ed3 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -357,7 +357,7 @@ out:
 }
 
 static int qemu_gluster_create(const char *filename,
-        QEMUOptionParameter *options)
+        QEMUOptionParameter *options, Error **errp)
 {
     struct glfs *glfs;
     struct glfs_fd *fd;
diff --git a/block/iscsi.c b/block/iscsi.c
index 5f26e7379c..997b122141 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1238,7 +1238,8 @@ static int iscsi_has_zero_init(BlockDriverState *bs)
     return 0;
 }
 
-static int iscsi_create(const char *filename, QEMUOptionParameter *options)
+static int iscsi_create(const char *filename, QEMUOptionParameter *options,
+                        Error **errp)
 {
     int ret = 0;
     int64_t total_size = 0;
diff --git a/block/qcow.c b/block/qcow.c
index 2a1c9c9e87..0911edff6c 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -659,7 +659,8 @@ static void qcow_close(BlockDriverState *bs)
     error_free(s->migration_blocker);
 }
 
-static int qcow_create(const char *filename, QEMUOptionParameter *options)
+static int qcow_create(const char *filename, QEMUOptionParameter *options,
+                       Error **errp)
 {
     int header_size, backing_filename_len, l1_size, shift, i;
     QCowHeader header;
diff --git a/block/qcow2.c b/block/qcow2.c
index eda085cfd8..43edc5bd2a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1468,7 +1468,8 @@ out:
     return ret;
 }
 
-static int qcow2_create(const char *filename, QEMUOptionParameter *options)
+static int qcow2_create(const char *filename, QEMUOptionParameter *options,
+                        Error **errp)
 {
     const char *backing_file = NULL;
     const char *backing_fmt = NULL;
diff --git a/block/qed.c b/block/qed.c
index 69e1834ff6..250fa89b0f 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -604,7 +604,8 @@ out:
     return ret;
 }
 
-static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options)
+static int bdrv_qed_create(const char *filename, QEMUOptionParameter *options,
+                           Error **errp)
 {
     uint64_t image_size = 0;
     uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index dcdf45c4ac..3ee5b62509 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1041,7 +1041,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
     return (int64_t)st.st_blocks * 512;
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+                      Error **errp)
 {
     int fd;
     int result = 0;
@@ -1506,7 +1507,8 @@ static coroutine_fn BlockDriverAIOCB *hdev_aio_discard(BlockDriverState *bs,
                        cb, opaque, QEMU_AIO_DISCARD|QEMU_AIO_BLKDEV);
 }
 
-static int hdev_create(const char *filename, QEMUOptionParameter *options)
+static int hdev_create(const char *filename, QEMUOptionParameter *options,
+                       Error **errp)
 {
     int fd;
     int ret = 0;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 80551b957c..1e7651be61 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -422,7 +422,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
     return st.st_size;
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+                      Error **errp)
 {
     int fd;
     int64_t total_size = 0;
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 2418f580cc..7d75ad282f 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -130,7 +130,8 @@ static int raw_has_zero_init(BlockDriverState *bs)
     return bdrv_has_zero_init(bs->file);
 }
 
-static int raw_create(const char *filename, QEMUOptionParameter *options)
+static int raw_create(const char *filename, QEMUOptionParameter *options,
+                      Error **errp)
 {
     return bdrv_create_file(filename, options);
 }
diff --git a/block/rbd.c b/block/rbd.c
index b77f73ca89..11086c35c4 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -287,7 +287,8 @@ static int qemu_rbd_set_conf(rados_t cluster, const char *conf)
     return ret;
 }
 
-static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options)
+static int qemu_rbd_create(const char *filename, QEMUOptionParameter *options,
+                           Error **errp)
 {
     int64_t bytes = 0;
     int64_t objsize;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 1cfc6aedd5..a93a32a2ae 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1438,7 +1438,8 @@ out:
     return ret;
 }
 
-static int sd_create(const char *filename, QEMUOptionParameter *options)
+static int sd_create(const char *filename, QEMUOptionParameter *options,
+                     Error **errp)
 {
     int ret = 0;
     uint32_t vid = 0, base_vid = 0;
diff --git a/block/ssh.c b/block/ssh.c
index 23cd1f07e5..aa63c9d20e 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -651,7 +651,8 @@ static QEMUOptionParameter ssh_create_options[] = {
     { NULL }
 };
 
-static int ssh_create(const char *filename, QEMUOptionParameter *options)
+static int ssh_create(const char *filename, QEMUOptionParameter *options,
+                      Error **errp)
 {
     int r, ret;
     Error *local_err = NULL;
diff --git a/block/vdi.c b/block/vdi.c
index dedbafb539..dcbc27c9cb 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -645,7 +645,8 @@ static int vdi_co_write(BlockDriverState *bs,
     return ret;
 }
 
-static int vdi_create(const char *filename, QEMUOptionParameter *options)
+static int vdi_create(const char *filename, QEMUOptionParameter *options,
+                      Error **errp)
 {
     int fd;
     int result = 0;
diff --git a/block/vmdk.c b/block/vmdk.c
index d60bf0cdf8..a98da086a7 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1552,7 +1552,8 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
     return VMDK_OK;
 }
 
-static int vmdk_create(const char *filename, QEMUOptionParameter *options)
+static int vmdk_create(const char *filename, QEMUOptionParameter *options,
+                       Error **errp)
 {
     int fd, idx = 0;
     char desc[BUF_SIZE];
diff --git a/block/vpc.c b/block/vpc.c
index ed00370bad..db61274332 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -684,7 +684,8 @@ static int create_fixed_disk(int fd, uint8_t *buf, int64_t total_size)
     return ret;
 }
 
-static int vpc_create(const char *filename, QEMUOptionParameter *options)
+static int vpc_create(const char *filename, QEMUOptionParameter *options,
+                      Error **errp)
 {
     uint8_t buf[1024];
     struct vhd_footer *footer = (struct vhd_footer *) buf;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 076b40aa43..3eeb6fe2a4 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -90,7 +90,8 @@ struct BlockDriver {
                       const uint8_t *buf, int nb_sectors);
     void (*bdrv_close)(BlockDriverState *bs);
     void (*bdrv_rebind)(BlockDriverState *bs);
-    int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
+    int (*bdrv_create)(const char *filename, QEMUOptionParameter *options,
+                       Error **errp);
     int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
     int (*bdrv_make_empty)(BlockDriverState *bs);
     /* aio */

From 34b5d2c68eb4082c288e70fb99c61af8f7b96fde Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Thu, 5 Sep 2013 14:45:29 +0200
Subject: [PATCH 24/33] block: Error parameter for open functions

Add an Error ** parameter to bdrv_open, bdrv_file_open and associated
functions to allow more specific error messages.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 100 +++++++++++++++++++++++++++---------------
 block/blkdebug.c      |   4 +-
 block/blkverify.c     |   8 +++-
 block/cow.c           |   5 ++-
 block/mirror.c        |   5 ++-
 block/qcow.c          |   5 ++-
 block/qcow2.c         |   4 +-
 block/qed.c           |   6 ++-
 block/sheepdog.c      |  10 ++++-
 block/vmdk.c          |  11 ++++-
 block/vvfat.c         |   6 ++-
 blockdev.c            |  30 ++++++-------
 hw/block/xen_disk.c   |   7 ++-
 include/block/block.h |   6 +--
 qemu-img.c            |  21 ++++++---
 qemu-io.c             |  14 ++++--
 qemu-nbd.c            |   6 ++-
 17 files changed, 163 insertions(+), 85 deletions(-)

diff --git a/block.c b/block.c
index f9b7033503..ba0d8760db 100644
--- a/block.c
+++ b/block.c
@@ -552,7 +552,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 }
 
 static int find_image_format(BlockDriverState *bs, const char *filename,
-                             BlockDriver **pdrv)
+                             BlockDriver **pdrv, Error **errp)
 {
     int score, score_max;
     BlockDriver *drv1, *drv;
@@ -563,6 +563,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
     if (bs->sg || !bdrv_is_inserted(bs) || bdrv_getlength(bs) == 0) {
         drv = bdrv_find_format("raw");
         if (!drv) {
+            error_setg(errp, "Could not find raw image format");
             ret = -ENOENT;
         }
         *pdrv = drv;
@@ -571,6 +572,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
 
     ret = bdrv_pread(bs, 0, buf, sizeof(buf));
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not read image for determining its "
+                         "format");
         *pdrv = NULL;
         return ret;
     }
@@ -587,6 +590,8 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
         }
     }
     if (!drv) {
+        error_setg(errp, "Could not determine image format: No compatible "
+                   "driver found");
         ret = -ENOENT;
     }
     *pdrv = drv;
@@ -706,10 +711,11 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
  * Removes all processed options from *options.
  */
 static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
-    QDict *options, int flags, BlockDriver *drv)
+    QDict *options, int flags, BlockDriver *drv, Error **errp)
 {
     int ret, open_flags;
     const char *filename;
+    Error *local_err = NULL;
 
     assert(drv != NULL);
     assert(bs->file == NULL);
@@ -738,6 +744,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     bs->read_only = !(open_flags & BDRV_O_RDWR);
 
     if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
+        error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name);
         return -ENOTSUP;
     }
 
@@ -761,25 +768,32 @@ static int bdrv_open_common(BlockDriverState *bs, BlockDriverState *file,
     if (drv->bdrv_file_open) {
         assert(file == NULL);
         assert(drv->bdrv_parse_filename || filename != NULL);
-        ret = drv->bdrv_file_open(bs, options, open_flags, NULL);
+        ret = drv->bdrv_file_open(bs, options, open_flags, &local_err);
     } else {
         if (file == NULL) {
-            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't use '%s' as a "
-                          "block driver for the protocol level",
-                          drv->format_name);
+            error_setg(errp, "Can't use '%s' as a block driver for the "
+                       "protocol level", drv->format_name);
             ret = -EINVAL;
             goto free_and_fail;
         }
         bs->file = file;
-        ret = drv->bdrv_open(bs, options, open_flags, NULL);
+        ret = drv->bdrv_open(bs, options, open_flags, &local_err);
     }
 
     if (ret < 0) {
+        if (error_is_set(&local_err)) {
+            error_propagate(errp, local_err);
+        } else if (filename) {
+            error_setg_errno(errp, -ret, "Could not open '%s'", filename);
+        } else {
+            error_setg_errno(errp, -ret, "Could not open image");
+        }
         goto free_and_fail;
     }
 
     ret = refresh_total_sectors(bs, bs->total_sectors);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not refresh total sector count");
         goto free_and_fail;
     }
 
@@ -808,12 +822,13 @@ free_and_fail:
  * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
  */
 int bdrv_file_open(BlockDriverState **pbs, const char *filename,
-                   QDict *options, int flags)
+                   QDict *options, int flags, Error **errp)
 {
     BlockDriverState *bs;
     BlockDriver *drv;
     const char *drvname;
     bool allow_protocol_prefix = false;
+    Error *local_err = NULL;
     int ret;
 
     /* NULL means an empty set of options */
@@ -832,8 +847,8 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
         qdict_put(options, "filename", qstring_from_str(filename));
         allow_protocol_prefix = true;
     } else {
-        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Can't specify 'file' and "
-                      "'filename' options at the same time");
+        error_setg(errp, "Can't specify 'file' and 'filename' options at the "
+                   "same time");
         ret = -EINVAL;
         goto fail;
     }
@@ -842,53 +857,53 @@ int bdrv_file_open(BlockDriverState **pbs, const char *filename,
     drvname = qdict_get_try_str(options, "driver");
     if (drvname) {
         drv = bdrv_find_whitelisted_format(drvname, !(flags & BDRV_O_RDWR));
+        if (!drv) {
+            error_setg(errp, "Unknown driver '%s'", drvname);
+        }
         qdict_del(options, "driver");
     } else if (filename) {
         drv = bdrv_find_protocol(filename, allow_protocol_prefix);
         if (!drv) {
-            qerror_report(ERROR_CLASS_GENERIC_ERROR, "Unknown protocol");
+            error_setg(errp, "Unknown protocol");
         }
     } else {
-        qerror_report(ERROR_CLASS_GENERIC_ERROR,
-                      "Must specify either driver or file");
+        error_setg(errp, "Must specify either driver or file");
         drv = NULL;
     }
 
     if (!drv) {
+        /* errp has been set already */
         ret = -ENOENT;
         goto fail;
     }
 
     /* Parse the filename and open it */
     if (drv->bdrv_parse_filename && filename) {
-        Error *local_err = NULL;
         drv->bdrv_parse_filename(filename, options, &local_err);
         if (error_is_set(&local_err)) {
-            qerror_report_err(local_err);
-            error_free(local_err);
+            error_propagate(errp, local_err);
             ret = -EINVAL;
             goto fail;
         }
         qdict_del(options, "filename");
     } else if (!drv->bdrv_parse_filename && !filename) {
-        qerror_report(ERROR_CLASS_GENERIC_ERROR,
-                      "The '%s' block driver requires a file name",
-                      drv->format_name);
+        error_setg(errp, "The '%s' block driver requires a file name",
+                   drv->format_name);
         ret = -EINVAL;
         goto fail;
     }
 
-    ret = bdrv_open_common(bs, NULL, options, flags, drv);
+    ret = bdrv_open_common(bs, NULL, options, flags, drv, &local_err);
     if (ret < 0) {
+        error_propagate(errp, local_err);
         goto fail;
     }
 
     /* Check if any unknown options were used */
     if (qdict_size(options) != 0) {
         const QDictEntry *entry = qdict_first(options);
-        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block protocol '%s' doesn't "
-                      "support the option '%s'",
-                      drv->format_name, entry->key);
+        error_setg(errp, "Block protocol '%s' doesn't support the option '%s'",
+                   drv->format_name, entry->key);
         ret = -EINVAL;
         goto fail;
     }
@@ -915,11 +930,12 @@ fail:
  * function (even on failure), so if the caller intends to reuse the dictionary,
  * it needs to use QINCREF() before calling bdrv_file_open.
  */
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
 {
     char backing_filename[PATH_MAX];
     int back_flags, ret;
     BlockDriver *back_drv = NULL;
+    Error *local_err = NULL;
 
     if (bs->backing_hd != NULL) {
         QDECREF(options);
@@ -952,11 +968,12 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options)
 
     ret = bdrv_open(bs->backing_hd,
                     *backing_filename ? backing_filename : NULL, options,
-                    back_flags, back_drv);
+                    back_flags, back_drv, &local_err);
     if (ret < 0) {
         bdrv_unref(bs->backing_hd);
         bs->backing_hd = NULL;
         bs->open_flags |= BDRV_O_NO_BACKING;
+        error_propagate(errp, local_err);
         return ret;
     }
     return 0;
@@ -990,7 +1007,7 @@ static void extract_subqdict(QDict *src, QDict **dst, const char *start)
  * dictionary, it needs to use QINCREF() before calling bdrv_open.
  */
 int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
-              int flags, BlockDriver *drv)
+              int flags, BlockDriver *drv, Error **errp)
 {
     int ret;
     /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
@@ -998,6 +1015,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     BlockDriverState *file = NULL;
     QDict *file_options = NULL;
     const char *drvname;
+    Error *local_err = NULL;
 
     /* NULL means an empty set of options */
     if (options == NULL) {
@@ -1016,7 +1034,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         char backing_filename[PATH_MAX];
 
         if (qdict_size(options) != 0) {
-            error_report("Can't use snapshot=on with driver-specific options");
+            error_setg(errp, "Can't use snapshot=on with driver-specific options");
             ret = -EINVAL;
             goto fail;
         }
@@ -1027,7 +1045,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 
         /* if there is a backing file, use it */
         bs1 = bdrv_new("");
-        ret = bdrv_open(bs1, filename, NULL, 0, drv);
+        ret = bdrv_open(bs1, filename, NULL, 0, drv, &local_err);
         if (ret < 0) {
             bdrv_unref(bs1);
             goto fail;
@@ -1038,6 +1056,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
 
         ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not get temporary filename");
             goto fail;
         }
 
@@ -1046,6 +1065,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
             snprintf(backing_filename, sizeof(backing_filename),
                      "%s", filename);
         } else if (!realpath(filename, backing_filename)) {
+            error_setg_errno(errp, errno, "Could not resolve path '%s'", filename);
             ret = -errno;
             goto fail;
         }
@@ -1065,6 +1085,8 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
         free_option_parameters(create_options);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not create temporary overlay "
+                             "'%s'", tmp_filename);
             goto fail;
         }
 
@@ -1081,7 +1103,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     extract_subqdict(options, &file_options, "file.");
 
     ret = bdrv_file_open(&file, filename, file_options,
-                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP));
+                         bdrv_open_flags(bs, flags | BDRV_O_UNMAP), &local_err);
     if (ret < 0) {
         goto fail;
     }
@@ -1094,7 +1116,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     }
 
     if (!drv) {
-        ret = find_image_format(file, filename, &drv);
+        ret = find_image_format(file, filename, &drv, &local_err);
     }
 
     if (!drv) {
@@ -1102,7 +1124,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     }
 
     /* Open the image */
-    ret = bdrv_open_common(bs, file, options, flags, drv);
+    ret = bdrv_open_common(bs, file, options, flags, drv, &local_err);
     if (ret < 0) {
         goto unlink_and_fail;
     }
@@ -1117,7 +1139,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
         QDict *backing_options;
 
         extract_subqdict(options, &backing_options, "backing.");
-        ret = bdrv_open_backing_file(bs, backing_options);
+        ret = bdrv_open_backing_file(bs, backing_options, &local_err);
         if (ret < 0) {
             goto close_and_fail;
         }
@@ -1126,9 +1148,9 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
     /* Check if any unknown options were used */
     if (qdict_size(options) != 0) {
         const QDictEntry *entry = qdict_first(options);
-        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Block format '%s' used by "
-            "device '%s' doesn't support the option '%s'",
-            drv->format_name, bs->device_name, entry->key);
+        error_setg(errp, "Block format '%s' used by device '%s' doesn't "
+                   "support the option '%s'", drv->format_name, bs->device_name,
+                   entry->key);
 
         ret = -EINVAL;
         goto close_and_fail;
@@ -1152,11 +1174,17 @@ fail:
     QDECREF(bs->options);
     QDECREF(options);
     bs->options = NULL;
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
     return ret;
 
 close_and_fail:
     bdrv_close(bs);
     QDECREF(options);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
     return ret;
 }
 
@@ -4519,7 +4547,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
             bs = bdrv_new("");
 
             ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
-                            backing_drv);
+                            backing_drv, NULL);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "Could not open '%s'",
                                  backing_file->value.s);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 52d65ffcd4..be948b2fdd 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -387,8 +387,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    ret = bdrv_file_open(&bs->file, filename, NULL, flags);
+    ret = bdrv_file_open(&bs->file, filename, NULL, flags, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         goto fail;
     }
 
diff --git a/block/blkverify.c b/block/blkverify.c
index 2093391c8b..bff95d2a45 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -141,8 +141,10 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    ret = bdrv_file_open(&bs->file, raw, NULL, flags);
+    ret = bdrv_file_open(&bs->file, raw, NULL, flags, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         goto fail;
     }
 
@@ -154,8 +156,10 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     s->test_file = bdrv_new("");
-    ret = bdrv_open(s->test_file, filename, NULL, flags, NULL);
+    ret = bdrv_open(s->test_file, filename, NULL, flags, NULL, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         bdrv_unref(s->test_file);
         s->test_file = NULL;
         goto fail;
diff --git a/block/cow.c b/block/cow.c
index e252c95601..3a93ed9966 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -302,6 +302,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
     struct stat st;
     int64_t image_sectors = 0;
     const char *image_filename = NULL;
+    Error *local_err = NULL;
     int ret;
     BlockDriverState *cow_bs;
 
@@ -320,8 +321,10 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
         return ret;
     }
 
-    ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&cow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/mirror.c b/block/mirror.c
index f61a7799de..6e7a274e43 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -505,14 +505,15 @@ static void mirror_iostatus_reset(BlockJob *job)
 static void mirror_complete(BlockJob *job, Error **errp)
 {
     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open_backing_file(s->target, NULL);
+    ret = bdrv_open_backing_file(s->target, NULL, &local_err);
     if (ret < 0) {
         char backing_filename[PATH_MAX];
         bdrv_get_full_backing_filename(s->target, backing_filename,
                                        sizeof(backing_filename));
-        error_setg_file_open(errp, -ret, backing_filename);
+        error_propagate(errp, local_err);
         return;
     }
     if (!s->synced) {
diff --git a/block/qcow.c b/block/qcow.c
index 0911edff6c..396636f7b4 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -668,6 +668,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
     int64_t total_size = 0;
     const char *backing_file = NULL;
     int flags = 0;
+    Error *local_err = NULL;
     int ret;
     BlockDriverState *qcow_bs;
 
@@ -688,8 +689,10 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
         return ret;
     }
 
-    ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&qcow_bs, filename, NULL, BDRV_O_RDWR, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 43edc5bd2a..dabfe8d08b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1370,7 +1370,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         return ret;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
     if (ret < 0) {
         return ret;
     }
@@ -1423,7 +1423,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     BlockDriver* drv = bdrv_find_format("qcow2");
     assert(drv != NULL);
     ret = bdrv_open(bs, filename, NULL,
-        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv);
+        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
     if (ret < 0) {
         goto out;
     }
diff --git a/block/qed.c b/block/qed.c
index 250fa89b0f..f17094cb92 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -551,6 +551,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     QEDHeader le_header;
     uint8_t *l1_table = NULL;
     size_t l1_size = header.cluster_size * header.table_size;
+    Error *local_err = NULL;
     int ret = 0;
     BlockDriverState *bs = NULL;
 
@@ -559,8 +560,11 @@ static int qed_create(const char *filename, uint32_t cluster_size,
         return ret;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR | BDRV_O_CACHE_WB,
+                         &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index a93a32a2ae..38fb629650 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1401,10 +1401,13 @@ static int sd_prealloc(const char *filename)
     uint32_t idx, max_idx;
     int64_t vdi_size;
     void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
+    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         goto out;
     }
 
@@ -1449,6 +1452,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
     char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
     uint32_t snapid;
     bool prealloc = false;
+    Error *local_err = NULL;
 
     s = g_malloc0(sizeof(BDRVSheepdogState));
 
@@ -1502,8 +1506,10 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
             goto out;
         }
 
-        ret = bdrv_file_open(&bs, backing_file, NULL, 0);
+        ret = bdrv_file_open(&bs, backing_file, NULL, 0, &local_err);
         if (ret < 0) {
+            qerror_report_err(local_err);
+            error_free(local_err);
             goto out;
         }
 
diff --git a/block/vmdk.c b/block/vmdk.c
index a98da086a7..96ef1b534e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -697,6 +697,7 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
     int64_t flat_offset;
     char extent_path[PATH_MAX];
     BlockDriverState *extent_file;
+    Error *local_err = NULL;
 
     while (*p) {
         /* parse extent line:
@@ -726,8 +727,11 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
 
         path_combine(extent_path, sizeof(extent_path),
                 desc_file_path, fname);
-        ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags);
+        ret = bdrv_file_open(&extent_file, extent_path, NULL, bs->open_flags,
+                             &local_err);
         if (ret) {
+            qerror_report_err(local_err);
+            error_free(local_err);
             return ret;
         }
 
@@ -1591,6 +1595,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
         "ddb.geometry.heads = \"%d\"\n"
         "ddb.geometry.sectors = \"63\"\n"
         "ddb.adapterType = \"%s\"\n";
+    Error *local_err = NULL;
 
     if (filename_decompose(filename, path, prefix, postfix, PATH_MAX)) {
         return -EINVAL;
@@ -1653,8 +1658,10 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
     }
     if (backing_file) {
         BlockDriverState *bs = bdrv_new("");
-        ret = bdrv_open(bs, backing_file, NULL, 0, NULL);
+        ret = bdrv_open(bs, backing_file, NULL, 0, NULL, &local_err);
         if (ret != 0) {
+            qerror_report_err(local_err);
+            error_free(local_err);
             bdrv_unref(bs);
             return ret;
         }
diff --git a/block/vvfat.c b/block/vvfat.c
index 2f8be7cf1a..788d0630fd 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2910,6 +2910,7 @@ static int enable_write_target(BDRVVVFATState *s)
 {
     BlockDriver *bdrv_qcow;
     QEMUOptionParameter *options;
+    Error *local_err = NULL;
     int ret;
     int size = sector2cluster(s, s->sector_count);
     s->used_clusters = calloc(size, 1);
@@ -2935,8 +2936,11 @@ static int enable_write_target(BDRVVVFATState *s)
     s->qcow = bdrv_new("");
 
     ret = bdrv_open(s->qcow, s->qcow_filename, NULL,
-            BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow);
+            BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, bdrv_qcow,
+            &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         bdrv_unref(s->qcow);
         goto err;
     }
diff --git a/blockdev.c b/blockdev.c
index 84d55c1adc..2ab236a82d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -710,17 +710,11 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
     }
 
     QINCREF(bs_opts);
-    ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv);
+    ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv, &error);
 
     if (ret < 0) {
-        if (ret == -EMEDIUMTYPE) {
-            error_report("could not open disk image %s: not in %s format",
-                         file ?: dinfo->id, drv ? drv->format_name :
-                         qdict_get_str(bs_opts, "driver"));
-        } else {
-            error_report("could not open disk image %s: %s",
-                         file ?: dinfo->id, strerror(-ret));
-        }
+        error_report("could not open disk image %s: %s",
+                     file ?: dinfo->id, error_get_pretty(error));
         goto err;
     }
 
@@ -1156,9 +1150,9 @@ static void external_snapshot_prepare(BlkTransactionState *common,
     /* TODO Inherit bs->options or only take explicit options with an
      * extended QMP command? */
     ret = bdrv_open(state->new_bs, new_image_file, NULL,
-                    flags | BDRV_O_NO_BACKING, drv);
+                    flags | BDRV_O_NO_BACKING, drv, &local_err);
     if (ret != 0) {
-        error_setg_file_open(errp, -ret, new_image_file);
+        error_propagate(errp, local_err);
     }
 }
 
@@ -1393,11 +1387,12 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
                                     int bdrv_flags, BlockDriver *drv,
                                     const char *password, Error **errp)
 {
+    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv);
+    ret = bdrv_open(bs, filename, NULL, bdrv_flags, drv, &local_err);
     if (ret < 0) {
-        error_setg_file_open(errp, -ret, filename);
+        error_propagate(errp, local_err);
         return;
     }
 
@@ -1817,10 +1812,10 @@ void qmp_drive_backup(const char *device, const char *target,
     }
 
     target_bs = bdrv_new("");
-    ret = bdrv_open(target_bs, target, NULL, flags, drv);
+    ret = bdrv_open(target_bs, target, NULL, flags, drv, &local_err);
     if (ret < 0) {
         bdrv_unref(target_bs);
-        error_setg_file_open(errp, -ret, target);
+        error_propagate(errp, local_err);
         return;
     }
 
@@ -1952,10 +1947,11 @@ void qmp_drive_mirror(const char *device, const char *target,
      * file.
      */
     target_bs = bdrv_new("");
-    ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv);
+    ret = bdrv_open(target_bs, target, NULL, flags | BDRV_O_NO_BACKING, drv,
+                    &local_err);
     if (ret < 0) {
         bdrv_unref(target_bs);
-        error_setg_file_open(errp, -ret, target);
+        error_propagate(errp, local_err);
         return;
     }
 
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 668cc069ff..f35fc5944a 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -809,10 +809,15 @@ static int blk_connect(struct XenDevice *xendev)
         xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
         blkdev->bs = bdrv_new(blkdev->dev);
         if (blkdev->bs) {
+            Error *local_err = NULL;
             BlockDriver *drv = bdrv_find_whitelisted_format(blkdev->fileproto,
                                                            readonly);
             if (bdrv_open(blkdev->bs,
-                          blkdev->filename, NULL, qflags, drv) != 0) {
+                          blkdev->filename, NULL, qflags, drv, &local_err) != 0)
+            {
+                xen_be_printf(&blkdev->xendev, 0, "error: %s\n",
+                              error_get_pretty(local_err));
+                error_free(local_err);
                 bdrv_unref(blkdev->bs);
                 blkdev->bs = NULL;
             }
diff --git a/include/block/block.h b/include/block/block.h
index 1c5f939d04..9dbc5ef855 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -151,10 +151,10 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 int bdrv_parse_cache_flags(const char *mode, int *flags);
 int bdrv_parse_discard_flags(const char *mode, int *flags);
 int bdrv_file_open(BlockDriverState **pbs, const char *filename,
-                   QDict *options, int flags);
-int bdrv_open_backing_file(BlockDriverState *bs, QDict *options);
+                   QDict *options, int flags, Error **errp);
+int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
 int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
-              int flags, BlockDriver *drv);
+              int flags, BlockDriver *drv, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
                                     BlockDriverState *bs, int flags);
 int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
diff --git a/qemu-img.c b/qemu-img.c
index 139526f348..49ff06869e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -266,6 +266,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
     BlockDriverState *bs;
     BlockDriver *drv;
     char password[256];
+    Error *local_err = NULL;
     int ret;
 
     bs = bdrv_new("image");
@@ -280,9 +281,11 @@ static BlockDriverState *bdrv_new_open(const char *filename,
         drv = NULL;
     }
 
-    ret = bdrv_open(bs, filename, NULL, flags, drv);
+    ret = bdrv_open(bs, filename, NULL, flags, drv, &local_err);
     if (ret < 0) {
-        error_report("Could not open '%s': %s", filename, strerror(-ret));
+        error_report("Could not open '%s': %s", filename,
+                     error_get_pretty(local_err));
+        error_free(local_err);
         goto fail;
     }
 
@@ -2127,6 +2130,7 @@ static int img_rebase(int argc, char **argv)
     int unsafe = 0;
     int progress = 0;
     bool quiet = false;
+    Error *local_err = NULL;
 
     /* Parse commandline parameters */
     fmt = NULL;
@@ -2230,18 +2234,21 @@ static int img_rebase(int argc, char **argv)
         bs_old_backing = bdrv_new("old_backing");
         bdrv_get_backing_filename(bs, backing_name, sizeof(backing_name));
         ret = bdrv_open(bs_old_backing, backing_name, NULL, BDRV_O_FLAGS,
-                        old_backing_drv);
+                        old_backing_drv, &local_err);
         if (ret) {
-            error_report("Could not open old backing file '%s'", backing_name);
+            error_report("Could not open old backing file '%s': %s",
+                         backing_name, error_get_pretty(local_err));
+            error_free(local_err);
             goto out;
         }
         if (out_baseimg[0]) {
             bs_new_backing = bdrv_new("new_backing");
             ret = bdrv_open(bs_new_backing, out_baseimg, NULL, BDRV_O_FLAGS,
-                        new_backing_drv);
+                        new_backing_drv, &local_err);
             if (ret) {
-                error_report("Could not open new backing file '%s'",
-                             out_baseimg);
+                error_report("Could not open new backing file '%s': %s",
+                             out_baseimg, error_get_pretty(local_err));
+                error_free(local_err);
                 goto out;
             }
         }
diff --git a/qemu-io.c b/qemu-io.c
index 71f4ff1302..f4b8efcceb 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -46,21 +46,27 @@ static const cmdinfo_t close_cmd = {
 
 static int openfile(char *name, int flags, int growable)
 {
+    Error *local_err = NULL;
+
     if (qemuio_bs) {
         fprintf(stderr, "file open already, try 'help close'\n");
         return 1;
     }
 
     if (growable) {
-        if (bdrv_file_open(&qemuio_bs, name, NULL, flags)) {
-            fprintf(stderr, "%s: can't open device %s\n", progname, name);
+        if (bdrv_file_open(&qemuio_bs, name, NULL, flags, &local_err)) {
+            fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
+                    error_get_pretty(local_err));
+            error_free(local_err);
             return 1;
         }
     } else {
         qemuio_bs = bdrv_new("hda");
 
-        if (bdrv_open(qemuio_bs, name, NULL, flags, NULL) < 0) {
-            fprintf(stderr, "%s: can't open device %s\n", progname, name);
+        if (bdrv_open(qemuio_bs, name, NULL, flags, NULL, &local_err) < 0) {
+            fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
+                    error_get_pretty(local_err));
+            error_free(local_err);
             bdrv_unref(qemuio_bs);
             qemuio_bs = NULL;
             return 1;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index f044546c28..c26c98ef1d 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -355,6 +355,7 @@ int main(int argc, char **argv)
 #endif
     pthread_t client_thread;
     const char *fmt = NULL;
+    Error *local_err = NULL;
 
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -573,10 +574,11 @@ int main(int argc, char **argv)
 
     bs = bdrv_new("hda");
     srcpath = argv[optind];
-    ret = bdrv_open(bs, srcpath, NULL, flags, drv);
+    ret = bdrv_open(bs, srcpath, NULL, flags, drv, &local_err);
     if (ret < 0) {
         errno = -ret;
-        err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]);
+        err(EXIT_FAILURE, "Failed to bdrv_open '%s': %s", argv[optind],
+            error_get_pretty(local_err));
     }
 
     fd_size = bdrv_getlength(bs);

From cc84d90ff54c025190dbe49ec5fea1268217c5f2 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Fri, 6 Sep 2013 17:14:26 +0200
Subject: [PATCH 25/33] block: Error parameter for create functions

Add an Error ** parameter to bdrv_create and its associated functions to
allow more specific error messages.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c               | 80 ++++++++++++++++++++++++++++++-------------
 block/cow.c           |  4 ++-
 block/qcow.c          |  4 ++-
 block/qcow2.c         |  2 +-
 block/qed.c           |  4 ++-
 block/raw_bsd.c       | 10 +++++-
 block/vvfat.c         |  4 ++-
 include/block/block.h |  5 +--
 qemu-img.c            | 16 +++------
 9 files changed, 86 insertions(+), 43 deletions(-)

diff --git a/block.c b/block.c
index ba0d8760db..e176c6f3bc 100644
--- a/block.c
+++ b/block.c
@@ -394,18 +394,26 @@ typedef struct CreateCo {
     char *filename;
     QEMUOptionParameter *options;
     int ret;
+    Error *err;
 } CreateCo;
 
 static void coroutine_fn bdrv_create_co_entry(void *opaque)
 {
+    Error *local_err = NULL;
+    int ret;
+
     CreateCo *cco = opaque;
     assert(cco->drv);
 
-    cco->ret = cco->drv->bdrv_create(cco->filename, cco->options, NULL);
+    ret = cco->drv->bdrv_create(cco->filename, cco->options, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(&cco->err, local_err);
+    }
+    cco->ret = ret;
 }
 
 int bdrv_create(BlockDriver *drv, const char* filename,
-    QEMUOptionParameter *options)
+    QEMUOptionParameter *options, Error **errp)
 {
     int ret;
 
@@ -415,9 +423,11 @@ int bdrv_create(BlockDriver *drv, const char* filename,
         .filename = g_strdup(filename),
         .options = options,
         .ret = NOT_DONE,
+        .err = NULL,
     };
 
     if (!drv->bdrv_create) {
+        error_setg(errp, "Driver '%s' does not support image creation", drv->format_name);
         ret = -ENOTSUP;
         goto out;
     }
@@ -434,22 +444,37 @@ int bdrv_create(BlockDriver *drv, const char* filename,
     }
 
     ret = cco.ret;
+    if (ret < 0) {
+        if (error_is_set(&cco.err)) {
+            error_propagate(errp, cco.err);
+        } else {
+            error_setg_errno(errp, -ret, "Could not create image");
+        }
+    }
 
 out:
     g_free(cco.filename);
     return ret;
 }
 
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+                     Error **errp)
 {
     BlockDriver *drv;
+    Error *local_err = NULL;
+    int ret;
 
     drv = bdrv_find_protocol(filename, true);
     if (drv == NULL) {
+        error_setg(errp, "Could not find protocol for file '%s'", filename);
         return -ENOENT;
     }
 
-    return bdrv_create(drv, filename, options);
+    ret = bdrv_create(drv, filename, options, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
+    return ret;
 }
 
 /*
@@ -1082,11 +1107,14 @@ int bdrv_open(BlockDriverState *bs, const char *filename, QDict *options,
                 drv->format_name);
         }
 
-        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options);
+        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
         free_option_parameters(create_options);
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Could not create temporary overlay "
-                             "'%s'", tmp_filename);
+                             "'%s': %s", tmp_filename,
+                             error_get_pretty(local_err));
+            error_free(local_err);
+            local_err = NULL;
             goto fail;
         }
 
@@ -4461,6 +4489,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
     BlockDriverState *bs = NULL;
     BlockDriver *drv, *proto_drv;
     BlockDriver *backing_drv = NULL;
+    Error *local_err = NULL;
     int ret = 0;
 
     /* Find driver and parse its options */
@@ -4547,10 +4576,13 @@ void bdrv_img_create(const char *filename, const char *fmt,
             bs = bdrv_new("");
 
             ret = bdrv_open(bs, backing_file->value.s, NULL, back_flags,
-                            backing_drv, NULL);
+                            backing_drv, &local_err);
             if (ret < 0) {
-                error_setg_errno(errp, -ret, "Could not open '%s'",
-                                 backing_file->value.s);
+                error_setg_errno(errp, -ret, "Could not open '%s': %s",
+                                 backing_file->value.s,
+                                 error_get_pretty(local_err));
+                error_free(local_err);
+                local_err = NULL;
                 goto out;
             }
             bdrv_get_geometry(bs, &size);
@@ -4569,22 +4601,19 @@ void bdrv_img_create(const char *filename, const char *fmt,
         print_option_parameters(param);
         puts("");
     }
-    ret = bdrv_create(drv, filename, param);
-    if (ret < 0) {
-        if (ret == -ENOTSUP) {
-            error_setg(errp,"Formatting or formatting option not supported for "
-                            "file format '%s'", fmt);
-        } else if (ret == -EFBIG) {
-            const char *cluster_size_hint = "";
-            if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
-                cluster_size_hint = " (try using a larger cluster size)";
-            }
-            error_setg(errp, "The image size is too large for file format '%s'%s",
-                       fmt, cluster_size_hint);
-        } else {
-            error_setg(errp, "%s: error while creating %s: %s", filename, fmt,
-                       strerror(-ret));
+    ret = bdrv_create(drv, filename, param, &local_err);
+    if (ret == -EFBIG) {
+        /* This is generally a better message than whatever the driver would
+         * deliver (especially because of the cluster_size_hint), since that
+         * is most probably not much different from "image too large". */
+        const char *cluster_size_hint = "";
+        if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
+            cluster_size_hint = " (try using a larger cluster size)";
         }
+        error_setg(errp, "The image size is too large for file format '%s'"
+                   "%s", fmt, cluster_size_hint);
+        error_free(local_err);
+        local_err = NULL;
     }
 
 out:
@@ -4594,6 +4623,9 @@ out:
     if (bs) {
         bdrv_unref(bs);
     }
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
 }
 
 AioContext *bdrv_get_aio_context(BlockDriverState *bs)
diff --git a/block/cow.c b/block/cow.c
index 3a93ed9966..909c3e7182 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -316,8 +316,10 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options);
+    ret = bdrv_create_file(filename, options, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/qcow.c b/block/qcow.c
index 396636f7b4..c470e05f60 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -684,8 +684,10 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
         options++;
     }
 
-    ret = bdrv_create_file(filename, options);
+    ret = bdrv_create_file(filename, options, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index dabfe8d08b..b257347c3a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1365,7 +1365,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     uint8_t* refcount_table;
     int ret;
 
-    ret = bdrv_create_file(filename, options);
+    ret = bdrv_create_file(filename, options, NULL);
     if (ret < 0) {
         return ret;
     }
diff --git a/block/qed.c b/block/qed.c
index f17094cb92..6c0cba04f3 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -555,8 +555,10 @@ static int qed_create(const char *filename, uint32_t cluster_size,
     int ret = 0;
     BlockDriverState *bs = NULL;
 
-    ret = bdrv_create_file(filename, NULL);
+    ret = bdrv_create_file(filename, NULL, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         return ret;
     }
 
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 7d75ad282f..d4ace6020b 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -133,7 +133,15 @@ static int raw_has_zero_init(BlockDriverState *bs)
 static int raw_create(const char *filename, QEMUOptionParameter *options,
                       Error **errp)
 {
-    return bdrv_create_file(filename, options);
+    Error *local_err = NULL;
+    int ret;
+
+    ret = bdrv_create_file(filename, options, &local_err);
+    if (error_is_set(&local_err)) {
+        qerror_report_err(local_err);
+        error_free(local_err);
+    }
+    return ret;
 }
 
 static int raw_open(BlockDriverState *bs, QDict *options, int flags,
diff --git a/block/vvfat.c b/block/vvfat.c
index 788d0630fd..3ddaa0bcce 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2928,8 +2928,10 @@ static int enable_write_target(BDRVVVFATState *s)
     set_option_parameter_int(options, BLOCK_OPT_SIZE, s->sector_count * 512);
     set_option_parameter(options, BLOCK_OPT_BACKING_FILE, "fat:");
 
-    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options);
+    ret = bdrv_create(bdrv_qcow, s->qcow_filename, options, &local_err);
     if (ret < 0) {
+        qerror_report_err(local_err);
+        error_free(local_err);
         goto err;
     }
 
diff --git a/include/block/block.h b/include/block/block.h
index 9dbc5ef855..f808550959 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -142,8 +142,9 @@ BlockDriver *bdrv_find_format(const char *format_name);
 BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
                                           bool readonly);
 int bdrv_create(BlockDriver *drv, const char* filename,
-    QEMUOptionParameter *options);
-int bdrv_create_file(const char* filename, QEMUOptionParameter *options);
+    QEMUOptionParameter *options, Error **errp);
+int bdrv_create_file(const char* filename, QEMUOptionParameter *options,
+                     Error **errp);
 BlockDriverState *bdrv_new(const char *device_name);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
diff --git a/qemu-img.c b/qemu-img.c
index 49ff06869e..ccb0468d94 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1139,6 +1139,7 @@ static int img_convert(int argc, char **argv)
     float local_progress = 0;
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
     bool quiet = false;
+    Error *local_err = NULL;
 
     fmt = NULL;
     out_fmt = "raw";
@@ -1341,18 +1342,11 @@ static int img_convert(int argc, char **argv)
 
     if (!skip_create) {
         /* Create the new image */
-        ret = bdrv_create(drv, out_filename, param);
+        ret = bdrv_create(drv, out_filename, param, &local_err);
         if (ret < 0) {
-            if (ret == -ENOTSUP) {
-                error_report("Formatting not supported for file format '%s'",
-                             out_fmt);
-            } else if (ret == -EFBIG) {
-                error_report("The image size is too large for file format '%s'",
-                             out_fmt);
-            } else {
-                error_report("%s: error while converting %s: %s",
-                             out_filename, out_fmt, strerror(-ret));
-            }
+            error_report("%s: error while converting %s: %s",
+                         out_filename, out_fmt, error_get_pretty(local_err));
+            error_free(local_err);
             goto out;
         }
     }

From b70d8c237a0e5e829474c3a12c8783893c4e470e Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Fri, 6 Sep 2013 16:51:03 +0200
Subject: [PATCH 26/33] qemu-img create: Emit filename on error

bdrv_img_create generally does not emit the target filename, although
this is pretty important information. Therefore, prepend its error
message with the output filename (if an error occurs).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index ccb0468d94..6a1ba697c5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -412,7 +412,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);
     if (error_is_set(&local_err)) {
-        error_report("%s", error_get_pretty(local_err));
+        error_report("%s: %s", filename, error_get_pretty(local_err));
         error_free(local_err);
         return 1;
     }

From 3ef6c40ad0b350e18c78135ffbdbe209cb479c1f Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Thu, 5 Sep 2013 09:40:43 +0200
Subject: [PATCH 27/33] qcow2: Use Error parameter

Employ usage of the new Error ** parameter in qcow2_open, qcow2_create
and associated functions.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 134 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 88 insertions(+), 46 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b257347c3a..318d95d972 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -79,7 +79,8 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
  * return 0 upon success, non-0 otherwise
  */
 static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
-                                 uint64_t end_offset, void **p_feature_table)
+                                 uint64_t end_offset, void **p_feature_table,
+                                 Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
     QCowExtension ext;
@@ -100,10 +101,10 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
         printf("attempting to read extended header in offset %lu\n", offset);
 #endif
 
-        if (bdrv_pread(bs->file, offset, &ext, sizeof(ext)) != sizeof(ext)) {
-            fprintf(stderr, "qcow2_read_extension: ERROR: "
-                    "pread fail from offset %" PRIu64 "\n",
-                    offset);
+        ret = bdrv_pread(bs->file, offset, &ext, sizeof(ext));
+        if (ret < 0) {
+            error_setg_errno(errp, -ret, "qcow2_read_extension: ERROR: "
+                             "pread fail from offset %" PRIu64, offset);
             return 1;
         }
         be32_to_cpus(&ext.magic);
@@ -113,7 +114,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
         printf("ext.magic = 0x%x\n", ext.magic);
 #endif
         if (ext.len > end_offset - offset) {
-            error_report("Header extension too large");
+            error_setg(errp, "Header extension too large");
             return -EINVAL;
         }
 
@@ -123,14 +124,16 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
 
         case QCOW2_EXT_MAGIC_BACKING_FORMAT:
             if (ext.len >= sizeof(bs->backing_format)) {
-                fprintf(stderr, "ERROR: ext_backing_format: len=%u too large"
-                        " (>=%zu)\n",
-                        ext.len, sizeof(bs->backing_format));
+                error_setg(errp, "ERROR: ext_backing_format: len=%u too large"
+                           " (>=%zu)", ext.len, sizeof(bs->backing_format));
                 return 2;
             }
-            if (bdrv_pread(bs->file, offset , bs->backing_format,
-                           ext.len) != ext.len)
+            ret = bdrv_pread(bs->file, offset, bs->backing_format, ext.len);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "ERROR: ext_backing_format: "
+                                 "Could not read format name");
                 return 3;
+            }
             bs->backing_format[ext.len] = '\0';
 #ifdef DEBUG_EXT
             printf("Qcow2: Got format extension %s\n", bs->backing_format);
@@ -142,6 +145,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
                 void* feature_table = g_malloc0(ext.len + 2 * sizeof(Qcow2Feature));
                 ret = bdrv_pread(bs->file, offset , feature_table, ext.len);
                 if (ret < 0) {
+                    error_setg_errno(errp, -ret, "ERROR: ext_feature_table: "
+                                     "Could not read table");
                     return ret;
                 }
 
@@ -161,6 +166,8 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
 
                 ret = bdrv_pread(bs->file, offset , uext->data, uext->len);
                 if (ret < 0) {
+                    error_setg_errno(errp, -ret, "ERROR: unknown extension: "
+                                     "Could not read data");
                     return ret;
                 }
             }
@@ -184,8 +191,8 @@ static void cleanup_unknown_header_ext(BlockDriverState *bs)
     }
 }
 
-static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs,
-    const char *fmt, ...)
+static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
+    Error **errp, const char *fmt, ...)
 {
     char msg[64];
     va_list ap;
@@ -194,17 +201,17 @@ static void GCC_FMT_ATTR(2, 3) report_unsupported(BlockDriverState *bs,
     vsnprintf(msg, sizeof(msg), fmt, ap);
     va_end(ap);
 
-    qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
-        bs->device_name, "qcow2", msg);
+    error_set(errp, QERR_UNKNOWN_BLOCK_FORMAT_FEATURE, bs->device_name, "qcow2",
+              msg);
 }
 
 static void report_unsupported_feature(BlockDriverState *bs,
-    Qcow2Feature *table, uint64_t mask)
+    Error **errp, Qcow2Feature *table, uint64_t mask)
 {
     while (table && table->name[0] != '\0') {
         if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
             if (mask & (1 << table->bit)) {
-                report_unsupported(bs, "%.46s",table->name);
+                report_unsupported(bs, errp, "%.46s", table->name);
                 mask &= ~(1 << table->bit);
             }
         }
@@ -212,7 +219,8 @@ static void report_unsupported_feature(BlockDriverState *bs,
     }
 
     if (mask) {
-        report_unsupported(bs, "Unknown incompatible feature: %" PRIx64, mask);
+        report_unsupported(bs, errp, "Unknown incompatible feature: %" PRIx64,
+                           mask);
     }
 }
 
@@ -363,6 +371,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not read qcow2 header");
         goto fail;
     }
     be32_to_cpus(&header.magic);
@@ -380,11 +389,12 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     be32_to_cpus(&header.nb_snapshots);
 
     if (header.magic != QCOW_MAGIC) {
+        error_setg(errp, "Image is not in qcow2 format");
         ret = -EMEDIUMTYPE;
         goto fail;
     }
     if (header.version < 2 || header.version > 3) {
-        report_unsupported(bs, "QCOW version %d", header.version);
+        report_unsupported(bs, errp, "QCOW version %d", header.version);
         ret = -ENOTSUP;
         goto fail;
     }
@@ -412,6 +422,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         ret = bdrv_pread(bs->file, sizeof(header), s->unknown_header_fields,
                          s->unknown_header_fields_size);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not read unknown qcow2 header "
+                             "fields");
             goto fail;
         }
     }
@@ -430,8 +442,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     if (s->incompatible_features & ~QCOW2_INCOMPAT_MASK) {
         void *feature_table = NULL;
         qcow2_read_extensions(bs, header.header_length, ext_end,
-                              &feature_table);
-        report_unsupported_feature(bs, feature_table,
+                              &feature_table, NULL);
+        report_unsupported_feature(bs, errp, feature_table,
                                    s->incompatible_features &
                                    ~QCOW2_INCOMPAT_MASK);
         ret = -ENOTSUP;
@@ -442,8 +454,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         /* Corrupt images may not be written to unless they are being repaired
          */
         if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_CHECK)) {
-            error_report("qcow2: Image is corrupt; cannot be opened "
-                    "read/write.");
+            error_setg(errp, "qcow2: Image is corrupt; cannot be opened "
+                       "read/write");
             ret = -EACCES;
             goto fail;
         }
@@ -451,7 +463,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
     /* Check support for various header values */
     if (header.refcount_order != 4) {
-        report_unsupported(bs, "%d bit reference counts",
+        report_unsupported(bs, errp, "%d bit reference counts",
                            1 << header.refcount_order);
         ret = -ENOTSUP;
         goto fail;
@@ -460,10 +472,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
     if (header.cluster_bits < MIN_CLUSTER_BITS ||
         header.cluster_bits > MAX_CLUSTER_BITS) {
+        error_setg(errp, "Unsupported cluster size: 2^%i", header.cluster_bits);
         ret = -EINVAL;
         goto fail;
     }
     if (header.crypt_method > QCOW_CRYPT_AES) {
+        error_setg(errp, "Unsupported encryption method: %i",
+                   header.crypt_method);
         ret = -EINVAL;
         goto fail;
     }
@@ -492,6 +507,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
     l1_vm_state_index = size_to_l1(s, header.size);
     if (l1_vm_state_index > INT_MAX) {
+        error_setg(errp, "Image is too big");
         ret = -EFBIG;
         goto fail;
     }
@@ -500,6 +516,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     /* the L1 table must contain at least enough entries to put
        header.size bytes */
     if (s->l1_size < s->l1_vm_state_index) {
+        error_setg(errp, "L1 table is too small");
         ret = -EINVAL;
         goto fail;
     }
@@ -510,6 +527,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
                          s->l1_size * sizeof(uint64_t));
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not read L1 table");
             goto fail;
         }
         for(i = 0;i < s->l1_size; i++) {
@@ -530,6 +548,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
     ret = qcow2_refcount_init(bs);
     if (ret != 0) {
+        error_setg_errno(errp, -ret, "Could not initialize refcount handling");
         goto fail;
     }
 
@@ -537,7 +556,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     QTAILQ_INIT(&s->discards);
 
     /* read qcow2 extensions */
-    if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL)) {
+    if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL,
+        &local_err)) {
+        error_propagate(errp, local_err);
         ret = -EINVAL;
         goto fail;
     }
@@ -551,6 +572,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         ret = bdrv_pread(bs->file, header.backing_file_offset,
                          bs->backing_file, len);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not read backing file name");
             goto fail;
         }
         bs->backing_file[len] = '\0';
@@ -558,6 +580,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
     ret = qcow2_read_snapshots(bs);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not read snapshots");
         goto fail;
     }
 
@@ -566,6 +589,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         s->autoclear_features = 0;
         ret = qcow2_update_header(bs);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not update qcow2 header");
             goto fail;
         }
     }
@@ -580,6 +604,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
         ret = qcow2_check(bs, &result, BDRV_FIX_ERRORS);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not repair dirty image");
             goto fail;
         }
     }
@@ -588,8 +613,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
-        qerror_report_err(local_err);
-        error_free(local_err);
+        error_propagate(errp, local_err);
         ret = -EINVAL;
         goto fail;
     }
@@ -610,8 +634,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     qemu_opts_del(opts);
 
     if (s->use_lazy_refcounts && s->qcow_version < 3) {
-        qerror_report(ERROR_CLASS_GENERIC_ERROR, "Lazy refcounts require "
-            "a qcow2 image with at least qemu 1.1 compatibility level");
+        error_setg(errp, "Lazy refcounts require a qcow2 image with at least "
+                   "qemu 1.1 compatibility level");
         ret = -EINVAL;
         goto fail;
     }
@@ -1334,7 +1358,8 @@ static int preallocate(BlockDriverState *bs)
 static int qcow2_create2(const char *filename, int64_t total_size,
                          const char *backing_file, const char *backing_format,
                          int flags, size_t cluster_size, int prealloc,
-                         QEMUOptionParameter *options, int version)
+                         QEMUOptionParameter *options, int version,
+                         Error **errp)
 {
     /* Calculate cluster_bits */
     int cluster_bits;
@@ -1342,9 +1367,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     if (cluster_bits < MIN_CLUSTER_BITS || cluster_bits > MAX_CLUSTER_BITS ||
         (1 << cluster_bits) != cluster_size)
     {
-        error_report(
-            "Cluster size must be a power of two between %d and %dk",
-            1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
+        error_setg(errp, "Cluster size must be a power of two between %d and "
+                   "%dk", 1 << MIN_CLUSTER_BITS, 1 << (MAX_CLUSTER_BITS - 10));
         return -EINVAL;
     }
 
@@ -1363,15 +1387,18 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     BlockDriverState* bs;
     QCowHeader header;
     uint8_t* refcount_table;
+    Error *local_err = NULL;
     int ret;
 
-    ret = bdrv_create_file(filename, options, NULL);
+    ret = bdrv_create_file(filename, options, &local_err);
     if (ret < 0) {
+        error_propagate(errp, local_err);
         return ret;
     }
 
-    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
+    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
     if (ret < 0) {
+        error_propagate(errp, local_err);
         return ret;
     }
 
@@ -1401,6 +1428,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
 
     ret = bdrv_pwrite(bs, 0, &header, sizeof(header));
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not write qcow2 header");
         goto out;
     }
 
@@ -1410,6 +1438,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     g_free(refcount_table);
 
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not write refcount table");
         goto out;
     }
 
@@ -1423,13 +1452,16 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     BlockDriver* drv = bdrv_find_format("qcow2");
     assert(drv != NULL);
     ret = bdrv_open(bs, filename, NULL,
-        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
+        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
     if (ret < 0) {
+        error_propagate(errp, local_err);
         goto out;
     }
 
     ret = qcow2_alloc_clusters(bs, 2 * cluster_size);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not allocate clusters for qcow2 "
+                         "header and refcount table");
         goto out;
 
     } else if (ret != 0) {
@@ -1440,6 +1472,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     /* Okay, now that we have a valid image, let's give it the right size */
     ret = bdrv_truncate(bs, total_size * BDRV_SECTOR_SIZE);
     if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not resize image");
         goto out;
     }
 
@@ -1447,6 +1480,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
     if (backing_file) {
         ret = bdrv_change_backing_file(bs, backing_file, backing_format);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not assign backing file '%s' "
+                             "with format '%s'", backing_file, backing_format);
             goto out;
         }
     }
@@ -1458,6 +1493,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         ret = preallocate(bs);
         qemu_co_mutex_unlock(&s->lock);
         if (ret < 0) {
+            error_setg_errno(errp, -ret, "Could not preallocate metadata");
             goto out;
         }
     }
@@ -1478,6 +1514,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
     size_t cluster_size = DEFAULT_CLUSTER_SIZE;
     int prealloc = 0;
     int version = 3;
+    Error *local_err = NULL;
+    int ret;
 
     /* Read out options */
     while (options && options->name) {
@@ -1499,8 +1537,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
             } else if (!strcmp(options->value.s, "metadata")) {
                 prealloc = 1;
             } else {
-                fprintf(stderr, "Invalid preallocation mode: '%s'\n",
-                    options->value.s);
+                error_setg(errp, "Invalid preallocation mode: '%s'",
+                           options->value.s);
                 return -EINVAL;
             }
         } else if (!strcmp(options->name, BLOCK_OPT_COMPAT_LEVEL)) {
@@ -1511,8 +1549,8 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
             } else if (!strcmp(options->value.s, "1.1")) {
                 version = 3;
             } else {
-                fprintf(stderr, "Invalid compatibility level: '%s'\n",
-                    options->value.s);
+                error_setg(errp, "Invalid compatibility level: '%s'",
+                           options->value.s);
                 return -EINVAL;
             }
         } else if (!strcmp(options->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
@@ -1522,19 +1560,23 @@ static int qcow2_create(const char *filename, QEMUOptionParameter *options,
     }
 
     if (backing_file && prealloc) {
-        fprintf(stderr, "Backing file and preallocation cannot be used at "
-            "the same time\n");
+        error_setg(errp, "Backing file and preallocation cannot be used at "
+                   "the same time");
         return -EINVAL;
     }
 
     if (version < 3 && (flags & BLOCK_FLAG_LAZY_REFCOUNTS)) {
-        fprintf(stderr, "Lazy refcounts only supported with compatibility "
-                "level 1.1 and above (use compat=1.1 or greater)\n");
+        error_setg(errp, "Lazy refcounts only supported with compatibility "
+                   "level 1.1 and above (use compat=1.1 or greater)");
         return -EINVAL;
     }
 
-    return qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
-                         cluster_size, prealloc, options, version);
+    ret = qcow2_create2(filename, sectors, backing_file, backing_fmt, flags,
+                        cluster_size, prealloc, options, version, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
+    return ret;
 }
 
 static int qcow2_make_empty(BlockDriverState *bs)

From 2c78857bf6a9b5d06e17533b8f40fee14e087987 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Tue, 10 Sep 2013 13:59:43 +0200
Subject: [PATCH 28/33] qemu-iotests: Adjustments due to error propagation

When opening/creating images, propagating errors instead of immediately
emitting them on occurrence results in errors generally being printed on
a single line rather than being split up into multiple ones. This in
turn requires adjustments to some test results.

Also, test 060 used a sed to filter out the test image directory and
format by removing everything from the affected line after a certain
keyword; this now also removes the error message itself, which can be
fixed by using _filter_testdir and _filter_imgfmt.

Finally, _make_test_img in common.rc did not filter out the test image
directory etc. from stderr. This has been fixed through a redirection of
stderr to stdout (which is already done in _check_test_img and
_img_info).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/049.out   | 18 +++++++-----------
 tests/qemu-iotests/051.out   | 35 ++++++++++++-----------------------
 tests/qemu-iotests/054.out   |  4 ++--
 tests/qemu-iotests/060       |  2 +-
 tests/qemu-iotests/060.out   |  3 +--
 tests/qemu-iotests/common.rc |  2 +-
 6 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out
index d2f0efe16d..ceb23289fd 100644
--- a/tests/qemu-iotests/049.out
+++ b/tests/qemu-iotests/049.out
@@ -96,7 +96,7 @@ qemu-img: Image size must be less than 8 EiB!
 
 qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2
 qemu-img: qcow2 doesn't support shrinking images yet
-qemu-img: Formatting or formatting option not supported for file format 'qcow2'
+qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off 
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k
@@ -104,7 +104,7 @@ qemu-img: Image size must be less than 8 EiB!
 
 qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2
 qemu-img: qcow2 doesn't support shrinking images yet
-qemu-img: Formatting or formatting option not supported for file format 'qcow2'
+qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off 
 
 qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte
@@ -120,7 +120,7 @@ qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.
 
 qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2
 qemu-img: Parameter 'size' expects a size
-qemu-img: Invalid options for file format 'qcow2'.
+qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2'.
 
 == Check correct interpretation of suffixes for cluster size ==
 
@@ -163,13 +163,11 @@ qemu-img create -f qcow2 -o compat=1.1 TEST_DIR/t.qcow2 64M
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='1.1' encryption=off cluster_size=65536 lazy_refcounts=off 
 
 qemu-img create -f qcow2 -o compat=0.42 TEST_DIR/t.qcow2 64M
-Invalid compatibility level: '0.42'
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Invalid compatibility level: '0.42'
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.42' encryption=off cluster_size=65536 lazy_refcounts=off 
 
 qemu-img create -f qcow2 -o compat=foobar TEST_DIR/t.qcow2 64M
-Invalid compatibility level: 'foobar'
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Invalid compatibility level: 'foobar'
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='foobar' encryption=off cluster_size=65536 lazy_refcounts=off 
 
 == Check preallocation option ==
@@ -181,8 +179,7 @@ qemu-img create -f qcow2 -o preallocation=metadata TEST_DIR/t.qcow2 64M
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='metadata' lazy_refcounts=off 
 
 qemu-img create -f qcow2 -o preallocation=1234 TEST_DIR/t.qcow2 64M
-Invalid preallocation mode: '1234'
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Invalid preallocation mode: '1234'
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 encryption=off cluster_size=65536 preallocation='1234' lazy_refcounts=off 
 
 == Check encryption option ==
@@ -205,8 +202,7 @@ qemu-img create -f qcow2 -o compat=0.10,lazy_refcounts=off TEST_DIR/t.qcow2 64M
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=off 
 
 qemu-img create -f qcow2 -o compat=0.10,lazy_refcounts=on TEST_DIR/t.qcow2 64M
-Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
-qemu-img: TEST_DIR/t.qcow2: error while creating qcow2: Invalid argument
+qemu-img: TEST_DIR/t.qcow2: Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
 Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat='0.10' encryption=off cluster_size=65536 lazy_refcounts=on 
 
 *** done
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 86e989cc6a..88e8fa7de0 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -4,20 +4,16 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 === Unknown option ===
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=on: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=1234: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,unknown_opt=foo: could not open disk image TEST_DIR/t.qcow2: Block format 'qcow2' used by device 'ide0-hd0' doesn't support the option 'unknown_opt'
 
 
 === Enable and disable lazy refcounting on the command line, plus some invalid values ===
@@ -31,24 +27,20 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qququiquit
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: Parameter 'lazy-refcounts' expects 'on' or 'off'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: Parameter 'lazy-refcounts' expects 'on' or 'off'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=42: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: Parameter 'lazy-refcounts' expects 'on' or 'off'
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=foo: could not open disk image TEST_DIR/t.qcow2: Parameter 'lazy-refcounts' expects 'on' or 'off'
 
 
 === With version 2 images enabling lazy refcounts must fail ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=on: could not open disk image TEST_DIR/t.qcow2: Lazy refcounts require a qcow2 image with at least qemu 1.1 compatibility level
 
 Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,lazy-refcounts=off
 QEMU X.Y.Z monitor - type 'help' for more information
@@ -208,21 +200,18 @@ QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) qququiquit
 
 Testing: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: Can't use 'qcow2' as a block driver for the protocol level
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk image TEST_DIR/t.qcow2: Invalid argument
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,file.driver=qcow2: could not open disk image TEST_DIR/t.qcow2: Can't use 'qcow2' as a block driver for the protocol level
 
 
 === Parsing protocol from file name ===
 
 Testing: -hda foo:bar
-QEMU_PROG: -hda foo:bar: Unknown protocol
-QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: No such file or directory
+QEMU_PROG: -hda foo:bar: could not open disk image foo:bar: Unknown protocol
 
 Testing: -drive file=foo:bar
-QEMU_PROG: -drive file=foo:bar: Unknown protocol
-QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: No such file or directory
+QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: Unknown protocol
 
 Testing: -drive file.filename=foo:bar
-QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: No such file or directory
+QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open 'foo:bar': No such file or directory
 
 *** done
diff --git a/tests/qemu-iotests/054.out b/tests/qemu-iotests/054.out
index 2f357c271d..7161d6e50b 100644
--- a/tests/qemu-iotests/054.out
+++ b/tests/qemu-iotests/054.out
@@ -1,10 +1,10 @@
 QA output created by 054
 
 creating too large image (1 EB)
-qemu-img: The image size is too large for file format 'qcow2' (try using a larger cluster size)
+qemu-img: TEST_DIR/t.IMGFMT: The image size is too large for file format 'IMGFMT' (try using a larger cluster size)
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1152921504606846976 
 
 creating too large image (1 EB) using qcow2.py
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 
-qemu-img: Could not open 'TEST_DIR/t.qcow2': File too large
+qemu-img: Could not open 'TEST_DIR/t.qcow2': Image is too big
 *** done
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 65bb09f023..9bbc43b706 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -71,7 +71,7 @@ $QEMU_IO -c "write -P 0x2a 0 512" "$TEST_IMG" | _filter_qemu_io
 ./qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
 
 # Try to open the image R/W (which should fail)
-$QEMU_IO -c "read 0 512" "$TEST_IMG" 2>&1 | _filter_qemu_io | sed -e "s/can't open device .*$/can't open device/"
+$QEMU_IO -c "read 0 512" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir | _filter_imgfmt
 
 # Try to open it RO (which should succeed)
 $QEMU_IO -c "read 0 512" -r "$TEST_IMG" | _filter_qemu_io
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index ca4583a4a4..648f7437a2 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -11,8 +11,7 @@ incompatible_features     0x0
 qcow2: Preventing invalid write on metadata (overlaps with active L1 table); image marked as corrupt.
 write failed: Input/output error
 incompatible_features     0x2
-qcow2: Image is corrupt; cannot be opened read/write.
-qemu-io: can't open device
+qemu-io: can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write
 no file open, try 'help open'
 read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 88fecf7870..28b39e429e 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -123,7 +123,7 @@ _make_test_img()
     fi
 
     # XXX(hch): have global image options?
-    $QEMU_IMG create -f $IMGFMT $extra_img_options $img_name $image_size | \
+    $QEMU_IMG create -f $IMGFMT $extra_img_options $img_name $image_size 2>&1 | \
         sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
             -e "s#$TEST_DIR#TEST_DIR#g" \
             -e "s#$IMGFMT#IMGFMT#g" \

From 70c60c089fdc6bf8a79324e492c13e8c08d55942 Mon Sep 17 00:00:00 2001
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Wed, 11 Sep 2013 16:42:35 +0200
Subject: [PATCH 29/33] coroutine: add ./configure --disable-coroutine-pool

The 'gthread' coroutine backend was written before the freelist (aka
pool) existed in qemu-coroutine.c.

This means that every thread is expected to exit when its coroutine
terminates.  It is not possible to reuse threads from a pool.

This patch automatically disables the pool when 'gthread' is used.  This
allows the 'gthread' backend to work again (for example,
tests/test-coroutine completes successfully instead of hanging).

I considered implementing thread reuse but I don't want quirks like CPU
affinity differences due to coroutine threads being recycled.  The
'gthread' backend is a reference backend and it's therefore okay to skip
the pool optimization.

Note this patch also makes it easy to toggle the pool for benchmarking
purposes:

  ./configure --with-coroutine-backend=ucontext \
              --disable-coroutine-pool

Reported-by: Gabriel Kerneis <gabriel@kerneis.info>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Gabriel Kerneis <gabriel@kerneis.info>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 configure        | 24 ++++++++++++++++++++++++
 qemu-coroutine.c | 32 ++++++++++++++++++--------------
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/configure b/configure
index b49902841e..1b6f68b691 100755
--- a/configure
+++ b/configure
@@ -238,6 +238,7 @@ win_sdk="no"
 want_tools="yes"
 libiscsi=""
 coroutine=""
+coroutine_pool=""
 seccomp=""
 glusterfs=""
 glusterfs_discard="no"
@@ -888,6 +889,10 @@ for opt do
   ;;
   --with-coroutine=*) coroutine="$optarg"
   ;;
+  --disable-coroutine-pool) coroutine_pool="no"
+  ;;
+  --enable-coroutine-pool) coroutine_pool="yes"
+  ;;
   --disable-docs) docs="no"
   ;;
   --enable-docs) docs="yes"
@@ -1189,6 +1194,8 @@ echo "  --disable-seccomp        disable seccomp support"
 echo "  --enable-seccomp         enables seccomp support"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "                           gthread, ucontext, sigaltstack, windows"
+echo "  --disable-coroutine-pool disable coroutine freelist (worse performance)"
+echo "  --enable-coroutine-pool  enable coroutine freelist (better performance)"
 echo "  --enable-glusterfs       enable GlusterFS backend"
 echo "  --disable-glusterfs      disable GlusterFS backend"
 echo "  --enable-gcov            enable test coverage analysis with gcov"
@@ -3362,6 +3369,17 @@ else
   esac
 fi
 
+if test "$coroutine_pool" = ""; then
+  if test "$coroutine" = "gthread"; then
+    coroutine_pool=no
+  else
+    coroutine_pool=yes
+  fi
+fi
+if test "$coroutine" = "gthread" -a "$coroutine_pool" = "yes"; then
+  error_exit "'gthread' coroutine backend does not support pool (use --disable-coroutine-pool)"
+fi
+
 ##########################################
 # check if we have open_by_handle_at
 
@@ -3733,6 +3751,7 @@ echo "build guest agent $guest_agent"
 echo "QGA VSS support   $guest_agent_with_vss"
 echo "seccomp support   $seccomp"
 echo "coroutine backend $coroutine"
+echo "coroutine pool    $coroutine_pool"
 echo "GlusterFS support $glusterfs"
 echo "virtio-blk-data-plane $virtio_blk_data_plane"
 echo "gcov              $gcov_tool"
@@ -4092,6 +4111,11 @@ if test "$rbd" = "yes" ; then
 fi
 
 echo "CONFIG_COROUTINE_BACKEND=$coroutine" >> $config_host_mak
+if test "$coroutine_pool" = "yes" ; then
+  echo "CONFIG_COROUTINE_POOL=1" >> $config_host_mak
+else
+  echo "CONFIG_COROUTINE_POOL=0" >> $config_host_mak
+fi
 
 if test "$open_by_handle_at" = "yes" ; then
   echo "CONFIG_OPEN_BY_HANDLE=y" >> $config_host_mak
diff --git a/qemu-coroutine.c b/qemu-coroutine.c
index 423430d3a0..470852100a 100644
--- a/qemu-coroutine.c
+++ b/qemu-coroutine.c
@@ -30,15 +30,17 @@ static unsigned int pool_size;
 
 Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
 {
-    Coroutine *co;
+    Coroutine *co = NULL;
 
-    qemu_mutex_lock(&pool_lock);
-    co = QSLIST_FIRST(&pool);
-    if (co) {
-        QSLIST_REMOVE_HEAD(&pool, pool_next);
-        pool_size--;
+    if (CONFIG_COROUTINE_POOL) {
+        qemu_mutex_lock(&pool_lock);
+        co = QSLIST_FIRST(&pool);
+        if (co) {
+            QSLIST_REMOVE_HEAD(&pool, pool_next);
+            pool_size--;
+        }
+        qemu_mutex_unlock(&pool_lock);
     }
-    qemu_mutex_unlock(&pool_lock);
 
     if (!co) {
         co = qemu_coroutine_new();
@@ -51,15 +53,17 @@ Coroutine *qemu_coroutine_create(CoroutineEntry *entry)
 
 static void coroutine_delete(Coroutine *co)
 {
-    qemu_mutex_lock(&pool_lock);
-    if (pool_size < POOL_MAX_SIZE) {
-        QSLIST_INSERT_HEAD(&pool, co, pool_next);
-        co->caller = NULL;
-        pool_size++;
+    if (CONFIG_COROUTINE_POOL) {
+        qemu_mutex_lock(&pool_lock);
+        if (pool_size < POOL_MAX_SIZE) {
+            QSLIST_INSERT_HEAD(&pool, co, pool_next);
+            co->caller = NULL;
+            pool_size++;
+            qemu_mutex_unlock(&pool_lock);
+            return;
+        }
         qemu_mutex_unlock(&pool_lock);
-        return;
     }
-    qemu_mutex_unlock(&pool_lock);
 
     qemu_coroutine_delete(co);
 }

From c745bfb4300206280ce6156b4bafe765f610057c Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed, 11 Sep 2013 18:47:52 +0200
Subject: [PATCH 30/33] qemu-img: fix invalid JSON

Single quotes for JSON are a QMP-ism, use real JSON in
qemu-img output.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 6a1ba697c5..926f0a0feb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1839,7 +1839,7 @@ static void dump_map_entry(OutputFormat output_format, MapEntry *e,
                (e->flags & BDRV_BLOCK_ZERO) ? "true" : "false",
                (e->flags & BDRV_BLOCK_DATA) ? "true" : "false");
         if (e->flags & BDRV_BLOCK_OFFSET_VALID) {
-            printf(", 'offset': %"PRId64"", e->offset);
+            printf(", \"offset\": %"PRId64"", e->offset);
         }
         putchar('}');
 

From 4aa846f25e7cf14c77f699d8c1dfdfeddb091161 Mon Sep 17 00:00:00 2001
From: Bharata B Rao <bharata@linux.vnet.ibm.com>
Date: Thu, 12 Sep 2013 14:07:59 +0530
Subject: [PATCH 31/33] qemu-iotests: Cleanup test image in test number 007

qemu-iotests number 007 doesn't do test image cleanup. This will affect
those protocols that expect a clean state before every test. Hence
ensure that test image is cleaned up in this test.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/007 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/007 b/tests/qemu-iotests/007
index c454f2c8ec..6fa760330d 100755
--- a/tests/qemu-iotests/007
+++ b/tests/qemu-iotests/007
@@ -30,7 +30,7 @@ status=1	# failure is the default!
 
 _cleanup()
 {
-#	_cleanup_test_img
+	_cleanup_test_img
 	true
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15

From aa3fe714f70654da47d9c2659b2d9ee295a9d930 Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Thu, 12 Sep 2013 14:57:27 +0200
Subject: [PATCH 32/33] block: Assert validity of BdrvActionOps

In qmp_transaction, assert that the BdrvActionOps to be used is actually
valid.

This assertion failing is very improbable, however, it might happen, if
a new TransactionActionKind is introduced "out of order" and the
actions[] array is not updated.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2ab236a82d..80605a2bac 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1286,6 +1286,8 @@ void qmp_transaction(TransactionActionList *dev_list, Error **errp)
         assert(dev_info->kind < ARRAY_SIZE(actions));
 
         ops = &actions[dev_info->kind];
+        assert(ops->instance_size > 0);
+
         state = g_malloc0(ops->instance_size);
         state->ops = ops;
         state->action = dev_info;

From c21bddf27fd8029890e9fc2ee314788919eababf Mon Sep 17 00:00:00 2001
From: Max Reitz <mreitz@redhat.com>
Date: Fri, 13 Sep 2013 10:37:12 +0200
Subject: [PATCH 33/33] qemu-iotests: Fix test 038

Test 038 uses asynchronous I/O, resulting (potentially) in a different
output for every run (regarding the order of the I/O accesses). This can
be fixed by simply sorting the I/O access messages, since their order is
irrelevant anyway (for this asynchonous I/O).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/038     |   3 +-
 tests/qemu-iotests/038.out | 122 ++++++++++++++++++-------------------
 2 files changed, 63 insertions(+), 62 deletions(-)

diff --git a/tests/qemu-iotests/038 b/tests/qemu-iotests/038
index 36125eab1e..90de1a73d9 100755
--- a/tests/qemu-iotests/038
+++ b/tests/qemu-iotests/038
@@ -95,7 +95,8 @@ function overlay_io()
 }
 
 overlay_io | $QEMU_IO $TEST_IMG | _filter_qemu_io |\
-	sed -e 's/bytes at offset [0-9]*/bytes at offset XXX/g'
+    sed -e 's/bytes at offset [0-9]*/bytes at offset XXX/g' \
+    -e 's/qemu-io> //g' | paste - - | sort | tr '\t' '\n'
 
 echo
 echo "== Verify image content =="
diff --git a/tests/qemu-iotests/038.out b/tests/qemu-iotests/038.out
index 9cd0cd8771..96c2f849bb 100644
--- a/tests/qemu-iotests/038.out
+++ b/tests/qemu-iotests/038.out
@@ -517,7 +517,65 @@ qemu-io> wrote 65536/65536 bytes at offset 16711680
 qemu-io> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=6442450944 backing_file='TEST_DIR/t.IMGFMT.base' 
 
 == Some concurrent requests touching the same cluster ==
-qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> qemu-io> wrote 65536/65536 bytes at offset XXX
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset XXX
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 65536/65536 bytes at offset XXX
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -577,8 +635,6 @@ wrote 65536/65536 bytes at offset XXX
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 65536/65536 bytes at offset XXX
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 81920/81920 bytes at offset XXX
-80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 65536/65536 bytes at offset XXX
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 65536/65536 bytes at offset XXX
@@ -647,64 +703,8 @@ wrote 65536/65536 bytes at offset XXX
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 81920/81920 bytes at offset XXX
 80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset XXX
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset XXX
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 == Verify image content ==
 qemu-io> read 4096/4096 bytes at offset 2064384