Re: [RFC PATCH v3 19/27] qcow2: Add subcluster support to expand_zero_clusters_in_l1()

[Max Reitz]

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 <address@hidden>
> ---
>  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