[v4] Add subcluster allocation to qcow2

[PATCH v4 21/30] qcow2: Add subcluster support to check_refcounts_l2()

Posted by Alberto Garcia 5 years, 10 months ago

Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
image has subclusters. Instead, the individual 'all zeroes' bits must
be used.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2-refcount.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 3b89a97fd0..9337496c84 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1686,8 +1686,13 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
                         int ign = active ? QCOW2_OL_ACTIVE_L2 :
                                            QCOW2_OL_INACTIVE_L2;
 
-                        l2_entry = QCOW_OFLAG_ZERO;
-                        set_l2_entry(s, l2_table, i, l2_entry);
+                        if (has_subclusters(s)) {
+                            set_l2_entry(s, l2_table, i, 0);
+                            set_l2_bitmap(s, l2_table, i,
+                                          QCOW_L2_BITMAP_ALL_ZEROES);
+                        } else {
+                            set_l2_entry(s, l2_table, i, QCOW_OFLAG_ZERO);
+                        }
                         ret = qcow2_pre_write_overlap_check(bs, ign,
                                 l2e_offset, l2_entry_size(s), false);
                         if (ret < 0) {
-- 
2.20.1

Re: [PATCH v4 21/30] qcow2: Add subcluster support to check_refcounts_l2()

Posted by Vladimir Sementsov-Ogievskiy 5 years, 9 months ago

17.03.2020 21:16, Alberto Garcia wrote:
> Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
> image has subclusters. Instead, the individual 'all zeroes' bits must
> be used.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Patch itself seems correct.. Still, would be good also to check, is QCOW_OFLAG_ZERO set in subclustres case and add corresponding corruptions++, and may be even fix (by using  QCOW_L2_BITMAP_ALL_ZEROES instead)

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> ---
>   block/qcow2-refcount.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 3b89a97fd0..9337496c84 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -1686,8 +1686,13 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res,
>                           int ign = active ? QCOW2_OL_ACTIVE_L2 :
>                                              QCOW2_OL_INACTIVE_L2;
>   
> -                        l2_entry = QCOW_OFLAG_ZERO;
> -                        set_l2_entry(s, l2_table, i, l2_entry);
> +                        if (has_subclusters(s)) {
> +                            set_l2_entry(s, l2_table, i, 0);
> +                            set_l2_bitmap(s, l2_table, i,
> +                                          QCOW_L2_BITMAP_ALL_ZEROES);
> +                        } else {
> +                            set_l2_entry(s, l2_table, i, QCOW_OFLAG_ZERO);
> +                        }
>                           ret = qcow2_pre_write_overlap_check(bs, ign,
>                                   l2e_offset, l2_entry_size(s), false);
>                           if (ret < 0) {
> 


-- 
Best regards,
Vladimir

Re: [PATCH v4 21/30] qcow2: Add subcluster support to check_refcounts_l2()

Posted by Alberto Garcia 5 years, 9 months ago

On Wed 22 Apr 2020 02:06:56 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> 17.03.2020 21:16, Alberto Garcia wrote:
>> Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
>> image has subclusters. Instead, the individual 'all zeroes' bits must
>> be used.
>> 
>> Signed-off-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
> Patch itself seems correct.. Still, would be good also to check, is
> QCOW_OFLAG_ZERO set in subclustres case and add corresponding
> corruptions++, and may be even fix (by using
> QCOW_L2_BITMAP_ALL_ZEROES instead)

I'll add it to my TODO list for a later patch.

Berto