[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [patch 5/5][v2] Try to aggregate free clusters and free
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [patch 5/5][v2] Try to aggregate free clusters and freed clusters |
Date: |
Mon, 11 Aug 2008 15:13:11 +0200 |
User-agent: |
Thunderbird 2.0.0.12 (X11/20071114) |
Hi Laurent,
Laurent Vivier schrieb:
> In free_used_clusters(), try to aggregate free clusters and freed clusters.
>
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
> block-qcow2.c | 61
> ++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 36 insertions(+), 25 deletions(-)
I know that this patch will be rewritten as well, but I'll comment on it
nevertheless. Maybe the comments apply to your new version, too.
> Index: qemu/block-qcow2.c
> ===================================================================
> --- qemu.orig/block-qcow2.c 2008-07-29 15:22:28.000000000 +0200
> +++ qemu/block-qcow2.c 2008-07-29 15:22:30.000000000 +0200
> @@ -714,22 +703,44 @@ static uint64_t free_used_clusters(Block
> return 0;
> }
>
> - if (nb_clusters) {
> - int i = 1;
> - uint64_t current;
> - while (i < *nb_clusters) {
> - current = be64_to_cpu((*l2_table)[(*l2_index) + i]);
> - if (cluster_offset + (i << s->cluster_bits) != current)
> - break;
> - i++;
> - }
> - *nb_clusters = i;
> - free_clusters(bs, cluster_offset, i << s->cluster_bits);
> + if (!nb_clusters) {
> + if (cluster_offset)
> + free_clusters(bs, cluster_offset, s->cluster_size);
> return 0;
> }
>
> - free_clusters(bs, cluster_offset, s->cluster_size);
> + i = 0;
> + do_loop = 1;
> + while (i < *nb_clusters && do_loop) {
> + i++;
> + if (!cluster_offset) {
> + while (i < *nb_clusters) {
> + cluster_offset = (*l2_table)[(*l2_index) + i];
> + if (cluster_offset)
> + break;
> + i++;
> + }
> + if ((cluster_offset & QCOW_OFLAG_COPIED) ||
> + (cluster_offset & QCOW_OFLAG_COMPRESSED))
> + do_loop = 0;
> + } else {
> + j = 1;
> + current = 0;
> + while (i < *nb_clusters) {
> + current = be64_to_cpu((*l2_table)[(*l2_index) + i]);
> + if (cluster_offset + (j << s->cluster_bits) != current)
> + break;
> + i++;
> + j++;
> + }
> + free_clusters(bs, cluster_offset, j << s->cluster_bits);
> + cluster_offset = current;
> + if (current)
> + do_loop = 0;
The else block changes its behaviour with this patch: I think you should
take QCOW_OFLAG_COPIED into consideration. In this implementation, when
a cluster with QCOW_OFLAG_COPIED is found, the inner loop aborts, but
the outer one will just call the else block once again. So you're also
accumulating clusters with QCOW_OFLAG_COPIED set.
BTW, why do you use that do_loop variable instead of a break statement?
Kevin
- Re: [Qemu-devel] [patch 5/5][v2] Try to aggregate free clusters and freed clusters,
Kevin Wolf <=