[Qemu-devel] [PULL 00/22] Block layer core and image format patches

[Qemu-devel] [PULL 00/22] Block layer core and image format patches

[Kevin Wolf]

The following changes since commit 8b6db32a4ec47d1171ccfa21d557096b99f4eef0:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-05-22 13:25:40 +0100)

are available in the git repository at:


  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 4120201d2fcfc24404fe6eb6b761b66bc35bca16:

  MAINTAINERS: Split "Block QAPI, monitor, command line" off core (2015-05-22 17:08:09 +0200)

----------------------------------------------------------------
Block layer core and image format patches

----------------------------------------------------------------
Alberto Garcia (7):
      qcow2: use one single memory block for the L2/refcount cache tables
      qcow2: simplify qcow2_cache_put() and qcow2_cache_entry_mark_dirty()
      qcow2: use an LRU algorithm to replace entries from the L2 cache
      qcow2: remove qcow2_cache_find_entry_to_replace()
      qcow2: use a hash to look for entries in the L2 cache
      qcow2: make qcow2_cache_put() a void function
      qcow2: style fixes in qcow2-cache.c

Christoph Hellwig (1):
      nvme: support NVME_VOLATILE_WRITE_CACHE feature

Daniel P. Berrange (5):
      qcow2/qcow: protect against uninitialized encryption key
      util: move read_password method out of qemu-img into osdep/oslib
      util: allow \n to terminate password input
      qemu-io: prompt for encryption keys when required
      tests: add test case for encrypted qcow2 read/write

Eric Blake (1):
      qemu-io: Use getopt() correctly

Fam Zheng (5):
      vmdk: Fix next_cluster_sector for compressed write
      vmdk: Fix overflow if l1_size is 0x20000000
      block: Detect multiplication overflow in bdrv_getlength
      qemu-iotests: qemu-img info on afl VMDK image with a huge capacity
      qemu-iotests: Make debugging python tests easier

Kevin Wolf (2):
      qcow2: Flush pending discards before allocating cluster
      MAINTAINERS: Add header files to Block Layer Core section

Markus Armbruster (1):
      MAINTAINERS: Split "Block QAPI, monitor, command line" off core

 MAINTAINERS                                    |   9 ++
 block.c                                        |   1 +
 block/qcow.c                                   |  10 +-
 block/qcow2-cache.c                            | 171 +++++++++++--------------
 block/qcow2-cluster.c                          |  65 +++-------
 block/qcow2-refcount.c                         |  42 +++---
 block/qcow2.c                                  |  18 ++-
 block/qcow2.h                                  |   5 +-
 block/vmdk.c                                   |  17 ++-
 hw/block/nvme.c                                |   3 +
 include/qemu/osdep.h                           |   2 +
 qemu-img.c                                     |  93 +-------------
 qemu-io-cmds.c                                 |  16 +--
 qemu-io.c                                      |  23 +++-
 tests/qemu-iotests/059                         |   5 +
 tests/qemu-iotests/059.out                     |   3 +
 tests/qemu-iotests/134                         |  69 ++++++++++
 tests/qemu-iotests/134.out                     |  46 +++++++
 tests/qemu-iotests/check                       |  12 +-
 tests/qemu-iotests/common                      |   6 +
 tests/qemu-iotests/group                       |   1 +
 tests/qemu-iotests/iotests.py                  |  14 +-
 tests/qemu-iotests/sample_images/afl9.vmdk.bz2 | Bin 0 -> 178 bytes
 util/oslib-posix.c                             |  67 ++++++++++
 util/oslib-win32.c                             |  24 ++++
 25 files changed, 429 insertions(+), 293 deletions(-)
 create mode 100755 tests/qemu-iotests/134
 create mode 100644 tests/qemu-iotests/134.out
 create mode 100644 tests/qemu-iotests/sample_images/afl9.vmdk.bz2

[Qemu-devel] [PULL 01/22] qcow2: Flush pending discards before allocating cluster

[Kevin Wolf]

Before a freed cluster can be reused, pending discards for this cluster
must be processed.

The original assumption was that this was not a problem because discards
are only cached during discard/write zeroes operations, which are
synchronous so that no concurrent write requests can cause cluster
allocations.

However, the discard/write zeroes operation itself can allocate a new L2
table (and it has to in order to put zero flags there), so make sure we
can cope with the situation.

This fixes https://bugs.launchpad.net/bugs/1349972.

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-refcount.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index f47260b..83467c3 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -833,6 +833,11 @@ static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size)
     uint64_t i, nb_clusters, refcount;
     int ret;
 
+    /* We can't allocate clusters if they may still be queued for discard. */
+    if (s->cache_discards) {
+        qcow2_process_discards(bs, 0);
+    }
+
     nb_clusters = size_to_clusters(s, size);
 retry:
     for(i = 0; i < nb_clusters; i++) {
-- 
1.8.3.1

[Qemu-devel] [PULL 02/22] nvme: support NVME_VOLATILE_WRITE_CACHE feature

[Kevin Wolf]

From: Christoph Hellwig <hch@lst.de>

The SCSI emulation in the Linux NVMe driver really wants to know
if a device has a volatile write cache.  Given that qemu has moved
away from a model where we report the backing store WCE bit to
one where the WCE bit is supposed to be part of the migratable
guest-visible state we always return 1 here.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Keith Busch <keith.busch@intel.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/nvme.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index ad988d7..4b6d5e6 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -479,6 +479,9 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
         req->cqe.result =
             cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16));
         break;
+    case NVME_VOLATILE_WRITE_CACHE:
+        req->cqe.result = cpu_to_le32(1);
+        break;
     default:
         return NVME_INVALID_FIELD | NVME_DNR;
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 03/22] vmdk: Fix next_cluster_sector for compressed write

[Kevin Wolf]

From: Fam Zheng <famz@redhat.com>

This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster
allocation).

Sometimes, write_len could be larger than cluster size, because it
contains both data and marker.  We must advance next_cluster_sector in
this case, otherwise the image gets corrupted.

Cc: qemu-stable@nongnu.org
Reported-by: Antoni Villalonga <qemu-list@friki.cat>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 1c5e2ef..4b4a862 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1302,6 +1302,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
     uLongf buf_len;
     const uint8_t *write_buf = buf;
     int write_len = nb_sectors * 512;
