Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
Am 13.12.2011 04:41, schrieb lan,Tianyu: On 一, 2011-12-12 at 19:15 +0800, Kevin Wolf wrote: Am 12.12.2011 11:58, schrieb Pekka Enberg: On Mon, 12 Dec 2011, Kevin Wolf wrote: @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 rft_idx = clust_idx (header-cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT); if (rft_idx = rft-rf_size) - return NULL; + return (void *)-ENOSPC; Is this allowed style in kvm-tool? :-/ It needs to use ERR_PTR() and related macros but otherwise I don't see a big problem with it. Can you be sure that it never clashes with a valid allocation when you use this in userspace? But yes, at least using appropriate functions should be required. And this means that you can't only check for -ENOSPC, but you need to check for all possible error codes (IS_ERR_VALUE() I guess). The qcow_read_refcount_block() is invoiked in the two places qcow_get_refcount() and update_cluster_refcount() and will only return NULL or -ENOSPC. In the qcow_get_refcount(), when qcow_read_refcount_block() returned -ENOSPEC(no refcount block is available.), returning zero which means the cluster is not in use and the refcount block can be grew in the update_cluster_refcount(). In the update_cluster_refcount(), when qcow_read_refcount_block() returned -ENOSPEC, it is necessary to grow the refcount blocks. So the ENOSPEC should be specially dealt with. Does this make sense? :) I'm not saying that your code won't work today, just that it's brittle. If someone adds a different error return code somewhere and doesn't check if all callers properly deal with error codes, it will break. Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
Am 13.12.2011 04:05, schrieb lan,Tianyu:
Thanks for your review.
On 一, 2011-12-12 at 17:55 +0800, Kevin Wolf wrote:
Am 12.12.2011 03:03, schrieb Lan Tianyu:
This patch enables allocating new refcount blocks and so then kvm tools
could expand qcow2 image much larger.
Signed-off-by: Lan Tianyu tianyu@intel.com
---
tools/kvm/disk/qcow.c | 105
+---
1 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index e139fa5..929ba69 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -12,6 +12,7 @@
#include string.h
#include unistd.h
#include fcntl.h
+#include errno.h
#ifdef CONFIG_HAS_ZLIB
#include zlib.h
#endif
@@ -20,6 +21,10 @@
#include linux/kernel.h
#include linux/types.h
+static int update_cluster_refcount(struct qcow *q, u64 clust_idx, u16
append);
+static int qcow_write_refcount_table(struct qcow *q);
+static u64 qcow_alloc_clusters(struct qcow *q, u64 size, int update_ref);
+static void qcow_free_clusters(struct qcow *q, u64 clust_start, u64 size);
static inline int qcow_pwrite_sync(int fd,
void *buf, size_t count, off_t offset)
@@ -657,6 +662,56 @@ static struct qcow_refcount_block
*refcount_block_search(struct qcow *q, u64 off
return rfb;
}
+static struct qcow_refcount_block *qcow_grow_refcount_block(struct qcow *q,
+ u64 clust_idx)
+{
+ struct qcow_header *header = q-header;
+ struct qcow_refcount_table *rft = q-refcount_table;
+ struct qcow_refcount_block *rfb;
+ u64 new_block_offset;
+ u64 rft_idx;
+
+ rft_idx = clust_idx (header-cluster_bits -
+ QCOW_REFCOUNT_BLOCK_SHIFT);
+
+ if (rft_idx = rft-rf_size) {
+ pr_warning(Don't support grow refcount block table);
+ return NULL;
+ }
+
+ new_block_offset = qcow_alloc_clusters(q, q-cluster_size, 0);
+ if (new_block_offset 0)
+ return NULL;
+
+ rfb = new_refcount_block(q, new_block_offset);
+ if (!rfb)
+ return NULL;
+
+ memset(rfb-entries, 0x00, q-cluster_size);
+ rfb-dirty = 1;
+
+ /* write refcount block */
+ if (write_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ if (cache_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ rft-rf_table[rft_idx] = cpu_to_be64(new_block_offset);
+ if (qcow_write_refcount_table(q) 0)
+ goto free_rfb;
+
+ if (update_cluster_refcount(q, new_block_offset
+ header-cluster_bits, 1) 0)
+ goto free_rfb;
This order is unsafe, you could end up with a refcount block that is
already in use, but still has a refcount of 0.
How about following?
rft-rf_table[rft_idx] = cpu_to_be64(new_block_offset);
if (update_cluster_refcount(q, new_block_offset
header-cluster_bits, 1) 0) {
rft-rf_table[rft_idx] = 0;
goto free_rfb;
}
if (qcow_write_refcount_table(q) 0) {
rft-rf_table[rft_idx] = 0;
qcow_free_clusters(q, new_block_offset, q-cluster_size);
goto free_rfb;
}
update_cluster_refcount() will use the rft-rf_table. So updating the
rft-rf_table
must be before update_cluster_refcount().
Yes, something like this should work.
Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
On Tue, Dec 13, 2011 at 5:16 AM, lan,Tianyu tianyu@intel.com wrote: I have tried to use ERR_PTR(). But when I included linux/err.h, a compile error was following. CC disk/qcow.o In file included from disk/qcow.c:20: ../../include/linux/err.h:22: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'ERR_PTR' ../../include/linux/err.h:27: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'PTR_ERR' ../../include/linux/err.h:32: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'IS_ERR' ../../include/linux/err.h:37: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'IS_ERR_OR_NULL' ../../include/linux/err.h:49: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'ERR_CAST' ../../include/linux/err.h:55: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'PTR_RET' We need to borrow tools/perf/util/include/linux/compiler.h in tools/kvm/include/linux. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
On Tue, Dec 13, 2011 at 10:57 AM, Kevin Wolf kw...@redhat.com wrote: So the ENOSPEC should be specially dealt with. Does this make sense? :) I'm not saying that your code won't work today, just that it's brittle. If someone adds a different error return code somewhere and doesn't check if all callers properly deal with error codes, it will break. Indeed. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
oh，yeah. Thanks for your reply. It's a good lesson for me. :) I will update soon. On 二, 2011-12-13 at 16:57 +0800, Kevin Wolf wrote: Am 13.12.2011 04:41, schrieb lan,Tianyu: On 一, 2011-12-12 at 19:15 +0800, Kevin Wolf wrote: Am 12.12.2011 11:58, schrieb Pekka Enberg: On Mon, 12 Dec 2011, Kevin Wolf wrote: @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 rft_idx = clust_idx (header-cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT); if (rft_idx = rft-rf_size) - return NULL; + return (void *)-ENOSPC; Is this allowed style in kvm-tool? :-/ It needs to use ERR_PTR() and related macros but otherwise I don't see a big problem with it. Can you be sure that it never clashes with a valid allocation when you use this in userspace? But yes, at least using appropriate functions should be required. And this means that you can't only check for -ENOSPC, but you need to check for all possible error codes (IS_ERR_VALUE() I guess). The qcow_read_refcount_block() is invoiked in the two places qcow_get_refcount() and update_cluster_refcount() and will only return NULL or -ENOSPC. In the qcow_get_refcount(), when qcow_read_refcount_block() returned -ENOSPEC(no refcount block is available.), returning zero which means the cluster is not in use and the refcount block can be grew in the update_cluster_refcount(). In the update_cluster_refcount(), when qcow_read_refcount_block() returned -ENOSPEC, it is necessary to grow the refcount blocks. So the ENOSPEC should be specially dealt with. Does this make sense? :) I'm not saying that your code won't work today, just that it's brittle. If someone adds a different error return code somewhere and doesn't check if all callers properly deal with error codes, it will break. Kevin Thanks Tianyu Lan -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
Thanks for your review.
On 一, 2011-12-12 at 17:55 +0800, Kevin Wolf wrote:
Am 12.12.2011 03:03, schrieb Lan Tianyu:
This patch enables allocating new refcount blocks and so then kvm tools
could expand qcow2 image much larger.
Signed-off-by: Lan Tianyu tianyu@intel.com
---
tools/kvm/disk/qcow.c | 105
+---
1 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index e139fa5..929ba69 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -12,6 +12,7 @@
#include string.h
#include unistd.h
#include fcntl.h
+#include errno.h
#ifdef CONFIG_HAS_ZLIB
#include zlib.h
#endif
@@ -20,6 +21,10 @@
#include linux/kernel.h
#include linux/types.h
+static int update_cluster_refcount(struct qcow *q, u64 clust_idx, u16
append);
+static int qcow_write_refcount_table(struct qcow *q);
+static u64 qcow_alloc_clusters(struct qcow *q, u64 size, int update_ref);
+static void qcow_free_clusters(struct qcow *q, u64 clust_start, u64 size);
static inline int qcow_pwrite_sync(int fd,
void *buf, size_t count, off_t offset)
@@ -657,6 +662,56 @@ static struct qcow_refcount_block
*refcount_block_search(struct qcow *q, u64 off
return rfb;
}
+static struct qcow_refcount_block *qcow_grow_refcount_block(struct qcow *q,
+ u64 clust_idx)
+{
+ struct qcow_header *header = q-header;
+ struct qcow_refcount_table *rft = q-refcount_table;
+ struct qcow_refcount_block *rfb;
+ u64 new_block_offset;
+ u64 rft_idx;
+
+ rft_idx = clust_idx (header-cluster_bits -
+ QCOW_REFCOUNT_BLOCK_SHIFT);
+
+ if (rft_idx = rft-rf_size) {
+ pr_warning(Don't support grow refcount block table);
+ return NULL;
+ }
+
+ new_block_offset = qcow_alloc_clusters(q, q-cluster_size, 0);
+ if (new_block_offset 0)
+ return NULL;
+
+ rfb = new_refcount_block(q, new_block_offset);
+ if (!rfb)
+ return NULL;
+
+ memset(rfb-entries, 0x00, q-cluster_size);
+ rfb-dirty = 1;
+
+ /* write refcount block */
+ if (write_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ if (cache_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ rft-rf_table[rft_idx] = cpu_to_be64(new_block_offset);
+ if (qcow_write_refcount_table(q) 0)
+ goto free_rfb;
+
+ if (update_cluster_refcount(q, new_block_offset
+ header-cluster_bits, 1) 0)
+ goto free_rfb;
This order is unsafe, you could end up with a refcount block that is
already in use, but still has a refcount of 0.
How about following?
rft-rf_table[rft_idx] = cpu_to_be64(new_block_offset);
if (update_cluster_refcount(q, new_block_offset
header-cluster_bits, 1) 0) {
rft-rf_table[rft_idx] = 0;
goto free_rfb;
}
if (qcow_write_refcount_table(q) 0) {
rft-rf_table[rft_idx] = 0;
qcow_free_clusters(q, new_block_offset, q-cluster_size);
goto free_rfb;
}
update_cluster_refcount() will use the rft-rf_table. So updating the
rft-rf_table
must be before update_cluster_refcount().
+
+ return rfb;
+
+free_rfb:
+ free(rfb);
+ return NULL;
+}
+
static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow
*q, u64 clust_idx)
{
struct qcow_header *header = q-header;
@@ -667,14 +722,11 @@ static struct qcow_refcount_block
*qcow_read_refcount_block(struct qcow *q, u64
rft_idx = clust_idx (header-cluster_bits -
QCOW_REFCOUNT_BLOCK_SHIFT);
if (rft_idx = rft-rf_size)
- return NULL;
+ return (void *)-ENOSPC;
Is this allowed style in kvm-tool? :-/
Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
On 一, 2011-12-12 at 18:58 +0800, Pekka Enberg wrote:
On Mon, 12 Dec 2011, Kevin Wolf wrote:
@@ -667,14 +722,11 @@ static struct qcow_refcount_block
*qcow_read_refcount_block(struct qcow *q, u64
rft_idx = clust_idx (header-cluster_bits -
QCOW_REFCOUNT_BLOCK_SHIFT);
if (rft_idx = rft-rf_size)
- return NULL;
+ return (void *)-ENOSPC;
Is this allowed style in kvm-tool? :-/
It needs to use ERR_PTR() and related macros but otherwise I don't see a
big problem with it.
Pekka
I have tried to use ERR_PTR(). But when I included linux/err.h, a
compile error was following.
CC disk/qcow.o
In file included from disk/qcow.c:20:
../../include/linux/err.h:22: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'ERR_PTR'
../../include/linux/err.h:27: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'PTR_ERR'
../../include/linux/err.h:32: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'IS_ERR'
../../include/linux/err.h:37: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'IS_ERR_OR_NULL'
../../include/linux/err.h:49: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'ERR_CAST'
../../include/linux/err.h:55: error: expected '=', ',', ';', 'asm' or
'__attribute__' before 'PTR_RET'
in the linux/err.h
static inline void * __must_check ERR_PTR(long error)
{
return (void *) error;
}
when I comment __must_check, the error disappear.
Finally I did cast myself rather than use ERR_PTR().
Are there some substitutions of ERR_PTR in the user space or choices?
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
On 一, 2011-12-12 at 19:15 +0800, Kevin Wolf wrote: Am 12.12.2011 11:58, schrieb Pekka Enberg: On Mon, 12 Dec 2011, Kevin Wolf wrote: @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 rft_idx = clust_idx (header-cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT); if (rft_idx = rft-rf_size) - return NULL; + return (void *)-ENOSPC; Is this allowed style in kvm-tool? :-/ It needs to use ERR_PTR() and related macros but otherwise I don't see a big problem with it. Can you be sure that it never clashes with a valid allocation when you use this in userspace? But yes, at least using appropriate functions should be required. And this means that you can't only check for -ENOSPC, but you need to check for all possible error codes (IS_ERR_VALUE() I guess). The qcow_read_refcount_block() is invoiked in the two places qcow_get_refcount() and update_cluster_refcount() and will only return NULL or -ENOSPC. In the qcow_get_refcount(), when qcow_read_refcount_block() returned -ENOSPEC(no refcount block is available.), returning zero which means the cluster is not in use and the refcount block can be grew in the update_cluster_refcount(). In the update_cluster_refcount(), when qcow_read_refcount_block() returned -ENOSPEC, it is necessary to grow the refcount blocks. So the ENOSPEC should be specially dealt with. Does this make sense? :) Kevin Thanks Tianyu Lan. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
Am 12.12.2011 03:03, schrieb Lan Tianyu:
This patch enables allocating new refcount blocks and so then kvm tools
could expand qcow2 image much larger.
Signed-off-by: Lan Tianyu tianyu@intel.com
---
tools/kvm/disk/qcow.c | 105 +---
1 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index e139fa5..929ba69 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -12,6 +12,7 @@
#include string.h
#include unistd.h
#include fcntl.h
+#include errno.h
#ifdef CONFIG_HAS_ZLIB
#include zlib.h
#endif
@@ -20,6 +21,10 @@
#include linux/kernel.h
#include linux/types.h
+static int update_cluster_refcount(struct qcow *q, u64 clust_idx, u16
append);
+static int qcow_write_refcount_table(struct qcow *q);
+static u64 qcow_alloc_clusters(struct qcow *q, u64 size, int update_ref);
+static void qcow_free_clusters(struct qcow *q, u64 clust_start, u64 size);
static inline int qcow_pwrite_sync(int fd,
void *buf, size_t count, off_t offset)
@@ -657,6 +662,56 @@ static struct qcow_refcount_block
*refcount_block_search(struct qcow *q, u64 off
return rfb;
}
+static struct qcow_refcount_block *qcow_grow_refcount_block(struct qcow *q,
+ u64 clust_idx)
+{
+ struct qcow_header *header = q-header;
+ struct qcow_refcount_table *rft = q-refcount_table;
+ struct qcow_refcount_block *rfb;
+ u64 new_block_offset;
+ u64 rft_idx;
+
+ rft_idx = clust_idx (header-cluster_bits -
+ QCOW_REFCOUNT_BLOCK_SHIFT);
+
+ if (rft_idx = rft-rf_size) {
+ pr_warning(Don't support grow refcount block table);
+ return NULL;
+ }
+
+ new_block_offset = qcow_alloc_clusters(q, q-cluster_size, 0);
+ if (new_block_offset 0)
+ return NULL;
+
+ rfb = new_refcount_block(q, new_block_offset);
+ if (!rfb)
+ return NULL;
+
+ memset(rfb-entries, 0x00, q-cluster_size);
+ rfb-dirty = 1;
+
+ /* write refcount block */
+ if (write_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ if (cache_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ rft-rf_table[rft_idx] = cpu_to_be64(new_block_offset);
+ if (qcow_write_refcount_table(q) 0)
+ goto free_rfb;
+
+ if (update_cluster_refcount(q, new_block_offset
+ header-cluster_bits, 1) 0)
+ goto free_rfb;
This order is unsafe, you could end up with a refcount block that is
already in use, but still has a refcount of 0.
+
+ return rfb;
+
+free_rfb:
+ free(rfb);
+ return NULL;
+}
+
static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q,
u64 clust_idx)
{
struct qcow_header *header = q-header;
@@ -667,14 +722,11 @@ static struct qcow_refcount_block
*qcow_read_refcount_block(struct qcow *q, u64
rft_idx = clust_idx (header-cluster_bits -
QCOW_REFCOUNT_BLOCK_SHIFT);
if (rft_idx = rft-rf_size)
- return NULL;
+ return (void *)-ENOSPC;
Is this allowed style in kvm-tool? :-/
Kevin
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
On Mon, 12 Dec 2011, Kevin Wolf wrote: @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 rft_idx = clust_idx (header-cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT); if (rft_idx = rft-rf_size) - return NULL; + return (void *)-ENOSPC; Is this allowed style in kvm-tool? :-/ It needs to use ERR_PTR() and related macros but otherwise I don't see a big problem with it. Pekka -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
Am 12.12.2011 11:58, schrieb Pekka Enberg: On Mon, 12 Dec 2011, Kevin Wolf wrote: @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 rft_idx = clust_idx (header-cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT); if (rft_idx = rft-rf_size) - return NULL; + return (void *)-ENOSPC; Is this allowed style in kvm-tool? :-/ It needs to use ERR_PTR() and related macros but otherwise I don't see a big problem with it. Can you be sure that it never clashes with a valid allocation when you use this in userspace? But yes, at least using appropriate functions should be required. And this means that you can't only check for -ENOSPC, but you need to check for all possible error codes (IS_ERR_VALUE() I guess). Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
Am 12.12.2011 12:15, schrieb Kevin Wolf: Am 12.12.2011 11:58, schrieb Pekka Enberg: On Mon, 12 Dec 2011, Kevin Wolf wrote: @@ -667,14 +722,11 @@ static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q, u64 rft_idx = clust_idx (header-cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT); if (rft_idx = rft-rf_size) - return NULL; + return (void *)-ENOSPC; Is this allowed style in kvm-tool? :-/ It needs to use ERR_PTR() and related macros but otherwise I don't see a big problem with it. Can you be sure that it never clashes with a valid allocation when you use this in userspace? Er, not sure what I was thinking, but it's the top 4k of the address space, which should be kernel memory. So I think you actually can be sure. Kevin -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH] kvm tools, qcow: Add support for growing refcount blocks
This patch enables allocating new refcount blocks and so then kvm tools
could expand qcow2 image much larger.
Signed-off-by: Lan Tianyu tianyu@intel.com
---
tools/kvm/disk/qcow.c | 105 +---
1 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index e139fa5..929ba69 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -12,6 +12,7 @@
#include string.h
#include unistd.h
#include fcntl.h
+#include errno.h
#ifdef CONFIG_HAS_ZLIB
#include zlib.h
#endif
@@ -20,6 +21,10 @@
#include linux/kernel.h
#include linux/types.h
+static int update_cluster_refcount(struct qcow *q, u64 clust_idx, u16 append);
+static int qcow_write_refcount_table(struct qcow *q);
+static u64 qcow_alloc_clusters(struct qcow *q, u64 size, int update_ref);
+static void qcow_free_clusters(struct qcow *q, u64 clust_start, u64 size);
static inline int qcow_pwrite_sync(int fd,
void *buf, size_t count, off_t offset)
@@ -657,6 +662,56 @@ static struct qcow_refcount_block
*refcount_block_search(struct qcow *q, u64 off
return rfb;
}
+static struct qcow_refcount_block *qcow_grow_refcount_block(struct qcow *q,
+ u64 clust_idx)
+{
+ struct qcow_header *header = q-header;
+ struct qcow_refcount_table *rft = q-refcount_table;
+ struct qcow_refcount_block *rfb;
+ u64 new_block_offset;
+ u64 rft_idx;
+
+ rft_idx = clust_idx (header-cluster_bits -
+ QCOW_REFCOUNT_BLOCK_SHIFT);
+
+ if (rft_idx = rft-rf_size) {
+ pr_warning(Don't support grow refcount block table);
+ return NULL;
+ }
+
+ new_block_offset = qcow_alloc_clusters(q, q-cluster_size, 0);
+ if (new_block_offset 0)
+ return NULL;
+
+ rfb = new_refcount_block(q, new_block_offset);
+ if (!rfb)
+ return NULL;
+
+ memset(rfb-entries, 0x00, q-cluster_size);
+ rfb-dirty = 1;
+
+ /* write refcount block */
+ if (write_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ if (cache_refcount_block(q, rfb) 0)
+ goto free_rfb;
+
+ rft-rf_table[rft_idx] = cpu_to_be64(new_block_offset);
+ if (qcow_write_refcount_table(q) 0)
+ goto free_rfb;
+
+ if (update_cluster_refcount(q, new_block_offset
+ header-cluster_bits, 1) 0)
+ goto free_rfb;
+
+ return rfb;
+
+free_rfb:
+ free(rfb);
+ return NULL;
+}
+
static struct qcow_refcount_block *qcow_read_refcount_block(struct qcow *q,
u64 clust_idx)
{
struct qcow_header *header = q-header;
@@ -667,14 +722,11 @@ static struct qcow_refcount_block
*qcow_read_refcount_block(struct qcow *q, u64
rft_idx = clust_idx (header-cluster_bits -
QCOW_REFCOUNT_BLOCK_SHIFT);
if (rft_idx = rft-rf_size)
- return NULL;
+ return (void *)-ENOSPC;
rfb_offset = be64_to_cpu(rft-rf_table[rft_idx]);
-
- if (!rfb_offset) {
- pr_warning(Don't support to grow refcount table);
- return NULL;
- }
+ if (!rfb_offset)
+ return (void *)-ENOSPC;
rfb = refcount_block_search(q, rfb_offset);
if (rfb)
@@ -708,7 +760,8 @@ static u16 qcow_get_refcount(struct qcow *q, u64 clust_idx)
if (!rfb) {
pr_warning(Error while reading refcount table);
return -1;
- }
+ } else if ((long)rfb == -ENOSPC)
+ return 0;
rfb_idx = clust_idx (((1ULL
(header-cluster_bits - QCOW_REFCOUNT_BLOCK_SHIFT)) - 1));
@@ -732,6 +785,12 @@ static int update_cluster_refcount(struct qcow *q, u64
clust_idx, u16 append)
if (!rfb) {
pr_warning(error while reading refcount table);
return -1;
+ } else if ((long)rfb == -ENOSPC) {
+ rfb = qcow_grow_refcount_block(q, clust_idx);
+ if (!rfb) {
+ pr_warning(error while growing refcount table);
+ return -1;
+ }
}
rfb_idx = clust_idx (((1ULL
@@ -774,11 +833,11 @@ static void qcow_free_clusters(struct qcow *q, u64
clust_start, u64 size)
* can satisfy the size. free_clust_idx is initialized to zero and
* Record last position.
*/
-static u64 qcow_alloc_clusters(struct qcow *q, u64 size)
+static u64 qcow_alloc_clusters(struct qcow *q, u64 size, int update_ref)
{
struct qcow_header *header = q-header;
u16 clust_refcount;
- u32 clust_idx, i;
+ u32 clust_idx = 0, i;
u64 clust_num;
clust_num = (size + (q-cluster_size - 1)) header-cluster_bits;
@@ -793,12 +852,15 @@ again:
goto again;
}
- for (i = 0; i clust_num; i++)
- if (update_cluster_refcount(q,
- q-free_clust_idx -
