Re: [PATCH 15/19] parallels: Remove unnecessary data_end field
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
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
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
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
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
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
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