+    int64_t write_offset;
+    int64_t write_end_sector;
 
     if (extent->compressed) {
         if (!extent->has_marker) {
@@ -1320,10 +1322,14 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
         write_buf = (uint8_t *)data;
         write_len = buf_len + sizeof(VmdkGrainMarker);
     }
-    ret = bdrv_pwrite(extent->file,
-                        cluster_offset + offset_in_cluster,
-                        write_buf,
-                        write_len);
+    write_offset = cluster_offset + offset_in_cluster,
+    ret = bdrv_pwrite(extent->file, write_offset, write_buf, write_len);
+
+    write_end_sector = DIV_ROUND_UP(write_offset + write_len, BDRV_SECTOR_SIZE);
+
+    extent->next_cluster_sector = MAX(extent->next_cluster_sector,
+                                      write_end_sector);
+
     if (ret != write_len) {
         ret = ret < 0 ? ret : -EIO;
         goto out;
-- 
1.8.3.1

[Qemu-devel] [PULL 04/22] vmdk: Fix overflow if l1_size is 0x20000000

[Kevin Wolf]

From: Fam Zheng <famz@redhat.com>

Richard Jones caught this bug with afl fuzzer.

In fact, that's the only possible value to overflow (extent->l1_size =
0x20000000) l1_size:

l1_size = extent->l1_size * sizeof(long) => 0x80000000;

g_try_malloc returns NULL because l1_size is interpreted as negative
during type casting from 'int' to 'gsize', which yields a enormous
value. Hence, by coincidence, we get a "not too bad" behavior:

qemu-img: Could not open '/tmp/afl6.img': Could not open
'/tmp/afl6.img': Cannot allocate memory

Values larger than 0x20000000 will be refused by the validation in
vmdk_add_extent.

Values smaller than 0x20000000 will not overflow l1_size.

Cc: qemu-stable@nongnu.org
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vmdk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 4b4a862..b66745d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -451,7 +451,8 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,
                             Error **errp)
 {
     int ret;
-    int l1_size, i;
+    size_t l1_size;
+    int i;
 
     /* read the L1 table */
     l1_size = extent->l1_size * sizeof(uint32_t);
-- 
1.8.3.1

[Qemu-devel] [PULL 05/22] qcow2: use one single memory block for the L2/refcount cache tables

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

The qcow2 L2/refcount cache contains one separate table for each cache
entry. Doing one allocation per table adds unnecessary overhead and it
also requires us to store the address of each table separately.

Since the size of the cache is constant during its lifetime, it's
better to have an array that contains all the tables using one single
allocation.

In my tests measuring freshly created caches with sizes 128MB (L2) and
32MB (refcount) this uses around 10MB of RAM less.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c    | 55 ++++++++++++++++++++++++--------------------------
 block/qcow2-cluster.c  | 12 +++++------
 block/qcow2-refcount.c |  8 +++++---
 block/qcow2.h          |  3 ++-
 4 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index b115549..f0dfb69 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -28,7 +28,6 @@
 #include "trace.h"
 
 typedef struct Qcow2CachedTable {
-    void*   table;
     int64_t offset;
     bool    dirty;
     int     cache_hits;
@@ -40,39 +39,35 @@ struct Qcow2Cache {
     struct Qcow2Cache*      depends;
     int                     size;
     bool                    depends_on_flush;
+    void                   *table_array;
 };
 
+static inline void *qcow2_cache_get_table_addr(BlockDriverState *bs,
+                    Qcow2Cache *c, int table)
+{
+    BDRVQcowState *s = bs->opaque;
+    return (uint8_t *) c->table_array + (size_t) table * s->cluster_size;
+}
+
 Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
 {
     BDRVQcowState *s = bs->opaque;
     Qcow2Cache *c;
-    int i;
 
     c = g_new0(Qcow2Cache, 1);
     c->size = num_tables;
     c->entries = g_try_new0(Qcow2CachedTable, num_tables);
-    if (!c->entries) {
-        goto fail;
-    }
-
-    for (i = 0; i < c->size; i++) {
-        c->entries[i].table = qemu_try_blockalign(bs->file, s->cluster_size);
-        if (c->entries[i].table == NULL) {
-            goto fail;
-        }
+    c->table_array = qemu_try_blockalign(bs->file,
+                                         (size_t) num_tables * s->cluster_size);
+
+    if (!c->entries || !c->table_array) {
+        qemu_vfree(c->table_array);
+        g_free(c->entries);
+        g_free(c);
+        c = NULL;
     }
 
     return c;
-
-fail:
-    if (c->entries) {
-        for (i = 0; i < c->size; i++) {
-            qemu_vfree(c->entries[i].table);
-        }
-    }
-    g_free(c->entries);
-    g_free(c);
-    return NULL;
 }
 
 int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c)
@@ -81,9 +76,9 @@ int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c)
 
     for (i = 0; i < c->size; i++) {
         assert(c->entries[i].ref == 0);
-        qemu_vfree(c->entries[i].table);
     }
 
+    qemu_vfree(c->table_array);
     g_free(c->entries);
     g_free(c);
 
@@ -151,8 +146,8 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, Qcow2Cache *c, int i)
         BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE);
     }
 
-    ret = bdrv_pwrite(bs->file, c->entries[i].offset, c->entries[i].table,
-        s->cluster_size);
+    ret = bdrv_pwrite(bs->file, c->entries[i].offset,
+                      qcow2_cache_get_table_addr(bs, c, i), s->cluster_size);
     if (ret < 0) {
         return ret;
     }
@@ -304,7 +299,8 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
             BLKDBG_EVENT(bs->file, BLKDBG_L2_LOAD);
         }
 
-        ret = bdrv_pread(bs->file, offset, c->entries[i].table, s->cluster_size);
+        ret = bdrv_pread(bs->file, offset, qcow2_cache_get_table_addr(bs, c, i),
+                         s->cluster_size);
         if (ret < 0) {
             return ret;
         }
@@ -319,7 +315,7 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
 found:
     c->entries[i].cache_hits++;
     c->entries[i].ref++;
-    *table = c->entries[i].table;
+    *table = qcow2_cache_get_table_addr(bs, c, i);
 
     trace_qcow2_cache_get_done(qemu_coroutine_self(),
                                c == s->l2_table_cache, i);
@@ -344,7 +340,7 @@ int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
     int i;
 
     for (i = 0; i < c->size; i++) {
-        if (c->entries[i].table == *table) {
+        if (qcow2_cache_get_table_addr(bs, c, i) == *table) {
             goto found;
         }
     }
@@ -358,12 +354,13 @@ found:
     return 0;
 }
 
-void qcow2_cache_entry_mark_dirty(Qcow2Cache *c, void *table)
+void qcow2_cache_entry_mark_dirty(BlockDriverState *bs, Qcow2Cache *c,
+     void *table)
 {
     int i;
 
     for (i = 0; i < c->size; i++) {
-        if (c->entries[i].table == table) {
+        if (qcow2_cache_get_table_addr(bs, c, i) == table) {
             goto found;
         }
     }
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ed2b44d..5cd418a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -263,7 +263,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table)
     BLKDBG_EVENT(bs->file, BLKDBG_L2_ALLOC_WRITE);
 
     trace_qcow2_l2_allocate_write_l2(bs, l1_index);
-    qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+    qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
     ret = qcow2_cache_flush(bs, s->l2_table_cache);
     if (ret < 0) {
         goto fail;
@@ -692,7 +692,7 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
     /* compressed clusters never have the copied flag */
 
     BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE_COMPRESSED);
-    qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+    qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
     l2_table[l2_index] = cpu_to_be64(cluster_offset);
     ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
     if (ret < 0) {
@@ -771,7 +771,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
     if (ret < 0) {
         goto err;
     }
-    qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+    qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
 
     assert(l2_index + m->nb_clusters <= s->l2_size);
     for (i = 0; i < m->nb_clusters; i++) {
@@ -1470,7 +1470,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
         }
 
         /* First remove L2 entries */
-        qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+        qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
         if (!full_discard && s->qcow_version >= 3) {
             l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
         } else {
@@ -1558,7 +1558,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
         old_offset = be64_to_cpu(l2_table[l2_index + i]);
 
         /* Update L2 entries */
-        qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+        qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
         if (old_offset & QCOW_OFLAG_COMPRESSED) {
             l2_table[l2_index + i] = cpu_to_be64(QCOW_OFLAG_ZERO);
             qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
@@ -1760,7 +1760,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
 
         if (is_active_l1) {
             if (l2_dirty) {
-                qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+                qcow2_cache_entry_mark_dirty(bs, 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);
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 83467c3..0707f94 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -424,7 +424,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
 
     /* Now the new refcount block needs to be written to disk */
     BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_ALLOC_WRITE);
-    qcow2_cache_entry_mark_dirty(s->refcount_block_cache, *refcount_block);
+    qcow2_cache_entry_mark_dirty(bs, s->refcount_block_cache, *refcount_block);
     ret = qcow2_cache_flush(bs, s->refcount_block_cache);
     if (ret < 0) {
         goto fail_block;
@@ -737,7 +737,8 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
         }
         old_table_index = table_index;
 
-        qcow2_cache_entry_mark_dirty(s->refcount_block_cache, refcount_block);
+        qcow2_cache_entry_mark_dirty(bs, s->refcount_block_cache,
+                                     refcount_block);
 
         /* we can update the count and save it */
         block_index = cluster_index & (s->refcount_block_size - 1);
@@ -1182,7 +1183,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
                             s->refcount_block_cache);
                     }
                     l2_table[j] = cpu_to_be64(offset);
-                    qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_table);
+                    qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache,
+                                                 l2_table);
                 }
             }
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 422b825..5d0995f 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -574,7 +574,8 @@ int qcow2_read_snapshots(BlockDriverState *bs);
 Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables);
 int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c);
 
