diff mbox series

[RFC,v2,18/26] qcow2: Add subcluster support to expand_zero_clusters_in_l1()

Message ID f99f051139f84a4d34bc52696aa2c2b125d5c3fd.1572125022.git.berto@igalia.com
State New
Headers show
Series Add subcluster allocation to qcow2 | expand

Commit Message

Alberto Garcia Oct. 26, 2019, 9:25 p.m. UTC
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.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2-cluster.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Max Reitz Nov. 5, 2019, 11:05 a.m. UTC | #1
On 26.10.19 23:25, 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.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2-cluster.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index aa3eb727a5..62f2a9fcc0 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -2036,6 +2036,7 @@ 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);
> +                        set_l2_bitmap(s, l2_slice, j, 0);
>                          l2_dirty = true;
>                          continue;
>                      }
> @@ -2102,6 +2103,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
>                  } else {
>                      set_l2_entry(s, l2_slice, j, offset);
>                  }
> +                set_l2_bitmap(s, l2_slice, j, QCOW_L2_BITMAP_ALL_ALLOC);
>                  l2_dirty = true;
>              }

Technically this isn’t needed because this function is only called when
downgrading v3 to v2 images (which can’t have subclusters), but of
course it won’t hurt.

Max
Alberto Garcia Nov. 14, 2019, 3:43 p.m. UTC | #2
On Tue 05 Nov 2019 12:05:02 PM CET, Max Reitz wrote:
>> @@ -2102,6 +2103,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
>>                  } else {
>>                      set_l2_entry(s, l2_slice, j, offset);
>>                  }
>> +                set_l2_bitmap(s, l2_slice, j, QCOW_L2_BITMAP_ALL_ALLOC);
>>                  l2_dirty = true;
>>              }
>
> Technically this isn’t needed because this function is only called
> when downgrading v3 to v2 images (which can’t have subclusters), but
> of course it won’t hurt.

Right, but we need to change the function anyway to either do this or
assert that there are no subclusters. I prefer to do this because it's
trivial but I won't oppose if someone prefers the alternative.

Berto
diff mbox series

Patch

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index aa3eb727a5..62f2a9fcc0 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2036,6 +2036,7 @@  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);
+                        set_l2_bitmap(s, l2_slice, j, 0);
                         l2_dirty = true;
                         continue;
                     }
@@ -2102,6 +2103,7 @@  static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
                 } else {
                     set_l2_entry(s, l2_slice, j, offset);
                 }
+                set_l2_bitmap(s, l2_slice, j, QCOW_L2_BITMAP_ALL_ALLOC);
                 l2_dirty = true;
             }