Thanks. I will give a new version in v3 of qcow2 shrink. Jun Li 2014-9-3 上午1:12于 "Greg Kurz" <gk...@linux.vnet.ibm.com>写道:
> On Mon, 1 Sep 2014 18:52:48 +0800 > Jun Li <junm...@gmail.com> wrote: > > > When every item of refcount block is NULL, free refcount block and reset > the > > corresponding item of refcount table with NULL. > > > > Signed-off-by: Jun Li <address@hidden> > > --- > > > > The v2 do following change to modify some potential issue. > > > > +------- Here should start from "0". > > | > > for (k = 0; k < refcount_block_entries; k++) { > > if (refcount_block[k] != cpu_to_be16(0)) { > > ... | | > > } | | > > } | +---- Using "0" is more > safe. > > | > > +-------- This should be "k" not "++k". > > --- > > block/qcow2-refcount.c | 31 +++++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > > index 43665b8..63f36e6 100644 > > --- a/block/qcow2-refcount.c > > +++ b/block/qcow2-refcount.c > > @@ -586,6 +586,37 @@ static int QEMU_WARN_UNUSED_RESULT > update_refcount(BlockDriverState *bs, > > if (refcount == 0 && s->discard_passthrough[type]) { > > update_refcount_discard(bs, cluster_offset, > s->cluster_size); > > } > > + > > + /* When refcount block is NULL, update refcount table */ > > + if (block_index == 0) { > > + int k = block_index; > > Hi, > > k = 0 is also done in the for block below... > > > + int refcount_block_entries = s->cluster_size / > sizeof(uint16_t); > > It's better for maintainance to count elements in an array this way: > > int refcount_block_entries = s->cluster_size / sizeof(refcount_block[0]); > > > + for (k = 0; k < refcount_block_entries; k++) { > > + if (refcount_block[k] != cpu_to_be16(0)) { > > + break; > > + } > > + } > > + > > + if (k == refcount_block_entries) { > > + qemu_vfree(refcount_block); > > + /* update refcount table */ > > + unsigned int refcount_table_index; > > + uint64_t data64 = cpu_to_be64(0); > > + refcount_table_index = cluster_index >> > (s->cluster_bits - > > + REFCOUNT_SHIFT); > > + ret = bdrv_pwrite_sync(bs->file, > > + s->refcount_table_offset + > > + refcount_table_index * > > + sizeof(uint64_t), > > + &data64, sizeof(data64)); > > + if (ret < 0) { > > + goto fail; > > + } > > + > > + s->refcount_table[refcount_table_index] = data64; > > + > > + } > > + } > > } > > > > ret = 0; > > Cheers. > > -- > Gregory Kurz kurzg...@fr.ibm.com > gk...@linux.vnet.ibm.com > Software Engineer @ IBM/Meiosys http://www.ibm.com > Tel +33 (0)562 165 496 > > "Anarchy is about taking complete responsibility for yourself." > Alan Moore. > >