-void qcow2_cache_entry_mark_dirty(Qcow2Cache *c, void *table);
+void qcow2_cache_entry_mark_dirty(BlockDriverState *bs, Qcow2Cache *c,
+     void *table);
 int qcow2_cache_flush(BlockDriverState *bs, Qcow2Cache *c);
 int qcow2_cache_set_dependency(BlockDriverState *bs, Qcow2Cache *c,
     Qcow2Cache *dependency);
-- 
1.8.3.1

[Qemu-devel] [PULL 06/22] qcow2: simplify qcow2_cache_put() and qcow2_cache_entry_mark_dirty()

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

Since all tables are now stored together, it is possible to obtain
the position of a particular table directly from its address, so the
operation becomes O(1).

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index f0dfb69..6e78c8f 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -49,6 +49,16 @@ static inline void *qcow2_cache_get_table_addr(BlockDriverState *bs,
     return (uint8_t *) c->table_array + (size_t) table * s->cluster_size;
 }
 
+static inline int qcow2_cache_get_table_idx(BlockDriverState *bs,
+                  Qcow2Cache *c, void *table)
+{
+    BDRVQcowState *s = bs->opaque;
+    ptrdiff_t table_offset = (uint8_t *) table - (uint8_t *) c->table_array;
+    int idx = table_offset / s->cluster_size;
+    assert(idx >= 0 && idx < c->size && table_offset % s->cluster_size == 0);
+    return idx;
+}
+
 Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
 {
     BDRVQcowState *s = bs->opaque;
@@ -337,16 +347,12 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
 
 int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
 {
-    int i;
+    int i = qcow2_cache_get_table_idx(bs, c, *table);
 
-    for (i = 0; i < c->size; i++) {
-        if (qcow2_cache_get_table_addr(bs, c, i) == *table) {
-            goto found;
-        }
+    if (c->entries[i].offset == 0) {
+        return -ENOENT;
     }
-    return -ENOENT;
 
-found:
     c->entries[i].ref--;
     *table = NULL;
 
@@ -357,15 +363,7 @@ found:
 void qcow2_cache_entry_mark_dirty(BlockDriverState *bs, Qcow2Cache *c,
      void *table)
 {
-    int i;
-
-    for (i = 0; i < c->size; i++) {
-        if (qcow2_cache_get_table_addr(bs, c, i) == table) {
-            goto found;
-        }
-    }
-    abort();
-
-found:
+    int i = qcow2_cache_get_table_idx(bs, c, table);
+    assert(c->entries[i].offset != 0);
     c->entries[i].dirty = true;
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 07/22] qcow2: use an LRU algorithm to replace entries from the L2 cache

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

The current algorithm to evict entries from the cache gives always
preference to those in the lowest positions. As the size of the cache
increases, the chances of the later elements of being removed decrease
exponentially.

In a scenario with random I/O and lots of cache misses, entries in
positions 8 and higher are rarely (if ever) evicted. This can be seen
even with the default cache size, but with larger caches the problem
becomes more obvious.

Using an LRU algorithm makes the chances of being removed from the
cache independent from the position.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 6e78c8f..3a31895 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -28,10 +28,10 @@
 #include "trace.h"
 
 typedef struct Qcow2CachedTable {
-    int64_t offset;
-    bool    dirty;
-    int     cache_hits;
-    int     ref;
+    int64_t  offset;
+    bool     dirty;
+    uint64_t lru_counter;
+    int      ref;
 } Qcow2CachedTable;
 
 struct Qcow2Cache {
@@ -40,6 +40,7 @@ struct Qcow2Cache {
     int                     size;
     bool                    depends_on_flush;
     void                   *table_array;
+    uint64_t                lru_counter;
 };
 
 static inline void *qcow2_cache_get_table_addr(BlockDriverState *bs,
@@ -233,16 +234,18 @@ int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c)
     for (i = 0; i < c->size; i++) {
         assert(c->entries[i].ref == 0);
         c->entries[i].offset = 0;
-        c->entries[i].cache_hits = 0;
+        c->entries[i].lru_counter = 0;
     }
 
+    c->lru_counter = 0;
+
     return 0;
 }
 
 static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c)
 {
     int i;
-    int min_count = INT_MAX;
+    uint64_t min_lru_counter = UINT64_MAX;
     int min_index = -1;
 
 
@@ -251,15 +254,9 @@ static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c)
             continue;
         }
 
-        if (c->entries[i].cache_hits < min_count) {
+        if (c->entries[i].lru_counter < min_lru_counter) {
             min_index = i;
-            min_count = c->entries[i].cache_hits;
-        }
-
-        /* Give newer hits priority */
-        /* TODO Check how to optimize the replacement strategy */
-        if (c->entries[i].cache_hits > 1) {
-            c->entries[i].cache_hits /= 2;
+            min_lru_counter = c->entries[i].lru_counter;
         }
     }
 
@@ -316,14 +313,10 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
         }
     }
 
-    /* Give the table some hits for the start so that it won't be replaced
-     * immediately. The number 32 is completely arbitrary. */
-    c->entries[i].cache_hits = 32;
     c->entries[i].offset = offset;
 
     /* And return the right table */
 found:
-    c->entries[i].cache_hits++;
     c->entries[i].ref++;
     *table = qcow2_cache_get_table_addr(bs, c, i);
 
@@ -356,6 +349,10 @@ int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
     c->entries[i].ref--;
     *table = NULL;
 
+    if (c->entries[i].ref == 0) {
+        c->entries[i].lru_counter = ++c->lru_counter;
+    }
+
     assert(c->entries[i].ref >= 0);
     return 0;
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 08/22] qcow2: remove qcow2_cache_find_entry_to_replace()

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

A cache miss means that the whole array was traversed and the entry
we were looking for was not found, so there's no need to traverse it
again in order to select an entry to replace.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c | 45 ++++++++++++++++-----------------------------
 1 file changed, 16 insertions(+), 29 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 3a31895..2035cd8 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -242,51 +242,38 @@ int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c)
     return 0;
 }
 
