Am 05.08.2011 09:34, schrieb Frediano Ziglio: > This function seems to be not that safe. > > The first problem is that the first thing it does if free refcount > calling qcow2_update_snapshot_refcount(bs, s->l1_table_offset, > s->l1_size, -1). Now if something goes wrong (crash, disk errors) this > could lead to refcount == 0 (depending on L1 size, cache, open flags). > > If table grow and qcow2_grow_l1_table needs to resize on disk it > probably will found some reference counter set to 0 (cause we already > decremented it), fail than qcow2_snapshot_goto return -EIO leaving > refcounts deallocated.
I can't say I'm surprised. This kind of problem used to be common all over the place in qcow2. I fixed it for most paths, but internal snapshots aren't considered supported for RHEL, so I never got around to fixing the snapshot code. So, yes, the order must be changed, so that in the worst case we have cluster leaks. Kevin