Re: [PATCH 15/19] parallels: Remove unnecessary data_end field

2023-10-08 Thread Alexander Ivanov 




On 10/7/23 19:54, Mike Maslenkin wrote:

On Sat, Oct 7, 2023 at 5:30 PM Alexander Ivanov
 wrote:



On 10/7/23 13:21, Mike Maslenkin wrote:

On Sat, Oct 7, 2023 at 1:18 PM Alexander Ivanov
  wrote:

On 10/6/23 21:43, Mike Maslenkin wrote:

On Mon, Oct 2, 2023 at 12:01 PM Alexander Ivanov
  wrote:

Since we have used bitmap, field data_end in BDRVParallelsState is
redundant and can be removed.

Add parallels_data_end() helper and remove data_end handling.

Signed-off-by: Alexander Ivanov
---
block/parallels.c | 33 +
block/parallels.h |  1 -
2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 48ea5b3f03..80a7171b84 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -265,6 +265,13 @@ static void parallels_free_used_bitmap(BlockDriverState 
*bs)
g_free(s->used_bmap);
}

+static int64_t parallels_data_end(BDRVParallelsState *s)
+{
+int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
+data_end += s->used_bmap_size * s->cluster_size;
+return data_end;
+}
+
int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
 int64_t *clusters)
{
@@ -275,7 +282,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,

first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
if (first_free == s->used_bmap_size) {
-host_off = s->data_end * BDRV_SECTOR_SIZE;
+host_off = parallels_data_end(s);
prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
bytes = prealloc_clusters * s->cluster_size;

@@ -297,9 +304,6 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
s->used_bmap = bitmap_zero_extend(s->used_bmap, s->used_bmap_size,
  new_usedsize);
s->used_bmap_size = new_usedsize;
-if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
-s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
-}
} else {
next_used = find_next_bit(s->used_bmap, s->used_bmap_size, 
first_free);

@@ -315,8 +319,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
 * branch. In the other case we are likely re-using hole. 
Preallocate
 * the space if required by the prealloc_mode.
 */
-if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
-host_off < s->data_end * BDRV_SECTOR_SIZE) {
+if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
if (ret < 0) {
return ret;
@@ -757,13 +760,7 @@ parallels_check_outside_image(BlockDriverState *bs, 
BdrvCheckResult *res,
}
}

-if (high_off == 0) {
-res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
-} else {
-res->image_end_offset = high_off + s->cluster_size;
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-}
-
+res->image_end_offset = parallels_data_end(s);
return 0;
}

@@ -806,7 +803,6 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult 
*res,
res->check_errors++;
return ret;
}
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;

parallels_free_used_bitmap(bs);
ret = parallels_fill_used_bitmap(bs);
@@ -1361,8 +1357,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
}

