qcow2: Patch for shrinking qcow2 disk image
PATCH 1/1 qcow2: Patch for shrinking qcow2 disk image
This patch can make sure the data still existing after shrinking. And only discard the unused (guest) clusters. If shrinking to the size which stored data, It will return an error and will not do any change. As this patch can support shrinking, so changed the func name of qcow2_grow_l1_table to qcow2_truncate_l1_table.
--- block/qcow2-cluster.c | 20 ++++++++++++++------ block/qcow2-snapshot.c | 2 +- block/qcow2.c | 8 +------- block/qcow2.h | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-)
block/qcow2-cluster.c +14 -6
@@ -29,7 +29,7 @@#include "block/qcow2.h" #include "trace.h" -int qcow2_grow_l1_table (BlockDriverState *bs,uint64_t min_size, +int qcow2_truncate_l1_table (BlockDriverState *bs,uint64_t min_size,bool exact_size) {BDRVQcowState *s = bs->opaque;
@@ -39,9 +39,6 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,int64_t new_l1_table_offset, new_l1_size;uint8_t data[12 ]; -if (min_size <= s->l1_size) -return 0 ; -if (exact_size) { new_l1_size = min_size; }else {
@@ -66,7 +63,18 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, new_l1_size2 =sizeof (uint64_t ) * new_l1_size; new_l1_table =g_malloc0 (align_offset (new_l1_size2,512 )); -memcpy (new_l1_table, s->l1_table, s->l1_size *sizeof (uint64_t )); + +/* shrinking the image */ +if (min_size <= s->l1_size) { +if (s->l1_table[new_l1_size] !=0 ) { +error_report ("Could not shrink to this size, " +"it will destory image data" ); +return -ENOTSUP ; + } +memcpy (new_l1_table, s->l1_table, new_l1_size2); + } + +memcpy (new_l1_table, s->l1_table, s->l1_size *sizeof (uint64_t ));/* write new table (align to cluster) */ BLKDBG_EVENT (bs->file,BLKDBG_L1_GROW_ALLOC_TABLE );
@@ -559,7 +567,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset, l1_index = offset >> (s->l2_bits + s->cluster_bits);if (l1_index >= s->l1_size) { - ret =qcow2_grow_l1_table (bs, l1_index +1 , false); + ret =qcow2_truncate_l1_table (bs, l1_index +1 , false);if (ret <0 ) {return ret; }
block/qcow2-snapshot.c +1 -1
@@ -491,7 +491,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)* L1 table of the snapshot. If the snapshot L1 table is smaller, the * current one must be padded with zeros. */ - ret =qcow2_grow_l1_table (bs, sn->l1_size, true); + ret =qcow2_truncate_l1_table (bs, sn->l1_size, true);if (ret <0 ) { goto fail; }
block/qcow2.c +1 -7
@@ -1764,14 +1764,8 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)return -ENOTSUP ; } -/* shrinking is currently not supported */ -if (offset < bs->total_sectors *512 ) { -error_report ("qcow2 doesn't support shrinking images yet" ); -return -ENOTSUP ; - } - new_l1_size =size_to_l1 (s, offset); - ret =qcow2_grow_l1_table (bs, new_l1_size, true); + ret =qcow2_truncate_l1_table (bs, new_l1_size, true);if (ret <0 ) {return ret; }
block/qcow2.h +1 -1
@@ -455,7 +455,7 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size);/* qcow2-cluster.c functions */ -int qcow2_grow_l1_table (BlockDriverState *bs,uint64_t min_size, +int qcow2_truncate_l1_table (BlockDriverState *bs,uint64_t min_size,bool exact_size);int qcow2_write_l1_entry (BlockDriverState *bs,int l1_index);void qcow2_l2_cache_reset (BlockDriverState *bs);
Signature · 1 line
1.8.3.1
Quoted 4 lines · Jun Lee
Signed-off-by Jun Li <[email protected]>This patch can make sure the data still existing after shrinking. And only discard the unused (guest) clusters. If shrinking to the size which stored data, It will return an error and will not do any change. As this patch can support shrinking, so changed the func name of
qcow2_grow_l1_tabletoqcow2_truncate_l1_table.
Signed-off-by line is missing.
Quoted 12 lines
--- block/qcow2-cluster.c | 20 ++++++++++++++------ block/qcow2-snapshot.c | 2 +- block/qcow2.c | 8 +------- block/qcow2.h | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-)block/qcow2-cluster.c +0
-0
block/qcow2-cluster.c line 33
Quoted 6 lines
block/qcow2-cluster.c +1
-1@@ -29,7 +29,7 @@#include "block/qcow2.h" #include "trace.h" -int qcow2_grow_l1_table (BlockDriverState *bs,uint64_t min_size, +int qcow2_truncate_l1_table (BlockDriverState *bs,uint64_t min_size,bool exact_size)
Parameter alignment is changed.
Quoted 13 lines
block/qcow2-cluster.c +0
-3{BDRVQcowState *s = bs->opaque;@@ -39,9 +39,6 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,int64_t new_l1_table_offset, new_l1_size;uint8_t data[12 ]; -if (min_size <= s->l1_size) -return 0 ; -if (exact_size) { new_l1_size = min_size; }else {
block/qcow2-cluster.c int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, · line 69
Quoted 8 lines
block/qcow2-cluster.c +4
-1@@ -66,7 +63,18 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, new_l1_size2 =sizeof (uint64_t ) * new_l1_size; new_l1_table =g_malloc0 (align_offset (new_l1_size2,512 )); -memcpy (new_l1_table, s->l1_table, s->l1_size *sizeof (uint64_t )); + +/* shrinking the image */ +if (min_size <= s->l1_size) { +if (s->l1_table[new_l1_size] !=0 ) {
Is this safe as advertised? Do we need to scan all the dropped l1 entries, instead of only the first one?
block/qcow2-cluster.c int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, · line 71
Quoted 2 lines
block/qcow2-cluster.c +2
-0+error_report ("Could not shrink to this size, " +"it will destory image data" );
Better to align by quote mark.
block/qcow2-cluster.c int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, · line 77
Quoted 6 lines
block/qcow2-cluster.c +6
-0+return -ENOTSUP ; + } +memcpy (new_l1_table, s->l1_table, new_l1_size2); + } + +memcpy (new_l1_table, s->l1_table, s->l1_size *sizeof (uint64_t ));
Redundant (and wrong size) memcpy for shrinking case?
Fam
Quoted 61 lines
block/qcow2-cluster.c +1
-1/* write new table (align to cluster) */ BLKDBG_EVENT (bs->file,BLKDBG_L1_GROW_ALLOC_TABLE );@@ -559,7 +567,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset, l1_index = offset >> (s->l2_bits + s->cluster_bits);if (l1_index >= s->l1_size) { - ret =qcow2_grow_l1_table (bs, l1_index +1 , false); + ret =qcow2_truncate_l1_table (bs, l1_index +1 , false);if (ret <0 ) {return ret; }block/qcow2-snapshot.c +1
-1@@ -491,7 +491,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)* L1 table of the snapshot. If the snapshot L1 table is smaller, the * current one must be padded with zeros. */ - ret =qcow2_grow_l1_table (bs, sn->l1_size, true); + ret =qcow2_truncate_l1_table (bs, sn->l1_size, true);if (ret <0 ) { goto fail; }block/qcow2.c +1
-7@@ -1764,14 +1764,8 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)return -ENOTSUP ; } -/* shrinking is currently not supported */ -if (offset < bs->total_sectors *512 ) { -error_report ("qcow2 doesn't support shrinking images yet" ); -return -ENOTSUP ; - } - new_l1_size =size_to_l1 (s, offset); - ret =qcow2_grow_l1_table (bs, new_l1_size, true); + ret =qcow2_truncate_l1_table (bs, new_l1_size, true);if (ret <0 ) {return ret; }block/qcow2.h +1
-1@@ -455,7 +455,7 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size);/* qcow2-cluster.c functions */ -int qcow2_grow_l1_table (BlockDriverState *bs,uint64_t min_size, +int qcow2_truncate_l1_table (BlockDriverState *bs,uint64_t min_size,bool exact_size);int qcow2_write_l1_entry (BlockDriverState *bs,int l1_index);void qcow2_l2_cache_reset (BlockDriverState *bs);Signature · 1 line
1.8.3.1
----- Original Message -----
From: "Fam Zheng" <[email protected]>
To: "Jun Lee" <[email protected]>
Cc: [email protected], [email protected], [email protected]
Sent: Wednesday, April 2, 2014 10:24:34 AM
Subject: Re: [Qemu-devel] [PATCH] qcow2: Patch for shrinking qcow2 disk image
Quoted 4 lines · Jun Lee
Signed-off-by Jun Li <[email protected]>This patch can make sure the data still existing after shrinking. And only discard the unused (guest) clusters. If shrinking to the size which stored data, It will return an error and will not do any change. As this patch can support shrinking, so changed the func name of
qcow2_grow_l1_tabletoqcow2_truncate_l1_table.
Signed-off-by line is missing.
-sorry, agree with you.
Quoted 12 lines
--- block/qcow2-cluster.c | 20 ++++++++++++++------ block/qcow2-snapshot.c | 2 +- block/qcow2.c | 8 +------- block/qcow2.h | 2 +- 4 files changed, 17 insertions(+), 15 deletions(-)block/qcow2-cluster.c +0
-0
block/qcow2-cluster.c line 33
Quoted 6 lines
block/qcow2-cluster.c +1
-1@@ -29,7 +29,7 @@#include "block/qcow2.h" #include "trace.h" -int qcow2_grow_l1_table (BlockDriverState *bs,uint64_t min_size, +int qcow2_truncate_l1_table (BlockDriverState *bs,uint64_t min_size,bool exact_size)
Parameter alignment is changed.
-sorry, agree with you. I will change it.
Quoted 13 lines
block/qcow2-cluster.c +0
-3{BDRVQcowState *s = bs->opaque;@@ -39,9 +39,6 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,int64_t new_l1_table_offset, new_l1_size;uint8_t data[12 ]; -if (min_size <= s->l1_size) -return 0 ; -if (exact_size) { new_l1_size = min_size; }else {
block/qcow2-cluster.c int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, · line 69
Quoted 8 lines
block/qcow2-cluster.c +4
-1@@ -66,7 +63,18 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, new_l1_size2 =sizeof (uint64_t ) * new_l1_size; new_l1_table =g_malloc0 (align_offset (new_l1_size2,512 )); -memcpy (new_l1_table, s->l1_table, s->l1_size *sizeof (uint64_t )); + +/* shrinking the image */ +if (min_size <= s->l1_size) { +if (s->l1_table[new_l1_size] !=0 ) {
Is this safe as advertised? Do we need to scan all the dropped l1 entries, instead of only the first one?
-maybe this is the fastest way to drop. I think no need to scan all the dropped l1 entries. -As the dropped l1 entries will not contain any value data.
block/qcow2-cluster.c int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, · line 71
Quoted 2 lines
block/qcow2-cluster.c +2
-0+error_report ("Could not shrink to this size, " +"it will destory image data" );
Better to align by quote mark.
-Sorry, agree with you. Thank you very much.
block/qcow2-cluster.c int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, · line 77
Quoted 6 lines
block/qcow2-cluster.c +6
-0+return -ENOTSUP ; + } +memcpy (new_l1_table, s->l1_table, new_l1_size2); + } + +memcpy (new_l1_table, s->l1_table, s->l1_size *sizeof (uint64_t ));
Redundant (and wrong size) memcpy for shrinking case?
-Sorry, this is my carelessness. I will submit the update version. Thanks again.
Quoted 50 lines
block/qcow2-cluster.c +1
-1/* write new table (align to cluster) */ BLKDBG_EVENT (bs->file,BLKDBG_L1_GROW_ALLOC_TABLE );@@ -559,7 +567,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset, l1_index = offset >> (s->l2_bits + s->cluster_bits);if (l1_index >= s->l1_size) { - ret =qcow2_grow_l1_table (bs, l1_index +1 , false); + ret =qcow2_truncate_l1_table (bs, l1_index +1 , false);if (ret <0 ) {return ret; }block/qcow2-snapshot.c +1
-1@@ -491,7 +491,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)* L1 table of the snapshot. If the snapshot L1 table is smaller, the * current one must be padded with zeros. */ - ret =qcow2_grow_l1_table (bs, sn->l1_size, true); + ret =qcow2_truncate_l1_table (bs, sn->l1_size, true);if (ret <0 ) { goto fail; }block/qcow2.c +1
-7@@ -1764,14 +1764,8 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)return -ENOTSUP ; } -/* shrinking is currently not supported */ -if (offset < bs->total_sectors *512 ) { -error_report ("qcow2 doesn't support shrinking images yet" ); -return -ENOTSUP ; - } - new_l1_size =size_to_l1 (s, offset); - ret =qcow2_grow_l1_table (bs, new_l1_size, true); + ret =qcow2_truncate_l1_table (bs, new_l1_size, true);if (ret <0 ) {return ret; }block/qcow2.h +0
-0
block/qcow2.h int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, · line 455
Quoted 1 line
block/qcow2.h +0
-0@@ -455,7 +455,7 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size);
-This parameter should to align.
block/qcow2.h int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, · line 459
Quoted 5 lines
block/qcow2.h +1
-1/* qcow2-cluster.c functions */ -int qcow2_grow_l1_table (BlockDriverState *bs,uint64_t min_size, +int qcow2_truncate_l1_table (BlockDriverState *bs,uint64_t min_size,bool exact_size);
-This parameter should to align, too. Thanks Fam.
Jun Li
Quoted 5 lines
block/qcow2.h +0
-0int qcow2_write_l1_entry (BlockDriverState *bs,int l1_index);void qcow2_l2_cache_reset (BlockDriverState *bs);Signature · 1 line
1.8.3.1
Quoted 13 lines · Jun Lee
block/qcow2-cluster.c int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, · line 69
Quoted 9 lines
block/qcow2-cluster.c +4
-1@@ -66,7 +63,18 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, new_l1_size2 =sizeof (uint64_t ) * new_l1_size; new_l1_table =g_malloc0 (align_offset (new_l1_size2,512 )); -memcpy (new_l1_table, s->l1_table, s->l1_size *sizeof (uint64_t )); + +/* shrinking the image */ +if (min_size <= s->l1_size) { +if (s->l1_table[new_l1_size] !=0 ) {Is this safe as advertised? Do we need to scan all the dropped l1 entries, instead of only the first one?
-As the dropped l1 entries will not contain any value data.
I don't understand, how do you make sure of that without checking it?
Fam