On Mon 23 Nov 2020 05:09:41 PM CET, Kevin Wolf wrote: >> Commit 205fa50750 ("qcow2: Add subcluster support to zero_in_l2_slice()") >> introduced a subtle change to code in zero_in_l2_slice: >> >> It swapped the order of >> >> 1. qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice); >> 2. set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO); >> 3. qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST); >> >> To >> >> 1. qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice); >> 2. qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST); >> 3. set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
Ouch :( Good catch! >> - qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice); >> if (unmap) { >> qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST); >> } >> set_l2_entry(s, l2_slice, l2_index + i, new_l2_entry); >> + qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice); > > Good catch, but I think your order is wrong, too. We need the original > order from before 205fa50750: > > 1. qcow2_cache_entry_mark_dirty() > set_l2_entry() + set_l2_bitmap() > > 2. qcow2_free_any_cluster() I agree with Kevin on this. Berto