[PATCH v3 04/21] parallels: Add parallels_mark_unused() helper

Alexander Ivanov posted 21 patches 2 years, 1 month ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, "Denis V. Lunev" <den@openvz.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
There is a newer version of this series
[PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
Posted by Alexander Ivanov 2 years, 1 month ago
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;
+    }
+    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);
-- 
2.34.1
Re: [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
Posted by Denis V. Lunev 2 years, 1 month ago
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
Re: [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
Posted by Denis V. Lunev 2 years, 1 month ago
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

Re: [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
Posted by Alexander Ivanov 2 years, 1 month ago

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()?

-- 
Best regards,
Alexander Ivanov