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.lan@xxxxxxxxx>
>>> ---
>>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