-static int qcow2_cache_find_entry_to_replace(Qcow2Cache *c)
-{
-    int i;
-    uint64_t min_lru_counter = UINT64_MAX;
-    int min_index = -1;
-
-
-    for (i = 0; i < c->size; i++) {
-        if (c->entries[i].ref) {
-            continue;
-        }
-
-        if (c->entries[i].lru_counter < min_lru_counter) {
-            min_index = i;
-            min_lru_counter = c->entries[i].lru_counter;
-        }
-    }
-
-    if (min_index == -1) {
-        /* This can't happen in current synchronous code, but leave the check
-         * here as a reminder for whoever starts using AIO with the cache */
-        abort();
-    }
-    return min_index;
-}
-
 static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
     uint64_t offset, void **table, bool read_from_disk)
 {
     BDRVQcowState *s = bs->opaque;
     int i;
     int ret;
+    uint64_t min_lru_counter = UINT64_MAX;
+    int min_lru_index = -1;
 
     trace_qcow2_cache_get(qemu_coroutine_self(), c == s->l2_table_cache,
                           offset, read_from_disk);
 
     /* Check if the table is already cached */
     for (i = 0; i < c->size; i++) {
-        if (c->entries[i].offset == offset) {
+        const Qcow2CachedTable *t = &c->entries[i];
+        if (t->offset == offset) {
             goto found;
         }
+        if (t->ref == 0 && t->lru_counter < min_lru_counter) {
+            min_lru_counter = t->lru_counter;
+            min_lru_index = i;
+        }
+    }
+
+    if (min_lru_index == -1) {
+        /* This can't happen in current synchronous code, but leave the check
+         * here as a reminder for whoever starts using AIO with the cache */
+        abort();
     }
 
-    /* If not, write a table back and replace it */
-    i = qcow2_cache_find_entry_to_replace(c);
+    /* Cache miss: write a table back and replace it */
+    i = min_lru_index;
     trace_qcow2_cache_get_replace_entry(qemu_coroutine_self(),
                                         c == s->l2_table_cache, i);
     if (i < 0) {
-- 
1.8.3.1

[Qemu-devel] [PULL 09/22] qcow2: use a hash to look for entries in the L2 cache

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

The current cache algorithm traverses the array starting always from
the beginning, so the average number of comparisons needed to perform
a lookup is proportional to the size of the array.

By using a hash of the offset as the starting point, lookups are
faster and independent from the array size.

The hash is computed using the cluster number of the table, multiplied
by 4 to make it perform better when there are collisions.

In my tests, using a cache with 2048 entries, this reduces the average
number of comparisons per lookup from 430 to 2.5.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 2035cd8..121e6e9 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -248,6 +248,7 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
     BDRVQcowState *s = bs->opaque;
     int i;
     int ret;
+    int lookup_index;
     uint64_t min_lru_counter = UINT64_MAX;
     int min_lru_index = -1;
 
@@ -255,7 +256,8 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
                           offset, read_from_disk);
 
     /* Check if the table is already cached */
-    for (i = 0; i < c->size; i++) {
+    i = lookup_index = (offset / s->cluster_size * 4) % c->size;
+    do {
         const Qcow2CachedTable *t = &c->entries[i];
         if (t->offset == offset) {
             goto found;
@@ -264,7 +266,10 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
             min_lru_counter = t->lru_counter;
             min_lru_index = i;
         }
-    }
+        if (++i == c->size) {
+            i = 0;
+        }
+    } while (i != lookup_index);
 
     if (min_lru_index == -1) {
         /* This can't happen in current synchronous code, but leave the check
-- 
1.8.3.1

[Qemu-devel] [PULL 10/22] qcow2: make qcow2_cache_put() a void function

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

This function never receives an invalid table pointer, so we can make
it void and remove all the error checking code.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c    |  7 +------
 block/qcow2-cluster.c  | 50 ++++++++++----------------------------------------
 block/qcow2-refcount.c | 29 +++++------------------------
 block/qcow2.h          |  2 +-
 4 files changed, 17 insertions(+), 71 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 121e6e9..bde3c4f 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -330,14 +330,10 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
     return qcow2_cache_do_get(bs, c, offset, table, false);
 }
 
-int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
+void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
 {
     int i = qcow2_cache_get_table_idx(bs, c, *table);
 
-    if (c->entries[i].offset == 0) {
-        return -ENOENT;
-    }
-
     c->entries[i].ref--;
     *table = NULL;
 
@@ -346,7 +342,6 @@ int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
     }
 
     assert(c->entries[i].ref >= 0);
-    return 0;
 }
 
 void qcow2_cache_entry_mark_dirty(BlockDriverState *bs, Qcow2Cache *c,
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 5cd418a..d74426c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -253,10 +253,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table)
 
         memcpy(l2_table, old_table, s->cluster_size);
 
-        ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &old_table);
-        if (ret < 0) {
-            goto fail;
-        }
+        qcow2_cache_put(bs, s->l2_table_cache, (void **) &old_table);
     }
 
     /* write the l2 table to the file */
@@ -694,10 +691,7 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
     BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE_COMPRESSED);
     qcow2_cache_entry_mark_dirty(bs, s->l2_table_cache, l2_table);
     l2_table[l2_index] = cpu_to_be64(cluster_offset);
-    ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
-    if (ret < 0) {
-        return 0;
-    }
+    qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
 
     return cluster_offset;
 }
@@ -789,10 +783,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
      }
 
 
-    ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
-    if (ret < 0) {
-        goto err;
-    }
+    qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
 
     /*
      * If this was a COW, we need to decrease the refcount of the old cluster.
@@ -944,7 +935,7 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
     uint64_t *l2_table;
     unsigned int nb_clusters;
     unsigned int keep_clusters;
-    int ret, pret;
+    int ret;
 
     trace_qcow2_handle_copied(qemu_coroutine_self(), guest_offset, *host_offset,
                               *bytes);
@@ -1011,10 +1002,7 @@ static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
 
     /* Cleanup */
 out:
-    pret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
-    if (pret < 0) {
-        return pret;
-    }
+    qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
 
     /* Only return a host offset if we actually made progress. Otherwise we
      * would make requirements for handle_alloc() that it can't fulfill */
@@ -1139,10 +1127,7 @@ static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset,
      * wrong with our code. */
     assert(nb_clusters > 0);
 
-    ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
-    if (ret < 0) {
-        return ret;
-    }
+    qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
 
     /* Allocate, if necessary at a given offset in the image file */
     alloc_cluster_offset = start_of_cluster(s, *host_offset);
@@ -1481,10 +1466,7 @@ static int discard_single_l2(BlockDriverState *bs, uint64_t offset,
         qcow2_free_any_clusters(bs, old_l2_entry, 1, type);
     }
 
-    ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
-    if (ret < 0) {
-        return ret;
-    }
+    qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
 
     return nb_clusters;
 }
@@ -1567,10 +1549,7 @@ static int zero_single_l2(BlockDriverState *bs, uint64_t offset,
         }
     }
 
-    ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
-    if (ret < 0) {
-        return ret;
-    }
+    qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
 
     return nb_clusters;
 }
@@ -1763,11 +1742,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                 qcow2_cache_entry_mark_dirty(bs, 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;
-            }
+            qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
         } else {
             if (l2_dirty) {
                 ret = qcow2_pre_write_overlap_check(bs,
@@ -1798,12 +1773,7 @@ fail:
         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);
-            }
+            qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
         }
     }
     return ret;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 0707f94..0632fc3 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -265,10 +265,7 @@ int qcow2_get_refcount(BlockDriverState *bs, int64_t cluster_index,
     block_index = cluster_index & (s->refcount_block_size - 1);
     *refcount = s->get_refcount(refcount_block, block_index);
 
-    ret = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
-    if (ret < 0) {
-        return ret;
-    }
+    qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
 
     return 0;
 }
@@ -448,10 +445,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
         return -EAGAIN;
     }
 
