diff mbox series

[v3,2/8] parallels: create parallels_set_bat_entry_helper() to assign BAT value

Message ID 20220815090216.1818622-3-alexander.ivanov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series parallels: Refactor the code of images checks and fix a bug | expand

Commit Message

Alexander Ivanov Aug. 15, 2022, 9:02 a.m. UTC
This helper will be reused in next patches during parallels_co_check
rework to simplify its code.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
v2: A new patch - a part of a splitted patch.
v3: Fix commit message.

 block/parallels.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Aug. 17, 2022, 7:21 p.m. UTC | #1
On 8/15/22 12:02, Alexander Ivanov wrote:
> This helper will be reused in next patches during parallels_co_check
> rework to simplify its code.
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

> ---
> v2: A new patch - a part of a splitted patch.
> v3: Fix commit message.
> 
>   block/parallels.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/block/parallels.c b/block/parallels.c
> index a76cf9d993..7f68f3cbc9 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -165,6 +165,13 @@ static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,
>       return start_off;
>   }
>   
> +static void parallels_set_bat_entry(BDRVParallelsState *s,
> +                                    uint32_t index, uint32_t offset)

Rather unobvious that offset should be passed already converted to LE. Worth a comment? Or may be do convertion inside function (depends on further usages of the helper)

> +{
> +    s->bat_bitmap[index] = offset;
> +    bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1);
> +}
> +
>   static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
>                                    int nb_sectors, int *pnum)
>   {
> @@ -250,10 +257,9 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
>       }
>   
>       for (i = 0; i < to_allocate; i++) {
> -        s->bat_bitmap[idx + i] = cpu_to_le32(s->data_end / s->off_multiplier);
> +        parallels_set_bat_entry(s, idx + i,
> +                                cpu_to_le32(s->data_end / s->off_multiplier));
>           s->data_end += s->tracks;
> -        bitmap_set(s->bat_dirty_bmap,
> -                   bat_entry_off(idx + i) / s->bat_dirty_block, 1);
>       }
>   
>       return bat2sect(s, idx) + sector_num % s->tracks;
Alexander Ivanov Aug. 18, 2022, 7:31 a.m. UTC | #2
On 17.08.2022 21:21, Vladimir Sementsov-Ogievskiy wrote:
> On 8/15/22 12:02, Alexander Ivanov wrote:
>> This helper will be reused in next patches during parallels_co_check
>> rework to simplify its code.
>>
>> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>
>> ---
>> v2: A new patch - a part of a splitted patch.
>> v3: Fix commit message.
>>
>>   block/parallels.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/parallels.c b/block/parallels.c
>> index a76cf9d993..7f68f3cbc9 100644
>> --- a/block/parallels.c
>> +++ b/block/parallels.c
>> @@ -165,6 +165,13 @@ static int64_t block_status(BDRVParallelsState 
>> *s, int64_t sector_num,
>>       return start_off;
>>   }
>>   +static void parallels_set_bat_entry(BDRVParallelsState *s,
>> +                                    uint32_t index, uint32_t offset)
>
> Rather unobvious that offset should be passed already converted to LE. 
> Worth a comment? Or may be do convertion inside function (depends on 
> further usages of the helper)
I agree, it would be better to convert the offset inside the helper.
>
>> +{
>> +    s->bat_bitmap[index] = offset;
>> +    bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / 
>> s->bat_dirty_block, 1);
>> +}
>> +
>>   static int64_t allocate_clusters(BlockDriverState *bs, int64_t 
>> sector_num,
>>                                    int nb_sectors, int *pnum)
>>   {
>> @@ -250,10 +257,9 @@ static int64_t 
>> allocate_clusters(BlockDriverState *bs, int64_t sector_num,
>>       }
>>         for (i = 0; i < to_allocate; i++) {
>> -        s->bat_bitmap[idx + i] = cpu_to_le32(s->data_end / 
>> s->off_multiplier);
>> +        parallels_set_bat_entry(s, idx + i,
>> +                                cpu_to_le32(s->data_end / 
>> s->off_multiplier));
>>           s->data_end += s->tracks;
>> -        bitmap_set(s->bat_dirty_bmap,
>> -                   bat_entry_off(idx + i) / s->bat_dirty_block, 1);
>>       }
>>         return bat2sect(s, idx) + sector_num % s->tracks;
>
>
diff mbox series

Patch

diff --git a/block/parallels.c b/block/parallels.c
index a76cf9d993..7f68f3cbc9 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -165,6 +165,13 @@  static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,
     return start_off;
 }
 
+static void parallels_set_bat_entry(BDRVParallelsState *s,
+                                    uint32_t index, uint32_t offset)
+{
+    s->bat_bitmap[index] = offset;
+    bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1);
+}
+
 static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
                                  int nb_sectors, int *pnum)
 {
@@ -250,10 +257,9 @@  static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
     }
 
     for (i = 0; i < to_allocate; i++) {
-        s->bat_bitmap[idx + i] = cpu_to_le32(s->data_end / s->off_multiplier);
+        parallels_set_bat_entry(s, idx + i,
+                                cpu_to_le32(s->data_end / s->off_multiplier));
         s->data_end += s->tracks;
-        bitmap_set(s->bat_dirty_bmap,
-                   bat_entry_off(idx + i) / s->bat_dirty_block, 1);
     }
 
     return bat2sect(s, idx) + sector_num % s->tracks;