Re: [Qemu-block] [PATCH 1/2] qcow2: add reduce image support

[Kevin Wolf]

Am 02.06.2017 um 11:53 hat Pavel Butsykin geschrieben:
> On 01.06.2017 17:41, Kevin Wolf wrote:
> >Am 31.05.2017 um 16:43 hat Pavel Butsykin geschrieben:
> >>This patch adds the reduction of the image file for qcow2. As a result, this
> >>allows us to reduce the virtual image size and free up space on the disk 
> >>without
> >>copying the image. Image can be fragmented and reduction is done by punching
> >>holes in the image file.
> >>
> >>Signed-off-by: Pavel Butsykin <address@hidden>
> >>---
> >>  block/qcow2-cache.c    |  8 +++++
> >>  block/qcow2-cluster.c  | 83 
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  block/qcow2-refcount.c | 65 +++++++++++++++++++++++++++++++++++++++
> >>  block/qcow2.c          | 40 ++++++++++++++++++------
> >>  block/qcow2.h          |  4 +++
> >>  qapi/block-core.json   |  4 ++-
> >>  6 files changed, 193 insertions(+), 11 deletions(-)
> >>
> >>diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> >>index 1d25147392..da55118ca7 100644
> >>--- a/block/qcow2-cache.c
> >>+++ b/block/qcow2-cache.c
> >>@@ -411,3 +411,11 @@ void qcow2_cache_entry_mark_dirty(BlockDriverState 
> >>*bs, Qcow2Cache *c,
> >>      assert(c->entries[i].offset != 0);
> >>      c->entries[i].dirty = true;
> >>  }
> >>+
> >>+void qcow2_cache_entry_mark_clean(BlockDriverState *bs, Qcow2Cache *c,
> >>+     void *table)
> >>+{
> >>+    int i = qcow2_cache_get_table_idx(bs, c, table);
> >>+    assert(c->entries[i].offset != 0);
> >>+    c->entries[i].dirty = false;
> >>+}
> >
> >This is an interesting function. We can use it whenever we're not
> >interested in the content of the table any more. However, we still keep
> >that data in the cache and may even evict other tables before this one.
> >The data in the cache also becomes inconsistent with the data in the
> >file, which should not be a problem in theory (because nobody should be
> >using it), but it surely could be confusing when debugging something in
> >the cache.
> >
> 
> Good idea!
> 
> >We can easily improve this a little: Make it qcow2_cache_discard(), a
> >function that gets a cluster offset, asserts that a table at this
> >offset isn't in use (not cached or ref == 0), and then just directly
> >drops it from the cache. This can be called from update_refcount()
> >whenever a refcount goes to 0, immediately before or after calling
> >update_refcount_discard() - those two are closely related. Then this
> >would automatically also be used for L2 tables.
> >
> 
> Did I understand correctly? Every time we need to check the incoming
> offset to make sure it is offset to L2/refcount table (not to the
> guest data) ?

Yes. Basically, whenever the refcount of a cluster becomes 0 and it is
in a cache, remove it from the cache.

Kevin