On 22.12.19 12:37, Alberto Garcia wrote: > Two changes are needed in order to add subcluster support to this > function: deallocated clusters must have their bitmaps cleared, and > expanded clusters must have all the "subcluster allocated" bits set.
Not really, to have real subcluster support it would need to be expand_zero_subclusters_in_l1(). Right now it can only deal with full zero clusters, which will actually never happen for images with subclusters. As noted in v2, this function is only called when downgrading qcow2 images to v2. It kind of made sense to just call set_l2_bitmap() in v2, but now with the if () conditional... I suppose it may make more sense to assert that the image does not have subclusters at the beginning of the function and be done with it. OTOH, well, this does make ensuring that we have subcluster “support” everywhere a bit easier because this way all set_l2_entry() calls are accompanied by an “if (subclusters) { set_l2_bitmap() }” part. But it is dead code. Max > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > block/qcow2-cluster.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index 207f670c94..ede75138d2 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -2054,6 +2054,9 @@ static int expand_zero_clusters_in_l1(BlockDriverState > *bs, uint64_t *l1_table, > /* not backed; therefore we can simply deallocate the > * cluster */ > set_l2_entry(s, l2_slice, j, 0); > + if (has_subclusters(s)) { > + set_l2_bitmap(s, l2_slice, j, 0); > + } > l2_dirty = true; > continue; > } > @@ -2120,6 +2123,9 @@ static int expand_zero_clusters_in_l1(BlockDriverState > *bs, uint64_t *l1_table, > } else { > set_l2_entry(s, l2_slice, j, offset); > } > + if (has_subclusters(s)) { > + set_l2_bitmap(s, l2_slice, j, QCOW_L2_BITMAP_ALL_ALLOC); > + } > l2_dirty = true; > } > >
signature.asc
Description: OpenPGP digital signature