[Qemu-devel] [PATCH 0/2] qcow2: Discard VM state in active L1 after creating snapshot

[Qemu-devel] [PATCH 0/2] qcow2: Discard VM state in active L1 after creating snapshot

[Kevin Wolf]

This series fixes that 'savevm' becomes much slower after the first snapshot.
The reason is unnecessary COW for the VM state, which is now avoided.

Kevin Wolf (2):
  qcow2: Pass discard type to qcow2_discard_clusters()
  qcow2: Discard VM state in active L1 after creating snapshot

 block/qcow2-cluster.c  | 8 ++++----
 block/qcow2-snapshot.c | 7 +++++++
 block/qcow2.c          | 7 +------
 block/qcow2.h          | 7 ++++++-
 4 files changed, 18 insertions(+), 11 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/2] qcow2: Pass discard type to qcow2_discard_clusters()

[Kevin Wolf]

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

Signed-off-by: Kevin Wolf <kwolf@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 2d5aa92..b0d688e 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 4bc679a..22569e6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1572,7 +1572,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 1000239..9c33b98 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 */
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/2] qcow2: Discard VM state in active L1 after creating snapshot

[Kevin Wolf]

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>
---
 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 e7e6013..ffead08 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 22569e6..d11ced1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1747,11 +1747,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 9c33b98..47af5c9 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) {
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 0/2] qcow2: Discard VM state in active L1 after creating snapshot

[Max Reitz]

On 2013-09-06 12:32, Kevin Wolf wrote:
> This series fixes that 'savevm' becomes much slower after the first snapshot.
> The reason is unnecessary COW for the VM state, which is now avoided.
>
> Kevin Wolf (2):
>    qcow2: Pass discard type to qcow2_discard_clusters()
>    qcow2: Discard VM state in active L1 after creating snapshot
>
>   block/qcow2-cluster.c  | 8 ++++----
>   block/qcow2-snapshot.c | 7 +++++++
>   block/qcow2.c          | 7 +------
>   block/qcow2.h          | 7 ++++++-
>   4 files changed, 18 insertions(+), 11 deletions(-)
>

Reviewed-by: Max Reitz <mreitz@redhat.com>