-    ret = qcow2_cache_put(bs, s->refcount_block_cache, refcount_block);
-    if (ret < 0) {
-        goto fail_block;
-    }
+    qcow2_cache_put(bs, s->refcount_block_cache, refcount_block);
 
     /*
      * If we come here, we need to grow the refcount table. Again, a new
@@ -723,13 +717,8 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
         /* Load the refcount block and allocate it if needed */
         if (table_index != old_table_index) {
             if (refcount_block) {
-                ret = qcow2_cache_put(bs, s->refcount_block_cache,
-                                      &refcount_block);
-                if (ret < 0) {
-                    goto fail;
-                }
+                qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
             }
-
             ret = alloc_refcount_block(bs, cluster_index, &refcount_block);
             if (ret < 0) {
                 goto fail;
@@ -774,11 +763,7 @@ fail:
 
     /* Write last changed block to disk */
     if (refcount_block) {
-        int wret;
-        wret = qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
-        if (wret < 0) {
-            return ret < 0 ? ret : wret;
-        }
+        qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
     }
 
     /*
@@ -1188,11 +1173,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
                 }
             }
 
-            ret = qcow2_cache_put(bs, s->l2_table_cache, (void**) &l2_table);
-            if (ret < 0) {
-                goto fail;
-            }
-
+            qcow2_cache_put(bs, s->l2_table_cache, (void **) &l2_table);
 
             if (addend != 0) {
                 ret = qcow2_update_cluster_refcount(bs, l2_offset >>
diff --git a/block/qcow2.h b/block/qcow2.h
index 5d0995f..0076512 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -587,6 +587,6 @@ 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,
     void **table);
-int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
+void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PULL 11/22] qcow2: style fixes in qcow2-cache.c

[Kevin Wolf]

From: Alberto Garcia <berto@igalia.com>

Fix pointer declaration to make it consistent with the rest of the
code.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index bde3c4f..ed92a09 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -35,8 +35,8 @@ typedef struct Qcow2CachedTable {
 } Qcow2CachedTable;
 
 struct Qcow2Cache {
-    Qcow2CachedTable*       entries;
-    struct Qcow2Cache*      depends;
+    Qcow2CachedTable       *entries;
+    struct Qcow2Cache      *depends;
     int                     size;
     bool                    depends_on_flush;
     void                   *table_array;
@@ -81,7 +81,7 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
     return c;
 }
 
-int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c)
+int qcow2_cache_destroy(BlockDriverState *bs, Qcow2Cache *c)
 {
     int i;
 
-- 
1.8.3.1

[Qemu-devel] [PULL 12/22] qemu-io: Use getopt() correctly

[Kevin Wolf]

From: Eric Blake <eblake@redhat.com>

POSIX says getopt() returns -1 on completion.  While Linux happens
to define EOF as -1, this definition is not required by POSIX, and
there may be platforms where checking for EOF instead of -1 would
lead to an infinite loop.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-io-cmds.c | 16 ++++++++--------
 qemu-io.c      |  2 +-
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 1afcfc0..52dc611 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -646,7 +646,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
     int total = 0;
     int pattern = 0, pattern_offset = 0, pattern_count = 0;
 
-    while ((c = getopt(argc, argv, "bCl:pP:qs:v")) != EOF) {
+    while ((c = getopt(argc, argv, "bCl:pP:qs:v")) != -1) {
         switch (c) {
         case 'b':
             bflag = 1;
@@ -830,7 +830,7 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
     int pattern = 0;
     int Pflag = 0;
 
-    while ((c = getopt(argc, argv, "CP:qv")) != EOF) {
+    while ((c = getopt(argc, argv, "CP:qv")) != -1) {
         switch (c) {
         case 'C':
             Cflag = 1;
@@ -961,7 +961,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
     int total = 0;
     int pattern = 0xcd;
 
-    while ((c = getopt(argc, argv, "bcCpP:qz")) != EOF) {
+    while ((c = getopt(argc, argv, "bcCpP:qz")) != -1) {
         switch (c) {
         case 'b':
             bflag = 1;
@@ -1116,7 +1116,7 @@ static int writev_f(BlockBackend *blk, int argc, char **argv)
     int pattern = 0xcd;
     QEMUIOVector qiov;
 
-    while ((c = getopt(argc, argv, "CqP:")) != EOF) {
+    while ((c = getopt(argc, argv, "CqP:")) != -1) {
         switch (c) {
         case 'C':
             Cflag = 1;
@@ -1228,7 +1228,7 @@ static int multiwrite_f(BlockBackend *blk, int argc, char **argv)
     int i;
     BlockRequest *reqs;
 
-    while ((c = getopt(argc, argv, "CqP:")) != EOF) {
+    while ((c = getopt(argc, argv, "CqP:")) != -1) {
         switch (c) {
         case 'C':
             Cflag = 1;
@@ -1463,7 +1463,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
     struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
 
     ctx->blk = blk;
-    while ((c = getopt(argc, argv, "CP:qv")) != EOF) {
+    while ((c = getopt(argc, argv, "CP:qv")) != -1) {
         switch (c) {
         case 'C':
             ctx->Cflag = 1;
@@ -1562,7 +1562,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
     struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
 
     ctx->blk = blk;
-    while ((c = getopt(argc, argv, "CqP:")) != EOF) {
+    while ((c = getopt(argc, argv, "CqP:")) != -1) {
         switch (c) {
         case 'C':
             ctx->Cflag = 1;
@@ -1779,7 +1779,7 @@ static int discard_f(BlockBackend *blk, int argc, char **argv)
     int64_t offset;
     int count;
 
-    while ((c = getopt(argc, argv, "Cq")) != EOF) {
+    while ((c = getopt(argc, argv, "Cq")) != -1) {
         switch (c) {
         case 'C':
             Cflag = 1;
diff --git a/qemu-io.c b/qemu-io.c
index 8e41080..ae5e274 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -120,7 +120,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
     QemuOpts *qopts;
     QDict *opts;
 
-    while ((c = getopt(argc, argv, "snrgo:")) != EOF) {
+    while ((c = getopt(argc, argv, "snrgo:")) != -1) {
         switch (c) {
         case 's':
             flags |= BDRV_O_SNAPSHOT;
-- 
1.8.3.1

[Qemu-devel] [PULL 13/22] block: Detect multiplication overflow in bdrv_getlength

[Kevin Wolf]

From: Fam Zheng <famz@redhat.com>

Bogus image may have a large total_sectors that will overflow the
multiplication. For cleanness, fix the return code so the error message
will be meaningful.

Reported-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block.c b/block.c
index 325f727..f42d70e 100644
--- a/block.c
+++ b/block.c
@@ -2341,6 +2341,7 @@ int64_t bdrv_getlength(BlockDriverState *bs)
 {
     int64_t ret = bdrv_nb_sectors(bs);
 
+    ret = ret > INT64_MAX / BDRV_SECTOR_SIZE ? -EFBIG : ret;
     return ret < 0 ? ret : ret * BDRV_SECTOR_SIZE;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 14/22] qemu-iotests: qemu-img info on afl VMDK image with a huge capacity

[Kevin Wolf]

From: Fam Zheng <famz@redhat.com>

The image is contributed by Richard W.M. Jones.

Cc: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/059                         |   5 +++++
 tests/qemu-iotests/059.out                     |   3 +++
 tests/qemu-iotests/sample_images/afl9.vmdk.bz2 | Bin 0 -> 178 bytes
 3 files changed, 8 insertions(+)
 create mode 100644 tests/qemu-iotests/sample_images/afl9.vmdk.bz2

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 50ca5ce..0ded0c3 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -132,6 +132,11 @@ _img_info
 $QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io
 
+echo
+echo "=== Testing afl image with a very large capacity ==="
+_use_sample_img afl9.vmdk.bz2
+_img_info
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index cbb0de4..67e3cf5 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2336,4 +2336,7 @@ e1000003e0:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
 e1000003f0:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
 read 1024/1024 bytes at offset 966367641600
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing afl image with a very large capacity ===
+qemu-img: Can't get size of device 'image': File too large
 *** done
diff --git a/tests/qemu-iotests/sample_images/afl9.vmdk.bz2 b/tests/qemu-iotests/sample_images/afl9.vmdk.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..03615d36a12425cf4240bab86f4cfe648db14572
GIT binary patch
literal 178
zcmV;j08RfwT4*^jL0KkKS>A08g#Z9x|HJ$H)ZJi0004xF0SE*D03g5s00IDLSQelF
ziVX^$pfWNUJrmRhn2k52pQ;Rs0EQC;(S%|!m`2~BZ@b++;etskRJUVl!Kt)wu7?VN
zl;%JdqX2?TgsNVJP?87M*MvL1qQnBkCES&?0@MeaN-bL4;bDzxmMm|da4fuh!=#fu
g@i9R@5z!av{9tA<GGr!3hi~HUNT&)C8_l7xpl%OKQ2+n{

literal 0
HcmV?d00001

-- 
1.8.3.1

[Qemu-devel] [PULL 15/22] qemu-iotests: Make debugging python tests easier

[Kevin Wolf]

From: Fam Zheng <famz@redhat.com>

Adding "-d" option. The output goes to "tee" so it appears in your
console. Also, raise the verbosity of unnitest runner.

When testing a topic branch, it's possible that a bug introduced by a
code change makes the python test case hang, with debug output, it is
much easier to locate the problem.

This can also be helpful if you want to watch the progress of a python
test, it offers you a way to sense the speed of each test case method
you're writing.

Note: because there is no easy way to get *both* the verbose output and
the output expected by ./check comparison, the case would always fail
with an "output mismatch". The sole purpose of using this option is
giving developers a quick way to debug when things go wrong.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/check      | 12 +++++++++---
 tests/qemu-iotests/common     |  6 ++++++
 tests/qemu-iotests/iotests.py | 14 +++++++++++---
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index baeae80..1fa6319 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -296,9 +296,15 @@ do
             run_command="./$seq"
         fi
         export OUTPUT_DIR=$PWD
-        (cd "$source_iotests";
-        MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
-                $run_command >$tmp.out 2>&1)
+        if $debug; then
+            (cd "$source_iotests";
+            MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
+                    $run_command -d 2>&1 | tee $tmp.out)
+        else
+            (cd "$source_iotests";
+            MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
+                    $run_command >$tmp.out 2>&1)
+        fi
         sts=$?
         $timestamp && _timestamp
         stop=`_wallclock`
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 1e556bb..1030aaf 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -32,6 +32,7 @@ check=${check-true}
 
 diff="diff -u"
 verbose=false
+debug=false
 group=false
 xgroup=false
 imgopts=false
@@ -132,6 +133,7 @@ s/ .*//p
 
 common options
     -v                  verbose
+    -d                  debug
 
 check options
     -raw                test raw (default)
@@ -322,6 +324,10 @@ testlist options
             verbose=true
             xpand=false
             ;;
+        -d)
+            debug=true
+            xpand=false
+            ;;
         -x)        # -x group ... exclude from group file
             xgroup=true
             xpand=false
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e93e623..04a294d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -338,6 +338,8 @@ def notrun(reason):
 def main(supported_fmts=[], supported_oses=['linux']):
     '''Run tests'''
 