s->data_start = data_start;
-s->data_end = s->data_start;
-if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
+if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
/*
 * There is not enough unused space to fit to block align between 
BAT
 * and actual data. We can't avoid read-modify-write...
@@ -1403,11 +1398,10 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,

for (i = 0; i < s->bat_size; i++) {
sector = bat2sect(s, i);
-if (sector + s->tracks > s->data_end) {
-s->data_end = sector + s->tracks;
+if (sector + s->tracks > file_nb_sectors) {
+need_check = true;
}
}
-need_check = need_check || s->data_end > file_nb_sectors;

ret = parallels_fill_used_bitmap(bs);
if (ret == -ENOMEM) {
@@ -1461,7 +1455,6 @@ static int 
parallels_truncate_unused_clusters(BlockDriverState *bs)
end_off = (end_off + 1) * s->cluster_size;
}
end_off += s->data_start * BDRV_SECTOR_SIZE;
-s->data_end = end_off / BDRV_SECTOR_SIZE;
return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, 
NULL);
}

diff --git a/block/parallels.h b/block/parallels.h
index 18b4f8068e..a6a048d890 100644
--- a/block/parallels.h
+++ b/block/parallel

Re: [PATCH 15/19] parallels: Remove unnecessary data_end field

2023-10-07 Thread Mike Maslenkin 
On Sat, Oct 7, 2023 at 5:30 PM Alexander Ivanov
 wrote:
>
>
>
> On 10/7/23 13:21, Mike Maslenkin wrote:
> > On Sat, Oct 7, 2023 at 1:18 PM Alexander Ivanov
> >   wrote:
> >>
> >> On 10/6/23 21:43, Mike Maslenkin wrote:
> >>> On Mon, Oct 2, 2023 at 12:01 PM Alexander Ivanov
> >>>   wrote:
>  Since we have used bitmap, field data_end in BDRVParallelsState is
>  redundant and can be removed.
> 
>  Add parallels_data_end() helper and remove data_end handling.
> 
>  Signed-off-by: Alexander Ivanov
>  ---
> block/parallels.c | 33 +
> block/parallels.h |  1 -
> 2 files changed, 13 insertions(+), 21 deletions(-)
> 
>  diff --git a/block/parallels.c b/block/parallels.c
>  index 48ea5b3f03..80a7171b84 100644
>  --- a/block/parallels.c
>  +++ b/block/parallels.c
>  @@ -265,6 +265,13 @@ static void 
>  parallels_free_used_bitmap(BlockDriverState *bs)
> g_free(s->used_bmap);
> }
> 
>  +static int64_t parallels_data_end(BDRVParallelsState *s)
>  +{
>  +int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
>  +data_end += s->used_bmap_size * s->cluster_size;
>  +return data_end;
>  +}
>  +
> int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
>  int64_t *clusters)
> {
>  @@ -275,7 +282,7 @@ int64_t 
>  parallels_allocate_host_clusters(BlockDriverState *bs,
> 
> first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
> if (first_free == s->used_bmap_size) {
>  -host_off = s->data_end * BDRV_SECTOR_SIZE;
>  +host_off = parallels_data_end(s);
> prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
> bytes = prealloc_clusters * s->cluster_size;
> 
>  @@ -297,9 +304,6 @@ int64_t 
>  parallels_allocate_host_clusters(BlockDriverState *bs,
> s->used_bmap = bitmap_zero_extend(s->used_bmap, 
>  s->used_bmap_size,
>   new_usedsize);
> s->used_bmap_size = new_usedsize;
>  -if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
>  -s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
>  -}
> } else {
> next_used = find_next_bit(s->used_bmap, s->used_bmap_size, 
>  first_free);
> 
>  @@ -315,8 +319,7 @@ int64_t 
>  parallels_allocate_host_clusters(BlockDriverState *bs,
>  * branch. In the other case we are likely re-using hole. 
>  Preallocate
>  * the space if required by the prealloc_mode.
>  */
>  -if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
>  -host_off < s->data_end * BDRV_SECTOR_SIZE) {
>  +if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
> ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
> if (ret < 0) {
> return ret;
>  @@ -757,13 +760,7 @@ parallels_check_outside_image(BlockDriverState *bs, 
>  BdrvCheckResult *res,
> }
> }
> 
>  -if (high_off == 0) {
>  -res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
>  -} else {
>  -res->image_end_offset = high_off + s->cluster_size;
>  -s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
>  -}
>  -
>  +res->image_end_offset = parallels_data_end(s);
> return 0;
> }
> 
>  @@ -806,7 +803,6 @@ parallels_check_leak(BlockDriverState *bs, 
>  BdrvCheckResult *res,
> res->check_errors++;
> return ret;
> }
>  -s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
> 
> parallels_free_used_bitmap(bs);
> ret = parallels_fill_used_bitmap(bs);
>  @@ -1361,8 +1357,7 @@ static int parallels_open(BlockDriverState *bs, 
>  QDict *options, int flags,
> }
> 
> s->data_start = data_start;
>  -s->data_end = s->data_start;
>  -if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
>  +if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
> /*
>  * There is not enough unused space to fit to block align 
>  between BAT
>  * and actual data. We can't avoid read-modify-write...
>  @@ -1403,11 +1398,10 @@ static int parallels_open(BlockDriverState *bs, 
>  QDict *options, int flags,
> 
> for (i = 0; i < s->bat_size; i++) {
> sector = bat2sect(s, i);
>  -if (sector + s->tracks > s->data_end) {
>  -s->data_end = sector + s->tracks;
>  +if (sector +

Re: [PATCH 15/19] parallels: Remove unnecessary data_end field

2023-10-07 Thread Alexander Ivanov 




On 10/7/23 13:21, Mike Maslenkin wrote:

On Sat, Oct 7, 2023 at 1:18 PM Alexander Ivanov
  wrote:


On 10/6/23 21:43, Mike Maslenkin wrote:

On Mon, Oct 2, 2023 at 12:01 PM Alexander Ivanov
  wrote:

Since we have used bitmap, field data_end in BDRVParallelsState is
redundant and can be removed.

Add parallels_data_end() helper and remove data_end handling.

Signed-off-by: Alexander Ivanov
---
   block/parallels.c | 33 +
   block/parallels.h |  1 -
   2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 48ea5b3f03..80a7171b84 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -265,6 +265,13 @@ static void parallels_free_used_bitmap(BlockDriverState 
*bs)
   g_free(s->used_bmap);
   }

+static int64_t parallels_data_end(BDRVParallelsState *s)
+{
+int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
+data_end += s->used_bmap_size * s->cluster_size;
+return data_end;
+}
+
   int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
int64_t *clusters)
   {
@@ -275,7 +282,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,

   first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
   if (first_free == s->used_bmap_size) {
-host_off = s->data_end * BDRV_SECTOR_SIZE;
+host_off = parallels_data_end(s);
   prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
   bytes = prealloc_clusters * s->cluster_size;

@@ -297,9 +304,6 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
   s->used_bmap = bitmap_zero_extend(s->used_bmap, s->used_bmap_size,
 new_usedsize);
   s->used_bmap_size = new_usedsize;
-if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
-s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
-}
   } else {
   next_used = find_next_bit(s->used_bmap, s->used_bmap_size, 
first_free);

@@ -315,8 +319,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
* branch. In the other case we are likely re-using hole. Preallocate
* the space if required by the prealloc_mode.
*/
-if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
-host_off < s->data_end * BDRV_SECTOR_SIZE) {
+if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
   ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
   if (ret < 0) {
   return ret;
@@ -757,13 +760,7 @@ parallels_check_outside_image(BlockDriverState *bs, 
BdrvCheckResult *res,
   }
   }

-if (high_off == 0) {
-res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
-} else {
-res->image_end_offset = high_off + s->cluster_size;
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-}
-
+res->image_end_offset = parallels_data_end(s);
   return 0;
   }

