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

2023-11-13 Thread Alexander Ivanov 




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 
---
  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

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

2023-10-30 Thread Denis V. Lunev 

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 
---
  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

2023-10-30 Thread Denis V. Lunev 

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 
---
  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

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

2023-10-27 Thread Alexander Ivanov 
Add a helper to set unused areas in the used bitmap.

Signed-off-by: Alexander Ivanov 
---
 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