| Message ID | 20230915184130.403366-13-den@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | implement discard operation for Parallels images | expand |
On 9/15/23 20:41, Denis V. Lunev wrote: > If the operation is failed, we need to check image consistency if the > problem is not about memory allocation. > > Bitmap adjustments in allocate_cluster are not performed yet. > They worth to be separate. This was proven useful during debug of this > series. Kept as is for future bissecting. > > It should be specifically noted that used bitmap must be recalculated > if data_off has been fixed during image consistency check. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > --- > block/parallels.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ > block/parallels.h | 3 ++ > 2 files changed, 76 insertions(+) > > diff --git a/block/parallels.c b/block/parallels.c > index bdc4dd081b..2517f35581 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -193,6 +193,58 @@ static int mark_used(BlockDriverState *bs, > return 0; > } > > +/* > + * Collect used bitmap. The image can contain errors, we should fill the > + * bitmap anyway, as much as we can. This information will be used for > + * error resolution. > + */ > +static int parallels_fill_used_bitmap(BlockDriverState *bs) > +{ > + BDRVParallelsState *s = bs->opaque; > + int64_t payload_bytes; > + uint32_t i; > + int err = 0; > + > + payload_bytes = bdrv_co_getlength(bs->file->bs); > + if (payload_bytes < 0) { > + return payload_bytes; > + } > + payload_bytes -= s->data_start * BDRV_SECTOR_SIZE; > + if (payload_bytes < 0) { > + return -EINVAL; > + } > + > + s->used_bmap_size = DIV_ROUND_UP(payload_bytes, s->cluster_size); > + if (s->used_bmap_size == 0) { > + return 0; > + } > + s->used_bmap = bitmap_try_new(s->used_bmap_size); > + if (s->used_bmap == NULL) { > + return -ENOMEM; > + } > + > + for (i = 0; i < s->bat_size; i++) { > + int err2; > + int64_t host_off = bat2sect(s, i) << BDRV_SECTOR_BITS; > + if (host_off == 0) { > + continue; > + } > + > + err2 = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off); > + if (err2 < 0 && err == 0) { > + err = err2; > + } > + } > + return err; > +} > + > +static void parallels_free_used_bitmap(BlockDriverState *bs) > +{ > + BDRVParallelsState *s = bs->opaque; > + s->used_bmap_size = 0; > + g_free(s->used_bmap); > +} > + > static int64_t coroutine_fn GRAPH_RDLOCK > allocate_clusters(BlockDriverState *bs, int64_t sector_num, > int nb_sectors, int *pnum) > @@ -530,8 +582,17 @@ parallels_check_data_off(BlockDriverState *bs, BdrvCheckResult *res, > > res->corruptions++; > if (fix & BDRV_FIX_ERRORS) { > + int err; > s->header->data_off = cpu_to_le32(data_off); > s->data_start = data_off; > + > + parallels_free_used_bitmap(bs); > + err = parallels_fill_used_bitmap(bs); > + if (err == -ENOMEM) { > + res->check_errors++; > + return err; > + } > + > res->corruptions_fixed++; > } > > @@ -1214,6 +1275,14 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > } > need_check = need_check || s->data_end > file_nb_sectors; > > + if (!need_check) { > + ret = parallels_fill_used_bitmap(bs); > + if (ret == -ENOMEM) { > + goto fail; > + } > + need_check = need_check || ret < 0; /* These are correctable errors */ > + } > + > /* > * We don't repair the image here if it's opened for checks. Also we don't > * want to change inactive images and can't change readonly images. > @@ -1243,6 +1312,8 @@ fail: > * "s" object was allocated by g_malloc0 so we can safely > * try to free its fields even they were not allocated. > */ > + parallels_free_used_bitmap(bs); > + > error_free(s->migration_blocker); > g_free(s->bat_dirty_bmap); > qemu_vfree(s->header); > @@ -1263,6 +1334,8 @@ static void parallels_close(BlockDriverState *bs) > PREALLOC_MODE_OFF, 0, NULL); > } > > + parallels_free_used_bitmap(bs); > + > g_free(s->bat_dirty_bmap); > qemu_vfree(s->header); > > diff --git a/block/parallels.h b/block/parallels.h > index 4e53e9572d..6b199443cf 100644 > --- a/block/parallels.h > +++ b/block/parallels.h > @@ -72,6 +72,9 @@ typedef struct BDRVParallelsState { > unsigned long *bat_dirty_bmap; > unsigned int bat_dirty_block; > > + unsigned long *used_bmap; > + unsigned long used_bmap_size; > + > uint32_t *bat_bitmap; > unsigned int bat_size; > Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
diff --git a/block/parallels.c b/block/parallels.c index bdc4dd081b..2517f35581 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -193,6 +193,58 @@ static int mark_used(BlockDriverState *bs, return 0; } +/* + * Collect used bitmap. The image can contain errors, we should fill the + * bitmap anyway, as much as we can. This information will be used for + * error resolution. + */ +static int parallels_fill_used_bitmap(BlockDriverState *bs) +{ + BDRVParallelsState *s = bs->opaque; + int64_t payload_bytes; + uint32_t i; + int err = 0; + + payload_bytes = bdrv_co_getlength(bs->file->bs); + if (payload_bytes < 0) { + return payload_bytes; + } + payload_bytes -= s->data_start * BDRV_SECTOR_SIZE; + if (payload_bytes < 0) { + return -EINVAL; + } + + s->used_bmap_size = DIV_ROUND_UP(payload_bytes, s->cluster_size); + if (s->used_bmap_size == 0) { + return 0; + } + s->used_bmap = bitmap_try_new(s->used_bmap_size); + if (s->used_bmap == NULL) { + return -ENOMEM; + } + + for (i = 0; i < s->bat_size; i++) { + int err2; + int64_t host_off = bat2sect(s, i) << BDRV_SECTOR_BITS; + if (host_off == 0) { + continue; + } + + err2 = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off); + if (err2 < 0 && err == 0) { + err = err2; + } + } + return err; +} + +static void parallels_free_used_bitmap(BlockDriverState *bs) +{ + BDRVParallelsState *s = bs->opaque; + s->used_bmap_size = 0; + g_free(s->used_bmap); +} + static int64_t coroutine_fn GRAPH_RDLOCK allocate_clusters(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum) @@ -530,8 +582,17 @@ parallels_check_data_off(BlockDriverState *bs, BdrvCheckResult *res, res->corruptions++; if (fix & BDRV_FIX_ERRORS) { + int err; s->header->data_off = cpu_to_le32(data_off); s->data_start = data_off; + + parallels_free_used_bitmap(bs); + err = parallels_fill_used_bitmap(bs); + if (err == -ENOMEM) { + res->check_errors++; + return err; + } + res->corruptions_fixed++; } @@ -1214,6 +1275,14 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, } need_check = need_check || s->data_end > file_nb_sectors; + if (!need_check) { + ret = parallels_fill_used_bitmap(bs); + if (ret == -ENOMEM) { + goto fail; + } + need_check = need_check || ret < 0; /* These are correctable errors */ + } + /* * We don't repair the image here if it's opened for checks. Also we don't * want to change inactive images and can't change readonly images. @@ -1243,6 +1312,8 @@ fail: * "s" object was allocated by g_malloc0 so we can safely * try to free its fields even they were not allocated. */ + parallels_free_used_bitmap(bs); + error_free(s->migration_blocker); g_free(s->bat_dirty_bmap); qemu_vfree(s->header); @@ -1263,6 +1334,8 @@ static void parallels_close(BlockDriverState *bs) PREALLOC_MODE_OFF, 0, NULL); } + parallels_free_used_bitmap(bs); + g_free(s->bat_dirty_bmap); qemu_vfree(s->header); diff --git a/block/parallels.h b/block/parallels.h index 4e53e9572d..6b199443cf 100644 --- a/block/parallels.h +++ b/block/parallels.h @@ -72,6 +72,9 @@ typedef struct BDRVParallelsState { unsigned long *bat_dirty_bmap; unsigned int bat_dirty_block; + unsigned long *used_bmap; + unsigned long used_bmap_size; + uint32_t *bat_bitmap; unsigned int bat_size;
If the operation is failed, we need to check image consistency if the problem is not about memory allocation. Bitmap adjustments in allocate_cluster are not performed yet. They worth to be separate. This was proven useful during debug of this series. Kept as is for future bissecting. It should be specifically noted that used bitmap must be recalculated if data_off has been fixed during image consistency check. Signed-off-by: Denis V. Lunev <den@openvz.org> --- block/parallels.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ block/parallels.h | 3 ++ 2 files changed, 76 insertions(+)