@@ -806,7 +803,6 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult 
*res,
   res->check_errors++;
   return ret;
   }
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;

   parallels_free_used_bitmap(bs);
   ret = parallels_fill_used_bitmap(bs);
@@ -1361,8 +1357,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
   }

   s->data_start = data_start;
-s->data_end = s->data_start;
-if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
+if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
   /*
* There is not enough unused space to fit to block align between BAT
* and actual data. We can't avoid read-modify-write...
@@ -1403,11 +1398,10 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,

   for (i = 0; i < s->bat_size; i++) {
   sector = bat2sect(s, i);
-if (sector + s->tracks > s->data_end) {
-s->data_end = sector + s->tracks;
+if (sector + s->tracks > file_nb_sectors) {
+need_check = true;
   }
   }
-need_check = need_check || s->data_end > file_nb_sectors;

   ret = parallels_fill_used_bitmap(bs);
   if (ret == -ENOMEM) {
@@ -1461,7 +1455,6 @@ static int 
parallels_truncate_unused_clusters(BlockDriverState *bs)
   end_off = (end_off + 1) * s->cluster_size;
   }
   end_off += s->data_start * BDRV_SECTOR_SIZE;
-s->data_end = end_off / BDRV_SECTOR_SIZE;
   return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, 
NULL);
   }

diff --git a/block/parallels.h b/block/parallels.h
index 18b4f8068e..a6a048d890 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -79,7 +79,6 @@ typedef struct BDRVParallelsState {
   unsigned int bat_size;

   int64_t  data_start;
-int64_t  data_end;
   u

Re: [PATCH 15/19] parallels: Remove unnecessary data_end field

2023-10-07 Thread Mike Maslenkin 
On Sat, Oct 7, 2023 at 1:18 PM Alexander Ivanov
 wrote:
>
>
>
> On 10/6/23 21:43, Mike Maslenkin wrote:
> > On Mon, Oct 2, 2023 at 12:01 PM Alexander Ivanov
> >  wrote:
> >> Since we have used bitmap, field data_end in BDRVParallelsState is
> >> redundant and can be removed.
> >>
> >> Add parallels_data_end() helper and remove data_end handling.
> >>
> >> Signed-off-by: Alexander Ivanov 
> >> ---
> >>   block/parallels.c | 33 +
> >>   block/parallels.h |  1 -
> >>   2 files changed, 13 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/block/parallels.c b/block/parallels.c
> >> index 48ea5b3f03..80a7171b84 100644
> >> --- a/block/parallels.c
> >> +++ b/block/parallels.c
> >> @@ -265,6 +265,13 @@ static void 
> >> parallels_free_used_bitmap(BlockDriverState *bs)
> >>   g_free(s->used_bmap);
> >>   }
> >>
> >> +static int64_t parallels_data_end(BDRVParallelsState *s)
> >> +{
> >> +int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
> >> +data_end += s->used_bmap_size * s->cluster_size;
> >> +return data_end;
> >> +}
> >> +
> >>   int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
> >>int64_t *clusters)
> >>   {
> >> @@ -275,7 +282,7 @@ int64_t 
> >> parallels_allocate_host_clusters(BlockDriverState *bs,
> >>
> >>   first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
> >>   if (first_free == s->used_bmap_size) {
> >> -host_off = s->data_end * BDRV_SECTOR_SIZE;
> >> +host_off = parallels_data_end(s);
> >>   prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
> >>   bytes = prealloc_clusters * s->cluster_size;
> >>
> >> @@ -297,9 +304,6 @@ int64_t 
> >> parallels_allocate_host_clusters(BlockDriverState *bs,
> >>   s->used_bmap = bitmap_zero_extend(s->used_bmap, 
> >> s->used_bmap_size,
> >> new_usedsize);
> >>   s->used_bmap_size = new_usedsize;
> >> -if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
> >> -s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
> >> -}
> >>   } else {
> >>   next_used = find_next_bit(s->used_bmap, s->used_bmap_size, 
> >> first_free);
> >>
> >> @@ -315,8 +319,7 @@ int64_t 
> >> parallels_allocate_host_clusters(BlockDriverState *bs,
> >>* branch. In the other case we are likely re-using hole. 
> >> Preallocate
> >>* the space if required by the prealloc_mode.
> >>*/
> >> -if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
> >> -host_off < s->data_end * BDRV_SECTOR_SIZE) {
> >> +if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
> >>   ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
> >>   if (ret < 0) {
> >>   return ret;
> >> @@ -757,13 +760,7 @@ parallels_check_outside_image(BlockDriverState *bs, 
> >> BdrvCheckResult *res,
> >>   }
> >>   }
> >>
> >> -if (high_off == 0) {
> >> -res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
> >> -} else {
> >> -res->image_end_offset = high_off + s->cluster_size;
> >> -s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
> >> -}
> >> -
> >> +res->image_end_offset = parallels_data_end(s);
> >>   return 0;
> >>   }
> >>
> >> @@ -806,7 +803,6 @@ parallels_check_leak(BlockDriverState *bs, 
> >> BdrvCheckResult *res,
> >>   res->check_errors++;
> >>   return ret;
> >>   }
> >> -s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
> >>
> >>   parallels_free_used_bitmap(bs);
> >>   ret = parallels_fill_used_bitmap(bs);
> >> @@ -1361,8 +1357,7 @@ static int parallels_open(BlockDriverState *bs, 
> >> QDict *options, int flags,
> >>   }
> >>
> >>   s->data_start = data_start;
> >> -s->data_end = s->data_start;
> >> -if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
> >> +if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
> >>   /*
> >>* There is not enough unused space to fit to block align 
> >> between BAT
> >>* and actual data. We can't avoid read-modify-write...
> >> @@ -1403,11 +1398,10 @@ static int parallels_open(BlockDriverState *bs, 
> >> QDict *options, int flags,
> >>
> >>   for (i = 0; i < s->bat_size; i++) {
> >>   sector = bat2sect(s, i);
> >> -if (sector + s->tracks > s->data_end) {
> >> -s->data_end = sector + s->tracks;
> >> +if (sector + s->tracks > file_nb_sectors) {
> >> +need_check = true;
> >>   }
> >>   }
> >> -need_check = need_check || s->data_end > file_nb_sectors;
> >>
> >>   ret = parallels_fill_used_bitmap(bs);
> >>   if (ret == -ENOMEM) {
> >> @@ -1461,7 +1455,6 @@ static int 
> >> parallels_truncate_unused_clusters(BlockDriverState *bs)
> >>   end_off

Re: [PATCH 15/19] parallels: Remove unnecessary data_end field

2023-10-07 Thread Alexander Ivanov 




On 10/6/23 21:43, Mike Maslenkin wrote:

On Mon, Oct 2, 2023 at 12:01 PM Alexander Ivanov
 wrote:

Since we have used bitmap, field data_end in BDRVParallelsState is
redundant and can be removed.

Add parallels_data_end() helper and remove data_end handling.

Signed-off-by: Alexander Ivanov 
---
  block/parallels.c | 33 +
  block/parallels.h |  1 -
  2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 48ea5b3f03..80a7171b84 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -265,6 +265,13 @@ static void parallels_free_used_bitmap(BlockDriverState 
*bs)
  g_free(s->used_bmap);
  }

+static int64_t parallels_data_end(BDRVParallelsState *s)
+{
+int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
+data_end += s->used_bmap_size * s->cluster_size;
+return data_end;
+}
+
  int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
   int64_t *clusters)
  {
@@ -275,7 +282,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,

  first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
  if (first_free == s->used_bmap_size) {
-host_off = s->data_end * BDRV_SECTOR_SIZE;
+host_off = parallels_data_end(s);
  prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
  bytes = prealloc_clusters * s->cluster_size;

@@ -297,9 +304,6 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
  s->used_bmap = bitmap_zero_extend(s->used_bmap, s->used_bmap_size,
new_usedsize);
  s->used_bmap_size = new_usedsize;
-if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
-s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
-}
  } else {
  next_used = find_next_bit(s->used_bmap, s->used_bmap_size, 
first_free);

@@ -315,8 +319,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
   * branch. In the other case we are likely re-using hole. Preallocate
   * the space if required by the prealloc_mode.
   */