+    debug = '-d' in sys.argv
+    verbosity = 1
     if supported_fmts and (imgfmt not in supported_fmts):
         notrun('not suitable for this image format: %s' % imgfmt)
 
@@ -347,14 +349,20 @@ def main(supported_fmts=[], supported_oses=['linux']):
     # We need to filter out the time taken from the output so that qemu-iotest
     # can reliably diff the results against master output.
     import StringIO
-    output = StringIO.StringIO()
+    if debug:
+        output = sys.stdout
+        verbosity = 2
+        sys.argv.remove('-d')
+    else:
+        output = StringIO.StringIO()
 
     class MyTestRunner(unittest.TextTestRunner):
-        def __init__(self, stream=output, descriptions=True, verbosity=1):
+        def __init__(self, stream=output, descriptions=True, verbosity=verbosity):
             unittest.TextTestRunner.__init__(self, stream, descriptions, verbosity)
 
     # unittest.main() will use sys.exit() so expect a SystemExit exception
     try:
         unittest.main(testRunner=MyTestRunner)
     finally:
-        sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', output.getvalue()))
+        if not debug:
+            sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', output.getvalue()))
-- 
1.8.3.1

[Qemu-devel] [PULL 16/22] qcow2/qcow: protect against uninitialized encryption key

[Kevin Wolf]

From: "Daniel P. Berrange" <berrange@redhat.com>

When a qcow[2] file is opened, if the header reports an
encryption method, this is used to set the 'crypt_method_header'
field on the BDRVQcow[2]State struct, and the 'encrypted' flag
in the BDRVState struct.

When doing I/O operations, the 'crypt_method' field on the
BDRVQcow[2]State struct is checked to determine if encryption
needs to be applied.

The crypt_method_header value is copied into crypt_method when
the bdrv_set_key() method is called.

The QEMU code which opens a block device is expected to always
do a check

   if (bdrv_is_encrypted(bs)) {
       bdrv_set_key(bs, ....key...);
   }

If code forgets to do this, then 'crypt_method' is never set
and so when I/O is performed, QEMU writes plain text data
into a sector which is expected to contain cipher text, or
when reading, will return cipher text instead of plain
text.

Change the qcow[2] code to consult bs->encrypted when deciding
whether encryption is required, and assert(s->crypt_method)
to protect against cases where the caller forgets to set the
encryption key.

Also put an assert in the set_key methods to protect against
the case where the caller sets an encryption key on a block
device that does not have encryption

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow.c          | 10 +++++++---
 block/qcow2-cluster.c |  3 ++-
 block/qcow2.c         | 18 ++++++++++++------
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index ab89328..911e59f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -269,6 +269,7 @@ static int qcow_set_key(BlockDriverState *bs, const char *key)
     for(i = 0;i < len;i++) {
         keybuf[i] = key[i];
     }
+    assert(bs->encrypted);
     s->crypt_method = s->crypt_method_header;
 
     if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0)
@@ -411,9 +412,10 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
                 bdrv_truncate(bs->file, cluster_offset + s->cluster_size);
                 /* if encrypted, we must initialize the cluster
                    content which won't be written */
