| Message ID | 20231228101232.372142-19-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parallels: Add full dirty bitmap support | expand |
On 12/28/23 11:12, 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 <alexander.ivanov@virtuozzo.com> > --- > 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 5ed58826bb..2803119699 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -268,6 +268,13 @@ static void parallels_free_used_bitmap(BlockDriverState *bs) > s->used_bmap = NULL; > } > > +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 GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs, > int64_t *clusters) > { > @@ -279,7 +286,7 @@ int64_t GRAPH_RDLOCK 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 = *clusters * s->cluster_size; > prealloc_bytes = prealloc_clusters * s->cluster_size; > @@ -302,9 +309,6 @@ int64_t GRAPH_RDLOCK 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); > > @@ -320,8 +324,7 @@ int64_t GRAPH_RDLOCK 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) { This seems wrong. The check whether the offset is in a tail area or not has been deleted. This looks incorrect. > + if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) { > ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0); > if (ret < 0) { > return ret; > @@ -758,13 +761,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; > } > > @@ -803,8 +800,6 @@ parallels_check_unused_clusters(BlockDriverState *bs, bool truncate) > return ret; > } > > - s->data_end = end_off / BDRV_SECTOR_SIZE; > - > parallels_free_used_bitmap(bs); > ret = parallels_fill_used_bitmap(bs); > if (ret < 0) { > @@ -1394,8 +1389,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... > @@ -1436,11 +1430,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; break; > } > } > - need_check = need_check || s->data_end > file_nb_sectors; > > ret = parallels_fill_used_bitmap(bs); > if (ret == -ENOMEM) { > diff --git a/block/parallels.h b/block/parallels.h > index 9db4f5c908..b494d93139 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; >
diff --git a/block/parallels.c b/block/parallels.c index 5ed58826bb..2803119699 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -268,6 +268,13 @@ static void parallels_free_used_bitmap(BlockDriverState *bs) s->used_bmap = NULL; } +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 GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs, int64_t *clusters) { @@ -279,7 +286,7 @@ int64_t GRAPH_RDLOCK 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 = *clusters * s->cluster_size; prealloc_bytes = prealloc_clusters * s->cluster_size; @@ -302,9 +309,6 @@ int64_t GRAPH_RDLOCK 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); @@ -320,8 +324,7 @@ int64_t GRAPH_RDLOCK 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; @@ -758,13 +761,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; } @@ -803,8 +800,6 @@ parallels_check_unused_clusters(BlockDriverState *bs, bool truncate) return ret; } - s->data_end = end_off / BDRV_SECTOR_SIZE; - parallels_free_used_bitmap(bs); ret = parallels_fill_used_bitmap(bs); if (ret < 0) { @@ -1394,8 +1389,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... @@ -1436,11 +1430,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) { diff --git a/block/parallels.h b/block/parallels.h index 9db4f5c908..b494d93139 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;
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 <alexander.ivanov@virtuozzo.com> --- block/parallels.c | 33 +++++++++++++-------------------- block/parallels.h | 1 - 2 files changed, 13 insertions(+), 21 deletions(-)