-if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
-host_off < s->data_end * BDRV_SECTOR_SIZE) {
+if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
  ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
  if (ret < 0) {
  return ret;
@@ -757,13 +760,7 @@ parallels_check_outside_image(BlockDriverState *bs, 
BdrvCheckResult *res,
  }
  }

-if (high_off == 0) {
-res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
-} else {
-res->image_end_offset = high_off + s->cluster_size;
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-}
-
+res->image_end_offset = parallels_data_end(s);
  return 0;
  }

@@ -806,7 +803,6 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult 
*res,
  res->check_errors++;
  return ret;
  }
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;

  parallels_free_used_bitmap(bs);
  ret = parallels_fill_used_bitmap(bs);
@@ -1361,8 +1357,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
  }

  s->data_start = data_start;
-s->data_end = s->data_start;
-if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
+if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
  /*
   * There is not enough unused space to fit to block align between BAT
   * and actual data. We can't avoid read-modify-write...
@@ -1403,11 +1398,10 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,

  for (i = 0; i < s->bat_size; i++) {
  sector = bat2sect(s, i);
-if (sector + s->tracks > s->data_end) {
-s->data_end = sector + s->tracks;
+if (sector + s->tracks > file_nb_sectors) {
+need_check = true;
  }
  }
-need_check = need_check || s->data_end > file_nb_sectors;

  ret = parallels_fill_used_bitmap(bs);
  if (ret == -ENOMEM) {
@@ -1461,7 +1455,6 @@ static int 
parallels_truncate_unused_clusters(BlockDriverState *bs)
  end_off = (end_off + 1) * s->cluster_size;
  }
  end_off += s->data_start * BDRV_SECTOR_SIZE;
-s->data_end = end_off / BDRV_SECTOR_SIZE;
  return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
  }

diff --git a/block/parallels.h b/block/parallels.h
index 18b4f8068e..a6a048d890 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -79,7 +79,6 @@ typedef struct BDRVParallelsState {
  unsigned int bat_size;

  int64_t  data_start;
-int64_t  data_end;
  uint64_t prealloc_size;
  ParallelsPreallocMode prealloc_mode;

--
2.34.1


Is it intended behavior?

Run:
1. ./qemu-img create -f parallels $TEST_IM

Re: [PATCH 15/19] parallels: Remove unnecessary data_end field

2023-10-06 Thread Mike Maslenkin 
On Mon, Oct 2, 2023 at 12:01 PM Alexander Ivanov
 wrote:
>
> Since we have used bitmap, field data_end in BDRVParallelsState is
> redundant and can be removed.
>
> Add parallels_data_end() helper and remove data_end handling.
>
> Signed-off-by: Alexander Ivanov 
> ---
>  block/parallels.c | 33 +
>  block/parallels.h |  1 -
>  2 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 48ea5b3f03..80a7171b84 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -265,6 +265,13 @@ static void parallels_free_used_bitmap(BlockDriverState 
> *bs)
>  g_free(s->used_bmap);
>  }
>
> +static int64_t parallels_data_end(BDRVParallelsState *s)
> +{
> +int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
> +data_end += s->used_bmap_size * s->cluster_size;
> +return data_end;
> +}
> +
>  int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
>   int64_t *clusters)
>  {
> @@ -275,7 +282,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
> *bs,
>
>  first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
>  if (first_free == s->used_bmap_size) {
> -host_off = s->data_end * BDRV_SECTOR_SIZE;
> +host_off = parallels_data_end(s);
>  prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
>  bytes = prealloc_clusters * s->cluster_size;
>
> @@ -297,9 +304,6 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
> *bs,
>  s->used_bmap = bitmap_zero_extend(s->used_bmap, s->used_bmap_size,
>new_usedsize);
>  s->used_bmap_size = new_usedsize;
> -if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
> -s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
> -}
>  } else {
>  next_used = find_next_bit(s->used_bmap, s->used_bmap_size, 
> first_free);
>
> @@ -315,8 +319,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
> *bs,
>   * branch. In the other case we are likely re-using hole. Preallocate
>   * the space if required by the prealloc_mode.
>   */
> -if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
> -host_off < s->data_end * BDRV_SECTOR_SIZE) {
> +if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
>  ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
>  if (ret < 0) {
>  return ret;
> @@ -757,13 +760,7 @@ parallels_check_outside_image(BlockDriverState *bs, 
> BdrvCheckResult *res,
>  }
>  }
>
> -if (high_off == 0) {
> -res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
> -} else {
> -res->image_end_offset = high_off + s->cluster_size;
> -s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
> -}
> -
> +res->image_end_offset = parallels_data_end(s);
>  return 0;
>  }
>
> @@ -806,7 +803,6 @@ parallels_check_leak(BlockDriverState *bs, 
> BdrvCheckResult *res,
>  res->check_errors++;
>  return ret;
>  }
> -s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
>
>  parallels_free_used_bitmap(bs);
>  ret = parallels_fill_used_bitmap(bs);
> @@ -1361,8 +1357,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  }
>
>  s->data_start = data_start;
> -s->data_end = s->data_start;
> -if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
> +if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
>  /*
>   * There is not enough unused space to fit to block align between BAT
>   * and actual data. We can't avoid read-modify-write...
> @@ -1403,11 +1398,10 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>
>  for (i = 0; i < s->bat_size; i++) {
>  sector = bat2sect(s, i);
> -if (sector + s->tracks > s->data_end) {
> -s->data_end = sector + s->tracks;
> +if (sector + s->tracks > file_nb_sectors) {
> +need_check = true;
>  }
>  }
> -need_check = need_check || s->data_end > file_nb_sectors;
>
>  ret = parallels_fill_used_bitmap(bs);
>  if (ret == -ENOMEM) {
> @@ -1461,7 +1455,6 @@ static int 
> parallels_truncate_unused_clusters(BlockDriverState *bs)
>  end_off = (end_off + 1) * s->cluster_size;
>  }
>  end_off += s->data_start * BDRV_SECTOR_SIZE;
> -s->data_end = end_off / BDRV_SECTOR_SIZE;
>  return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, 
> NULL);
>  }
>
> diff --git a/block/parallels.h b/block/parallels.h
> index 18b4f8068e..a6a048d890 100644
> --- a/block/parallels.h
> +++ b/block/parallels.h
> @@ -79,7 +79,6 @@ typedef struct BDRVParallelsState {
>  unsigned int bat_size;
>
>  int64_t  data_start;
> -int64_t  data_end;
>

[PATCH 15/19] parallels: Remove unnecessary data_end field

2023-10-02 Thread Alexander Ivanov 
Since we have used bitmap, field data_end in BDRVParallelsState is
redundant and can be removed.

Add parallels_data_end() helper and remove data_end handling.

Signed-off-by: Alexander Ivanov 
---
 block/parallels.c | 33 +
 block/parallels.h |  1 -
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 48ea5b3f03..80a7171b84 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -265,6 +265,13 @@ static void parallels_free_used_bitmap(BlockDriverState 
*bs)
 g_free(s->used_bmap);
 }
 
+static int64_t parallels_data_end(BDRVParallelsState *s)
+{
+int64_t data_end = s->data_start * BDRV_SECTOR_SIZE;
+data_end += s->used_bmap_size * s->cluster_size;
+return data_end;
+}
+
 int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
  int64_t *clusters)
 {
@@ -275,7 +282,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
 
 first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
 if (first_free == s->used_bmap_size) {
-host_off = s->data_end * BDRV_SECTOR_SIZE;
+host_off = parallels_data_end(s);
 prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
 bytes = prealloc_clusters * s->cluster_size;
 
@@ -297,9 +304,6 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
 s->used_bmap = bitmap_zero_extend(s->used_bmap, s->used_bmap_size,
   new_usedsize);
 s->used_bmap_size = new_usedsize;
-if (host_off + bytes > s->data_end * BDRV_SECTOR_SIZE) {
-s->data_end = (host_off + bytes) / BDRV_SECTOR_SIZE;
-}
 } else {
 next_used = find_next_bit(s->used_bmap, s->used_bmap_size, first_free);
 
@@ -315,8 +319,7 @@ int64_t parallels_allocate_host_clusters(BlockDriverState 
*bs,
  * branch. In the other case we are likely re-using hole. Preallocate
  * the space if required by the prealloc_mode.
  */