-                if (s->crypt_method &&
+                if (bs->encrypted &&
                     (n_end - n_start) < s->cluster_sectors) {
                     uint64_t start_sect;
+                    assert(s->crypt_method);
                     start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
                     memset(s->cluster_data + 512, 0x00, 512);
                     for(i = 0; i < s->cluster_sectors; i++) {
@@ -590,7 +592,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
             if (ret < 0) {
                 break;
             }
-            if (s->crypt_method) {
+            if (bs->encrypted) {
+                assert(s->crypt_method);
                 encrypt_sectors(s, sector_num, buf, buf,
                                 n, 0,
                                 &s->aes_decrypt_key);
@@ -661,7 +664,8 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
             ret = -EIO;
             break;
         }
-        if (s->crypt_method) {
+        if (bs->encrypted) {
+            assert(s->crypt_method);
             if (!cluster_data) {
                 cluster_data = g_malloc0(s->cluster_size);
             }
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index d74426c..1a5c97a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -400,7 +400,8 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
         goto out;
     }
 
-    if (s->crypt_method) {
+    if (bs->encrypted) {
+        assert(s->crypt_method);
         qcow2_encrypt_sectors(s, start_sect + n_start,
                         iov.iov_base, iov.iov_base, n, 1,
                         &s->aes_encrypt_key);
diff --git a/block/qcow2.c b/block/qcow2.c
index b9a72e3..f7b4cc6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1037,6 +1037,7 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key)
     for(i = 0;i < len;i++) {
         keybuf[i] = key[i];
     }
+    assert(bs->encrypted);
     s->crypt_method = s->crypt_method_header;
 
     if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0)
@@ -1224,7 +1225,9 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
                 goto fail;
             }
 
-            if (s->crypt_method) {
+            if (bs->encrypted) {
+                assert(s->crypt_method);
+
                 /*
                  * For encrypted images, read everything into a temporary
                  * contiguous buffer on which the AES functions can work.
@@ -1255,7 +1258,8 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
             if (ret < 0) {
                 goto fail;
             }
-            if (s->crypt_method) {
+            if (bs->encrypted) {
+                assert(s->crypt_method);
                 qcow2_encrypt_sectors(s, sector_num,  cluster_data,
                     cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
                 qemu_iovec_from_buf(qiov, bytes_done,
@@ -1315,7 +1319,7 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
         trace_qcow2_writev_start_part(qemu_coroutine_self());
         index_in_cluster = sector_num & (s->cluster_sectors - 1);
         cur_nr_sectors = remaining_sectors;
-        if (s->crypt_method &&
+        if (bs->encrypted &&
             cur_nr_sectors >
             QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors - index_in_cluster) {
             cur_nr_sectors =
@@ -1334,7 +1338,8 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
         qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
             cur_nr_sectors * 512);
 
-        if (s->crypt_method) {
+        if (bs->encrypted) {
+            assert(s->crypt_method);
             if (!cluster_data) {
                 cluster_data = qemu_try_blockalign(bs->file,
                                                    QCOW_MAX_CRYPT_CLUSTERS
@@ -1484,7 +1489,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
      * that means we don't have to worry about reopening them here.
      */
 
-    if (s->crypt_method) {
+    if (bs->encrypted) {
+        assert(s->crypt_method);
         crypt_method = s->crypt_method;
         memcpy(&aes_encrypt_key, &s->aes_encrypt_key, sizeof(aes_encrypt_key));
         memcpy(&aes_decrypt_key, &s->aes_decrypt_key, sizeof(aes_decrypt_key));
@@ -1513,7 +1519,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
         return;
     }
 
-    if (crypt_method) {
+    if (bs->encrypted) {
         s->crypt_method = crypt_method;
         memcpy(&s->aes_encrypt_key, &aes_encrypt_key, sizeof(aes_encrypt_key));
         memcpy(&s->aes_decrypt_key, &aes_decrypt_key, sizeof(aes_decrypt_key));
-- 
1.8.3.1

[Qemu-devel] [PULL 17/22] util: move read_password method out of qemu-img into osdep/oslib

[Kevin Wolf]

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu-img.c file has a read_password() method impl that is
used to prompt for passwords on the console, with impls for
POSIX and Windows. This will be needed by qemu-io.c too, so
move it into the QEMU osdep/oslib files where it can be shared
without code duplication

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/qemu/osdep.h |  2 ++
 qemu-img.c           | 93 +---------------------------------------------------
 util/oslib-posix.c   | 66 +++++++++++++++++++++++++++++++++++++
 util/oslib-win32.c   | 24 ++++++++++++++
 4 files changed, 93 insertions(+), 92 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b3300cc..3247364 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -259,4 +259,6 @@ void qemu_set_tty_echo(int fd, bool echo);
 
 void os_mem_prealloc(int fd, char *area, size_t sz);
 
+int qemu_read_password(char *buf, int buf_size);
+
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index 8d30e43..60c820d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -165,97 +165,6 @@ static int GCC_FMT_ATTR(2, 3) qprintf(bool quiet, const char *fmt, ...)
     return ret;
 }
 
-#if defined(WIN32)
-/* XXX: put correct support for win32 */
-static int read_password(char *buf, int buf_size)
-{
-    int c, i;
-
-    printf("Password: ");
-    fflush(stdout);
-    i = 0;
-    for(;;) {
-        c = getchar();
-        if (c < 0) {
-            buf[i] = '\0';
-            return -1;
-        } else if (c == '\n') {
-            break;
-        } else if (i < (buf_size - 1)) {
-            buf[i++] = c;
-        }
-    }
-    buf[i] = '\0';
-    return 0;
-}
-
-#else
-
-#include <termios.h>
-
-static struct termios oldtty;
-
-static void term_exit(void)
-{
-    tcsetattr (0, TCSANOW, &oldtty);
-}
-
-static void term_init(void)
-{
-    struct termios tty;
-
-    tcgetattr (0, &tty);
-    oldtty = tty;
-
-    tty.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP
-                          |INLCR|IGNCR|ICRNL|IXON);
-    tty.c_oflag |= OPOST;
-    tty.c_lflag &= ~(ECHO|ECHONL|ICANON|IEXTEN);
-    tty.c_cflag &= ~(CSIZE|PARENB);
-    tty.c_cflag |= CS8;
-    tty.c_cc[VMIN] = 1;
-    tty.c_cc[VTIME] = 0;
-
-    tcsetattr (0, TCSANOW, &tty);
-
-    atexit(term_exit);
-}
-
-static int read_password(char *buf, int buf_size)
-{
-    uint8_t ch;
-    int i, ret;
-
-    printf("password: ");
-    fflush(stdout);
-    term_init();
-    i = 0;
-    for(;;) {
-        ret = read(0, &ch, 1);
-        if (ret == -1) {
-            if (errno == EAGAIN || errno == EINTR) {
-                continue;
-            } else {
-                break;
-            }
-        } else if (ret == 0) {
-            ret = -1;
-            break;
-        } else {
-            if (ch == '\r') {
-                ret = 0;
-                break;
-            }
-            if (i < (buf_size - 1))
-                buf[i++] = ch;
-        }
-    }
-    term_exit();
-    buf[i] = '\0';
-    printf("\n");
-    return ret;
-}
-#endif
 
 static int print_block_option_help(const char *filename, const char *fmt)
 {
@@ -312,7 +221,7 @@ static BlockBackend *img_open(const char *id, const char *filename,
     bs = blk_bs(blk);
     if (bdrv_is_encrypted(bs) && require_io) {
         qprintf(quiet, "Disk image '%s' is encrypted.\n", filename);
-        if (read_password(password, sizeof(password)) < 0) {
+        if (qemu_read_password(password, sizeof(password)) < 0) {
             error_report("No password given");
             goto fail;
         }
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 37ffd96..1c23fd2 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -50,6 +50,7 @@ extern int daemon(int, int);
 
 #include <termios.h>
 #include <unistd.h>
+#include <termios.h>
 
 #include <glib/gprintf.h>
 
@@ -415,3 +416,68 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
         pthread_sigmask(SIG_SETMASK, &oldset, NULL);
     }
 }
+
+
+static struct termios oldtty;
+
+static void term_exit(void)
+{
+    tcsetattr(0, TCSANOW, &oldtty);
+}
+
+static void term_init(void)
+{
+    struct termios tty;
+
+    tcgetattr(0, &tty);
+    oldtty = tty;
+
+    tty.c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP
+                          |INLCR|IGNCR|ICRNL|IXON);
+    tty.c_oflag |= OPOST;
+    tty.c_lflag &= ~(ECHO|ECHONL|ICANON|IEXTEN);
+    tty.c_cflag &= ~(CSIZE|PARENB);
+    tty.c_cflag |= CS8;
+    tty.c_cc[VMIN] = 1;
+    tty.c_cc[VTIME] = 0;
+
+    tcsetattr(0, TCSANOW, &tty);
+
+    atexit(term_exit);
+}
+
+int qemu_read_password(char *buf, int buf_size)
+{
+    uint8_t ch;
+    int i, ret;
+
+    printf("password: ");
+    fflush(stdout);
+    term_init();
+    i = 0;
+    for (;;) {
+        ret = read(0, &ch, 1);
+        if (ret == -1) {
+            if (errno == EAGAIN || errno == EINTR) {
+                continue;
+            } else {
+                break;
+            }
+        } else if (ret == 0) {
+            ret = -1;
+            break;
+        } else {
+            if (ch == '\r') {
+                ret = 0;
+                break;
+            }
+            if (i < (buf_size - 1)) {
+                buf[i++] = ch;
+            }
+        }
+    }
+    term_exit();
+    buf[i] = '\0';
+    printf("\n");
+    return ret;
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 87cfbe0..730a670 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -470,3 +470,27 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
         memset(area + pagesize * i, 0, 1);
     }
 }
+
+
+/* XXX: put correct support for win32 */
+int qemu_read_password(char *buf, int buf_size)
+{
+    int c, i;
+
+    printf("Password: ");
+    fflush(stdout);
+    i = 0;
+    for (;;) {
+        c = getchar();
+        if (c < 0) {
+            buf[i] = '\0';
+            return -1;
+        } else if (c == '\n') {
+            break;
+        } else if (i < (buf_size - 1)) {
+            buf[i++] = c;
+        }
+    }
+    buf[i] = '\0';
+    return 0;
+}
-- 
1.8.3.1

[Qemu-devel] [PULL 18/22] util: allow \n to terminate password input

[Kevin Wolf]

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu_read_password() method looks for \r to terminate the
reading of the a password. This is what will be seen when
reading the password from a TTY. When scripting though, it is
useful to be able to send the password via a pipe, in which
case we must look for \n to terminate password input.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 util/oslib-posix.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 1c23fd2..3ae4987 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -467,7 +467,8 @@ int qemu_read_password(char *buf, int buf_size)
             ret = -1;
             break;
         } else {
-            if (ch == '\r') {
+            if (ch == '\r' ||
+                ch == '\n') {
                 ret = 0;
                 break;
             }
-- 
1.8.3.1

[Qemu-devel] [PULL 19/22] qemu-io: prompt for encryption keys when required

[Kevin Wolf]

From: "Daniel P. Berrange" <berrange@redhat.com>

The qemu-io tool does not check if the image is encrypted so
historically would silently corrupt the sectors by writing
plain text data into them instead of cipher text. The earlier
commit turns this mistake into a fatal abort, so check for
encryption and prompt for key when required.

This enables us to add unit tests to ensure we don't break
the ability of qemu-img to convert existing encrypted qcow2
files into a non-encrypted format.

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

diff --git a/qemu-io.c b/qemu-io.c
index ae5e274..9bc83c6 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -52,6 +52,7 @@ static const cmdinfo_t close_cmd = {
 static int openfile(char *name, int flags, QDict *opts)
 {
     Error *local_err = NULL;
+    BlockDriverState *bs;
 
     if (qemuio_blk) {
         fprintf(stderr, "file open already, try 'help close'\n");
@@ -68,7 +69,27 @@ static int openfile(char *name, int flags, QDict *opts)
         return 1;
     }
 
+    bs = blk_bs(qemuio_blk);
+    if (bdrv_is_encrypted(bs)) {
+        char password[256];
+        printf("Disk image '%s' is encrypted.\n", name);
+        if (qemu_read_password(password, sizeof(password)) < 0) {
+            error_report("No password given");
+            goto error;
+        }
+        if (bdrv_set_key(bs, password) < 0) {
+            error_report("invalid password");
+            goto error;
+        }
+    }
+
+
     return 0;
+
+ error:
+    blk_unref(qemuio_blk);
+    qemuio_blk = NULL;
+    return 1;
 }
 
 static void open_help(void)
-- 
1.8.3.1

[Qemu-devel] [PULL 20/22] tests: add test case for encrypted qcow2 read/write

[Kevin Wolf]

From: "Daniel P. Berrange" <berrange@redhat.com>

Add a simple test case for qemu-iotests that covers read/write
with encrypted qcow2 files.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/134     | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/134.out | 46 +++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 116 insertions(+)
 create mode 100755 tests/qemu-iotests/134
 create mode 100644 tests/qemu-iotests/134.out

diff --git a/tests/qemu-iotests/134 b/tests/qemu-iotests/134
new file mode 100755
index 0000000..1c3820b
--- /dev/null
+++ b/tests/qemu-iotests/134
@@ -0,0 +1,69 @@
+#!/bin/bash
+#
+# Test encrypted read/write using plain bdrv_read/bdrv_write
+#
+# Copyright (C) 2015 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=berrange@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
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+
+size=128M
+IMGOPTS="encryption=on" _make_test_img $size
+
+echo
+echo "== reading whole image =="
+echo "astrochicken" | $QEMU_IO -c "read 0 $size" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== rewriting whole image =="
+echo "astrochicken" | $QEMU_IO -c "write -P 0xa 0 $size" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== verify pattern =="
+echo "astrochicken" | $QEMU_IO -c "read -P 0xa 0 $size" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== verify pattern failure with wrong password =="
+echo "platypus" | $QEMU_IO -c "read -P 0xa 0 $size" "$TEST_IMG" | _filter_qemu_io | _filter_testdir
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/134.out b/tests/qemu-iotests/134.out
new file mode 100644
index 0000000..a16acb8
--- /dev/null
+++ b/tests/qemu-iotests/134.out
@@ -0,0 +1,46 @@
+QA output created by 134
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+qemu-img: Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on
+
+== reading whole image ==
+Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+read 134217728/134217728 bytes at offset 0
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== rewriting whole image ==
+Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+wrote 134217728/134217728 bytes at offset 0
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern ==
+Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+read 134217728/134217728 bytes at offset 0
+128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verify pattern failure with wrong password ==
+Encrypted images are deprecated
+Support for them will be removed in a future release.
+You can use 'qemu-img convert' to convert your image to an unencrypted one.
+Disk image 'TEST_DIR/t.qcow2' is encrypted.
+password:
+Pattern verification failed at offset 0, 134217728 bytes
+read 134217728/134217728 bytes at offset 0
+128 MiB, 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 34b16cb..0b817ca 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -129,3 +129,4 @@
 129 rw auto quick
 130 rw auto quick
 131 rw auto quick
+134 rw auto quick
-- 
1.8.3.1

[Qemu-devel] [PULL 21/22] MAINTAINERS: Add header files to Block Layer Core section

[Kevin Wolf]

Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b3552b2..9ff7c36 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -785,6 +785,7 @@ S: Supported
 F: block*
 F: block/
 F: hw/block/
+F: include/block/
 F: qemu-img*
 F: qemu-io*
 F: tests/qemu-iotests/
-- 
1.8.3.1

[Qemu-devel] [PULL 22/22] MAINTAINERS: Split "Block QAPI, monitor, command line" off core

[Kevin Wolf]

From: Markus Armbruster <armbru@redhat.com>

Kevin and Stefan asked me to take care of this part.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9ff7c36..0463696 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -813,6 +813,14 @@ F: block/stream.h
 F: block/mirror.c
 T: git git://github.com/codyprime/qemu-kvm-jtc.git block
 
+Block QAPI, monitor, command line
+M: Markus Armbruster <armbru@redhat.com>
+S: Supported
+F: blockdev.c
+F: block/qapi.c
+F: qapi/block*.json
+T: git git://repo.or.cz/qemu/armbru.git block-next
+
 Character Devices
 M: Paolo Bonzini <pbonzini@redhat.com>
 S: Maintained
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 00/22] Block layer core and image format patches

[Peter Maydell]

On 22 May 2015 at 16:26, Kevin Wolf <kwolf@redhat.com> wrote:
> The following changes since commit 8b6db32a4ec47d1171ccfa21d557096b99f4eef0:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2015-05-22 13:25:40 +0100)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 4120201d2fcfc24404fe6eb6b761b66bc35bca16:
>
>   MAINTAINERS: Split "Block QAPI, monitor, command line" off core (2015-05-22 17:08:09 +0200)
>
> ----------------------------------------------------------------
> Block layer core and image format patches
>

Applied, thanks.

-- PMM