| Message ID | 20231027074636.430139-5-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parallels: Add full dirty bitmap support | expand |
On 10/27/23 09:46, Alexander Ivanov wrote: > Add a helper to set unused areas in the used bitmap. > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > --- > block/parallels.c | 17 +++++++++++++++++ > block/parallels.h | 2 ++ > 2 files changed, 19 insertions(+) > > diff --git a/block/parallels.c b/block/parallels.c > index e9a8cbe430..a30bb5fe0d 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -195,6 +195,23 @@ int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap, > return 0; > } > > +int parallels_mark_unused(BlockDriverState *bs, unsigned long *bitmap, > + uint32_t bitmap_size, int64_t off, uint32_t count) > +{ > + BDRVParallelsState *s = bs->opaque; > + uint32_t cluster_index = host_cluster_index(s, off); > + unsigned long next_unused; > + if (cluster_index + count > bitmap_size) { > + return -E2BIG; > + } > + next_unused = find_next_zero_bit(bitmap, bitmap_size, cluster_index); > + if (next_unused < cluster_index + count) { > + return -EINVAL; > + } I would limit the search with 'off + count'. There is no necessity to traverse the bitmap further. Den
On 10/30/23 10:06, Denis V. Lunev wrote: > On 10/27/23 09:46, Alexander Ivanov wrote: >> Add a helper to set unused areas in the used bitmap. >> >> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> >> --- >> block/parallels.c | 17 +++++++++++++++++ >> block/parallels.h | 2 ++ >> 2 files changed, 19 insertions(+) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index e9a8cbe430..a30bb5fe0d 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -195,6 +195,23 @@ int parallels_mark_used(BlockDriverState *bs, >> unsigned long *bitmap, >> return 0; >> } >> +int parallels_mark_unused(BlockDriverState *bs, unsigned long >> *bitmap, >> + uint32_t bitmap_size, int64_t off, >> uint32_t count) >> +{ >> + BDRVParallelsState *s = bs->opaque; >> + uint32_t cluster_index = host_cluster_index(s, off); >> + unsigned long next_unused; >> + if (cluster_index + count > bitmap_size) { >> + return -E2BIG; >> + } >> + next_unused = find_next_zero_bit(bitmap, bitmap_size, >> cluster_index); >> + if (next_unused < cluster_index + count) { >> + return -EINVAL; >> + } > I would limit the search with 'off + count'. > There is no necessity to traverse the bitmap further. > > Den I mean 'cluster_index + off' to avoid the confusion. Den
On 10/30/23 10:09, Denis V. Lunev wrote: > On 10/30/23 10:06, Denis V. Lunev wrote: >> On 10/27/23 09:46, Alexander Ivanov wrote: >>> Add a helper to set unused areas in the used bitmap. >>> >>> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> >>> --- >>> block/parallels.c | 17 +++++++++++++++++ >>> block/parallels.h | 2 ++ >>> 2 files changed, 19 insertions(+) >>> >>> diff --git a/block/parallels.c b/block/parallels.c >>> index e9a8cbe430..a30bb5fe0d 100644 >>> --- a/block/parallels.c >>> +++ b/block/parallels.c >>> @@ -195,6 +195,23 @@ int parallels_mark_used(BlockDriverState *bs, >>> unsigned long *bitmap, >>> return 0; >>> } >>> +int parallels_mark_unused(BlockDriverState *bs, unsigned long >>> *bitmap, >>> + uint32_t bitmap_size, int64_t off, >>> uint32_t count) >>> +{ >>> + BDRVParallelsState *s = bs->opaque; >>> + uint32_t cluster_index = host_cluster_index(s, off); >>> + unsigned long next_unused; >>> + if (cluster_index + count > bitmap_size) { >>> + return -E2BIG; >>> + } >>> + next_unused = find_next_zero_bit(bitmap, bitmap_size, >>> cluster_index); >>> + if (next_unused < cluster_index + count) { >>> + return -EINVAL; >>> + } >> I would limit the search with 'off + count'. >> There is no necessity to traverse the bitmap further. >> >> Den > I mean 'cluster_index + off' to avoid the confusion. > > Den Do you mean 'cluster_index + count'? Should I set the same limit in parallels_mark_used()?
diff --git a/block/parallels.c b/block/parallels.c index e9a8cbe430..a30bb5fe0d 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -195,6 +195,23 @@ int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap, return 0; } +int parallels_mark_unused(BlockDriverState *bs, unsigned long *bitmap, + uint32_t bitmap_size, int64_t off, uint32_t count) +{ + BDRVParallelsState *s = bs->opaque; + uint32_t cluster_index = host_cluster_index(s, off); + unsigned long next_unused; + if (cluster_index + count > bitmap_size) { + return -E2BIG; + } + next_unused = find_next_zero_bit(bitmap, bitmap_size, cluster_index); + if (next_unused < cluster_index + count) { + return -EINVAL; + } + bitmap_clear(bitmap, cluster_index, count); + 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 diff --git a/block/parallels.h b/block/parallels.h index bb18ee0527..31ebbd6846 100644 --- a/block/parallels.h +++ b/block/parallels.h @@ -92,6 +92,8 @@ typedef struct BDRVParallelsState { int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap, uint32_t bitmap_size, int64_t off, uint32_t count); +int parallels_mark_unused(BlockDriverState *bs, unsigned long *bitmap, + uint32_t bitmap_size, int64_t off, uint32_t count); int parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off, Error **errp);
Add a helper to set unused areas in the used bitmap. Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- block/parallels.c | 17 +++++++++++++++++ block/parallels.h | 2 ++ 2 files changed, 19 insertions(+)