-if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
-host_off < s->data_end * BDRV_SECTOR_SIZE) {
+if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
 ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
 if (ret < 0) {
 return ret;
@@ -757,13 +760,7 @@ parallels_check_outside_image(BlockDriverState *bs, 
BdrvCheckResult *res,
 }
 }
 
-if (high_off == 0) {
-res->image_end_offset = s->data_end << BDRV_SECTOR_BITS;
-} else {
-res->image_end_offset = high_off + s->cluster_size;
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-}
-
+res->image_end_offset = parallels_data_end(s);
 return 0;
 }
 
@@ -806,7 +803,6 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult 
*res,
 res->check_errors++;
 return ret;
 }
-s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
 
 parallels_free_used_bitmap(bs);
 ret = parallels_fill_used_bitmap(bs);
@@ -1361,8 +1357,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 
 s->data_start = data_start;
-s->data_end = s->data_start;
-if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) {
+if (s->data_start < (s->header_size >> BDRV_SECTOR_BITS)) {
 /*
  * There is not enough unused space to fit to block align between BAT
  * and actual data. We can't avoid read-modify-write...
@@ -1403,11 +1398,10 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 for (i = 0; i < s->bat_size; i++) {
 sector = bat2sect(s, i);
-if (sector + s->tracks > s->data_end) {
-s->data_end = sector + s->tracks;
+if (sector + s->tracks > file_nb_sectors) {
+need_check = true;
 }
 }
-need_check = need_check || s->data_end > file_nb_sectors;
 
 ret = parallels_fill_used_bitmap(bs);
 if (ret == -ENOMEM) {
@@ -1461,7 +1455,6 @@ static int 
parallels_truncate_unused_clusters(BlockDriverState *bs)
 end_off = (end_off + 1) * s->cluster_size;
 }
 end_off += s->data_start * BDRV_SECTOR_SIZE;
-s->data_end = end_off / BDRV_SECTOR_SIZE;
 return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
 }
 
diff --git a/block/parallels.h b/block/parallels.h
index 18b4f8068e..a6a048d890 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -79,7 +79,6 @@ typedef struct BDRVParallelsState {
 unsigned int bat_size;
 
 int64_t  data_start;
-int64_t  data_end;
 uint64_t prealloc_size;
 ParallelsPreallocMode prealloc_mode;
 
-- 
2.34.1