* [PATCH v3 01/21] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap()
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-30 8:44 ` Denis V. Lunev
2023-10-27 7:46 ` [PATCH v3 02/21] parallels: Move inactivation code to a separate function Alexander Ivanov
` (19 subsequent siblings)
20 siblings, 1 reply; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
After used bitmap freeng s->used_bmap points to the freed memory. If we try
to free used bitmap one more time it leads to double free error.
Set s->used_bmap to NULL to exclude double free error.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/parallels.c b/block/parallels.c
index 1d695ce7fb..01a61a4ebd 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -245,6 +245,7 @@ static void parallels_free_used_bitmap(BlockDriverState *bs)
BDRVParallelsState *s = bs->opaque;
s->used_bmap_size = 0;
g_free(s->used_bmap);
+ s->used_bmap = NULL;
}
static int64_t coroutine_fn GRAPH_RDLOCK
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v3 01/21] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap()
2023-10-27 7:46 ` [PATCH v3 01/21] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap() Alexander Ivanov
@ 2023-10-30 8:44 ` Denis V. Lunev
0 siblings, 0 replies; 28+ messages in thread
From: Denis V. Lunev @ 2023-10-30 8:44 UTC (permalink / raw)
To: Alexander Ivanov, qemu-devel
Cc: qemu-block, stefanha, vsementsov, kwolf, hreitz
On 10/27/23 09:46, Alexander Ivanov wrote:
> After used bitmap freeng s->used_bmap points to the freed memory. If we try
> to free used bitmap one more time it leads to double free error.
>
> Set s->used_bmap to NULL to exclude double free error.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> block/parallels.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 1d695ce7fb..01a61a4ebd 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -245,6 +245,7 @@ static void parallels_free_used_bitmap(BlockDriverState *bs)
> BDRVParallelsState *s = bs->opaque;
> s->used_bmap_size = 0;
> g_free(s->used_bmap);
> + s->used_bmap = NULL;
> }
>
> static int64_t coroutine_fn GRAPH_RDLOCK
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 02/21] parallels: Move inactivation code to a separate function
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 01/21] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap() Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-30 8:45 ` Denis V. Lunev
2023-10-27 7:46 ` [PATCH v3 03/21] parallels: Make mark_used() a global function Alexander Ivanov
` (18 subsequent siblings)
20 siblings, 1 reply; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
We are going to add parallels image extensions storage and need a separate
function for inactivation code.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 01a61a4ebd..8962bc9fe5 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1428,18 +1428,27 @@ fail:
return ret;
}
+static int parallels_inactivate(BlockDriverState *bs)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int ret;
+
+ s->header->inuse = 0;
+ parallels_update_header(bs);
+
+ /* errors are ignored, so we might as well pass exact=true */
+ ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
+ PREALLOC_MODE_OFF, 0, NULL);
+
+ return ret;
+}
static void parallels_close(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
- s->header->inuse = 0;
- parallels_update_header(bs);
-
- /* errors are ignored, so we might as well pass exact=true */
- bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
- PREALLOC_MODE_OFF, 0, NULL);
+ parallels_inactivate(bs);
}
parallels_free_used_bitmap(bs);
@@ -1478,6 +1487,7 @@ static BlockDriver bdrv_parallels = {
.bdrv_co_check = parallels_co_check,
.bdrv_co_pdiscard = parallels_co_pdiscard,
.bdrv_co_pwrite_zeroes = parallels_co_pwrite_zeroes,
+ .bdrv_inactivate = parallels_inactivate,
};
static void bdrv_parallels_init(void)
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v3 02/21] parallels: Move inactivation code to a separate function
2023-10-27 7:46 ` [PATCH v3 02/21] parallels: Move inactivation code to a separate function Alexander Ivanov
@ 2023-10-30 8:45 ` Denis V. Lunev
0 siblings, 0 replies; 28+ messages in thread
From: Denis V. Lunev @ 2023-10-30 8:45 UTC (permalink / raw)
To: Alexander Ivanov, qemu-devel
Cc: qemu-block, stefanha, vsementsov, kwolf, hreitz
On 10/27/23 09:46, Alexander Ivanov wrote:
> We are going to add parallels image extensions storage and need a separate
> function for inactivation code.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> block/parallels.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 01a61a4ebd..8962bc9fe5 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -1428,18 +1428,27 @@ fail:
> return ret;
> }
>
> +static int parallels_inactivate(BlockDriverState *bs)
> +{
> + BDRVParallelsState *s = bs->opaque;
> + int ret;
> +
> + s->header->inuse = 0;
> + parallels_update_header(bs);
> +
> + /* errors are ignored, so we might as well pass exact=true */
> + ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
> + PREALLOC_MODE_OFF, 0, NULL);
> +
> + return ret;
> +}
>
> static void parallels_close(BlockDriverState *bs)
> {
> BDRVParallelsState *s = bs->opaque;
>
> if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
> - s->header->inuse = 0;
> - parallels_update_header(bs);
> -
> - /* errors are ignored, so we might as well pass exact=true */
> - bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
> - PREALLOC_MODE_OFF, 0, NULL);
> + parallels_inactivate(bs);
> }
>
> parallels_free_used_bitmap(bs);
> @@ -1478,6 +1487,7 @@ static BlockDriver bdrv_parallels = {
> .bdrv_co_check = parallels_co_check,
> .bdrv_co_pdiscard = parallels_co_pdiscard,
> .bdrv_co_pwrite_zeroes = parallels_co_pwrite_zeroes,
> + .bdrv_inactivate = parallels_inactivate,
> };
>
> static void bdrv_parallels_init(void)
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 03/21] parallels: Make mark_used() a global function
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 01/21] parallels: Set s->used_bmap to NULL in parallels_free_used_bitmap() Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 02/21] parallels: Move inactivation code to a separate function Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-30 8:47 ` Denis V. Lunev
2023-10-27 7:46 ` [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper Alexander Ivanov
` (17 subsequent siblings)
20 siblings, 1 reply; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
We will need this function and a function for marking unused clusters (will
be added in the next patch) in parallels-ext.c too. Let it be a global
function parallels_mark_used().
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 14 ++++++++------
block/parallels.h | 3 +++
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 8962bc9fe5..e9a8cbe430 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -178,8 +178,8 @@ static void parallels_set_bat_entry(BDRVParallelsState *s,
bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1);
}
-static int mark_used(BlockDriverState *bs, unsigned long *bitmap,
- uint32_t bitmap_size, int64_t off, uint32_t count)
+int parallels_mark_used(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);
@@ -232,7 +232,8 @@ static int parallels_fill_used_bitmap(BlockDriverState *bs)
continue;
}
- err2 = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off, 1);
+ err2 = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
+ host_off, 1);
if (err2 < 0 && err == 0) {
err = err2;
}
@@ -366,7 +367,8 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
}
}
- ret = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off, to_allocate);
+ ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
+ host_off, to_allocate);
if (ret < 0) {
/* Image consistency is broken. Alarm! */
return ret;
@@ -831,7 +833,7 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
continue;
}
- ret = mark_used(bs, bitmap, bitmap_size, host_off, 1);
+ ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
assert(ret != -E2BIG);
if (ret == 0) {
continue;
@@ -891,7 +893,7 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
* considered, and the bitmap size doesn't change. This specifically
* means that -E2BIG is OK.
*/
- ret = mark_used(bs, bitmap, bitmap_size, host_off, 1);
+ ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
if (ret == -EBUSY) {
res->check_errors++;
goto out_repair_bat;
diff --git a/block/parallels.h b/block/parallels.h
index 6b199443cf..bb18ee0527 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -90,6 +90,9 @@ typedef struct BDRVParallelsState {
Error *migration_blocker;
} BDRVParallelsState;
+int parallels_mark_used(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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v3 03/21] parallels: Make mark_used() a global function
2023-10-27 7:46 ` [PATCH v3 03/21] parallels: Make mark_used() a global function Alexander Ivanov
@ 2023-10-30 8:47 ` Denis V. Lunev
0 siblings, 0 replies; 28+ messages in thread
From: Denis V. Lunev @ 2023-10-30 8:47 UTC (permalink / raw)
To: Alexander Ivanov, qemu-devel
Cc: qemu-block, stefanha, vsementsov, kwolf, hreitz
On 10/27/23 09:46, Alexander Ivanov wrote:
> We will need this function and a function for marking unused clusters (will
> be added in the next patch) in parallels-ext.c too. Let it be a global
> function parallels_mark_used().
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> block/parallels.c | 14 ++++++++------
> block/parallels.h | 3 +++
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 8962bc9fe5..e9a8cbe430 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -178,8 +178,8 @@ static void parallels_set_bat_entry(BDRVParallelsState *s,
> bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1);
> }
>
> -static int mark_used(BlockDriverState *bs, unsigned long *bitmap,
> - uint32_t bitmap_size, int64_t off, uint32_t count)
> +int parallels_mark_used(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);
> @@ -232,7 +232,8 @@ static int parallels_fill_used_bitmap(BlockDriverState *bs)
> continue;
> }
>
> - err2 = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off, 1);
> + err2 = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
> + host_off, 1);
> if (err2 < 0 && err == 0) {
> err = err2;
> }
> @@ -366,7 +367,8 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
> }
> }
>
> - ret = mark_used(bs, s->used_bmap, s->used_bmap_size, host_off, to_allocate);
> + ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
> + host_off, to_allocate);
> if (ret < 0) {
> /* Image consistency is broken. Alarm! */
> return ret;
> @@ -831,7 +833,7 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
> continue;
> }
>
> - ret = mark_used(bs, bitmap, bitmap_size, host_off, 1);
> + ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
> assert(ret != -E2BIG);
> if (ret == 0) {
> continue;
> @@ -891,7 +893,7 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
> * considered, and the bitmap size doesn't change. This specifically
> * means that -E2BIG is OK.
> */
> - ret = mark_used(bs, bitmap, bitmap_size, host_off, 1);
> + ret = parallels_mark_used(bs, bitmap, bitmap_size, host_off, 1);
> if (ret == -EBUSY) {
> res->check_errors++;
> goto out_repair_bat;
> diff --git a/block/parallels.h b/block/parallels.h
> index 6b199443cf..bb18ee0527 100644
> --- a/block/parallels.h
> +++ b/block/parallels.h
> @@ -90,6 +90,9 @@ typedef struct BDRVParallelsState {
> Error *migration_blocker;
> } BDRVParallelsState;
>
> +int parallels_mark_used(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);
>
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (2 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 03/21] parallels: Make mark_used() a global function Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-30 9:06 ` Denis V. Lunev
2023-10-27 7:46 ` [PATCH v3 05/21] parallels: Move host clusters allocation to a separate function Alexander Ivanov
` (16 subsequent siblings)
20 siblings, 1 reply; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
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
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
2023-10-27 7:46 ` [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper Alexander Ivanov
@ 2023-10-30 9:06 ` Denis V. Lunev
2023-10-30 9:09 ` Denis V. Lunev
0 siblings, 1 reply; 28+ messages in thread
From: Denis V. Lunev @ 2023-10-30 9:06 UTC (permalink / raw)
To: Alexander Ivanov, qemu-devel
Cc: qemu-block, stefanha, vsementsov, kwolf, hreitz
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
2023-10-30 9:06 ` Denis V. Lunev
@ 2023-10-30 9:09 ` Denis V. Lunev
2023-11-13 9:53 ` Alexander Ivanov
0 siblings, 1 reply; 28+ messages in thread
From: Denis V. Lunev @ 2023-10-30 9:09 UTC (permalink / raw)
To: Alexander Ivanov, qemu-devel
Cc: qemu-block, stefanha, vsementsov, kwolf, hreitz
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
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper
2023-10-30 9:09 ` Denis V. Lunev
@ 2023-11-13 9:53 ` Alexander Ivanov
0 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-11-13 9:53 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel
Cc: qemu-block, stefanha, vsementsov, kwolf, hreitz
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 05/21] parallels: Move host clusters allocation to a separate function
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (3 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 04/21] parallels: Add parallels_mark_unused() helper Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 06/21] parallels: Set data_end value in parallels_check_leak() Alexander Ivanov
` (15 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
For parallels images extensions we need to allocate host clusters
without any connection to BAT. Move host clusters allocation code to
allocate_host_clusters().
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 128 ++++++++++++++++++++++++----------------------
block/parallels.h | 4 ++
2 files changed, 72 insertions(+), 60 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index a30bb5fe0d..33bb8f1084 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -266,58 +266,31 @@ static void parallels_free_used_bitmap(BlockDriverState *bs)
s->used_bmap = NULL;
}
-static int64_t coroutine_fn GRAPH_RDLOCK
-allocate_clusters(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, int *pnum)
+int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
+ int64_t *clusters)
{
- int ret = 0;
BDRVParallelsState *s = bs->opaque;
- int64_t i, pos, idx, to_allocate, first_free, host_off;
-
- pos = block_status(s, sector_num, nb_sectors, pnum);
- if (pos > 0) {
- return pos;
- }
-
- idx = sector_num / s->tracks;
- to_allocate = DIV_ROUND_UP(sector_num + *pnum, s->tracks) - idx;
-
- /*
- * This function is called only by parallels_co_writev(), which will never
- * pass a sector_num at or beyond the end of the image (because the block
- * layer never passes such a sector_num to that function). Therefore, idx
- * is always below s->bat_size.
- * block_status() will limit *pnum so that sector_num + *pnum will not
- * exceed the image end. Therefore, idx + to_allocate cannot exceed
- * s->bat_size.
- * Note that s->bat_size is an unsigned int, therefore idx + to_allocate
- * will always fit into a uint32_t.
- */
- assert(idx < s->bat_size && idx + to_allocate <= s->bat_size);
+ int64_t first_free, next_used, host_off, prealloc_clusters;
+ int64_t bytes, prealloc_bytes;
+ uint32_t new_usedsize;
+ int ret = 0;
first_free = find_first_zero_bit(s->used_bmap, s->used_bmap_size);
if (first_free == s->used_bmap_size) {
- uint32_t new_usedsize;
- int64_t bytes = to_allocate * s->cluster_size;
- bytes += s->prealloc_size * BDRV_SECTOR_SIZE;
-
host_off = s->data_end * BDRV_SECTOR_SIZE;
+ prealloc_clusters = *clusters + s->prealloc_size / s->tracks;
+ bytes = *clusters * s->cluster_size;
+ prealloc_bytes = prealloc_clusters * s->cluster_size;
- /*
- * We require the expanded size to read back as zero. If the
- * user permitted truncation, we try that; but if it fails, we
- * force the safer-but-slower fallocate.
- */
if (s->prealloc_mode == PRL_PREALLOC_MODE_TRUNCATE) {
- ret = bdrv_co_truncate(bs->file, host_off + bytes,
- false, PREALLOC_MODE_OFF,
- BDRV_REQ_ZERO_WRITE, NULL);
+ ret = bdrv_truncate(bs->file, host_off + prealloc_bytes, false,
+ PREALLOC_MODE_OFF, BDRV_REQ_ZERO_WRITE, NULL);
if (ret == -ENOTSUP) {
s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
}
}
if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE) {
- ret = bdrv_co_pwrite_zeroes(bs->file, host_off, bytes, 0);
+ ret = bdrv_pwrite_zeroes(bs->file, host_off, prealloc_bytes, 0);
}
if (ret < 0) {
return ret;
@@ -327,15 +300,15 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
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 {
- int64_t next_used;
next_used = find_next_bit(s->used_bmap, s->used_bmap_size, first_free);
/* Not enough continuous clusters in the middle, adjust the size */
- if (next_used - first_free < to_allocate) {
- to_allocate = next_used - first_free;
- *pnum = (idx + to_allocate) * s->tracks - sector_num;
- }
+ *clusters = MIN(*clusters, next_used - first_free);
+ bytes = *clusters * s->cluster_size;
host_off = s->data_start * BDRV_SECTOR_SIZE;
host_off += first_free * s->cluster_size;
@@ -347,14 +320,59 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
*/
if (s->prealloc_mode == PRL_PREALLOC_MODE_FALLOCATE &&
host_off < s->data_end * BDRV_SECTOR_SIZE) {
- ret = bdrv_co_pwrite_zeroes(bs->file, host_off,
- s->cluster_size * to_allocate, 0);
+ ret = bdrv_pwrite_zeroes(bs->file, host_off, bytes, 0);
if (ret < 0) {
return ret;
}
}
}
+ ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
+ host_off, *clusters);
+ if (ret < 0) {
+ /* Image consistency is broken. Alarm! */
+ return ret;
+ }
+
+ return host_off;
+}
+
+static int64_t coroutine_fn GRAPH_RDLOCK
+allocate_clusters(BlockDriverState *bs, int64_t sector_num,
+ int nb_sectors, int *pnum)
+{
+ int ret = 0;
+ BDRVParallelsState *s = bs->opaque;
+ int64_t i, pos, idx, to_allocate, host_off;
+
+ pos = block_status(s, sector_num, nb_sectors, pnum);
+ if (pos > 0) {
+ return pos;
+ }
+
+ idx = sector_num / s->tracks;
+ to_allocate = DIV_ROUND_UP(sector_num + *pnum, s->tracks) - idx;
+
+ /*
+ * This function is called only by parallels_co_writev(), which will never
+ * pass a sector_num at or beyond the end of the image (because the block
+ * layer never passes such a sector_num to that function). Therefore, idx
+ * is always below s->bat_size.
+ * block_status() will limit *pnum so that sector_num + *pnum will not
+ * exceed the image end. Therefore, idx + to_allocate cannot exceed
+ * s->bat_size.
+ * Note that s->bat_size is an unsigned int, therefore idx + to_allocate
+ * will always fit into a uint32_t.
+ */
+ assert(idx < s->bat_size && idx + to_allocate <= s->bat_size);
+
+ host_off = parallels_allocate_host_clusters(bs, &to_allocate);
+ if (host_off < 0) {
+ return host_off;
+ }
+
+ *pnum = MIN(*pnum, (idx + to_allocate) * s->tracks - sector_num);
+
/*
* Try to read from backing to fill empty clusters
* FIXME: 1. previous write_zeroes may be redundant
@@ -371,33 +389,23 @@ allocate_clusters(BlockDriverState *bs, int64_t sector_num,
ret = bdrv_co_pread(bs->backing, idx * s->tracks * BDRV_SECTOR_SIZE,
nb_cow_bytes, buf, 0);
- if (ret < 0) {
- qemu_vfree(buf);
- return ret;
+ if (ret == 0) {
+ ret = bdrv_co_pwrite(bs->file, host_off, nb_cow_bytes, buf, 0);
}
- ret = bdrv_co_pwrite(bs->file, s->data_end * BDRV_SECTOR_SIZE,
- nb_cow_bytes, buf, 0);
qemu_vfree(buf);
if (ret < 0) {
+ parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size,
+ host_off, to_allocate);
return ret;
}
}
- ret = parallels_mark_used(bs, s->used_bmap, s->used_bmap_size,
- host_off, to_allocate);
- if (ret < 0) {
- /* Image consistency is broken. Alarm! */
- return ret;
- }
for (i = 0; i < to_allocate; i++) {
parallels_set_bat_entry(s, idx + i,
host_off / BDRV_SECTOR_SIZE / s->off_multiplier);
host_off += s->cluster_size;
}
- if (host_off > s->data_end * BDRV_SECTOR_SIZE) {
- s->data_end = host_off / BDRV_SECTOR_SIZE;
- }
return bat2sect(s, idx) + sector_num % s->tracks;
}
diff --git a/block/parallels.h b/block/parallels.h
index 31ebbd6846..4e7aa6b80f 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -95,7 +95,11 @@ int parallels_mark_used(BlockDriverState *bs, unsigned long *bitmap,
int parallels_mark_unused(BlockDriverState *bs, unsigned long *bitmap,
uint32_t bitmap_size, int64_t off, uint32_t count);
+int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
+ int64_t *clusters);
+
int parallels_read_format_extension(BlockDriverState *bs,
int64_t ext_off, Error **errp);
+
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 06/21] parallels: Set data_end value in parallels_check_leak()
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (4 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 05/21] parallels: Move host clusters allocation to a separate function Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 07/21] parallels: Recreate used bitmap " Alexander Ivanov
` (14 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
In parallels_check_leak() we change file size but don't correct data_end
field of BDRVParallelsState structure. Fix it.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/parallels.c b/block/parallels.c
index 33bb8f1084..d6dbb6757f 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -808,6 +808,7 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
res->check_errors++;
return ret;
}
+ s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
if (explicit) {
res->leaks_fixed += count;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 07/21] parallels: Recreate used bitmap in parallels_check_leak()
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (5 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 06/21] parallels: Set data_end value in parallels_check_leak() Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 08/21] parallels: Add a note about used bitmap in parallels_check_duplicate() Alexander Ivanov
` (13 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
In parallels_check_leak() file can be truncated. In this case the used
bitmap would not comply to the file. Recreate the bitmap after file
truncation.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/parallels.c b/block/parallels.c
index d6dbb6757f..2ce3e40ce3 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -809,6 +809,14 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
return ret;
}
s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
+
+ parallels_free_used_bitmap(bs);
+ ret = parallels_fill_used_bitmap(bs);
+ if (ret == -ENOMEM) {
+ res->check_errors++;
+ return ret;
+ }
+
if (explicit) {
res->leaks_fixed += count;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 08/21] parallels: Add a note about used bitmap in parallels_check_duplicate()
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (6 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 07/21] parallels: Recreate used bitmap " Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 09/21] parallels: Create used bitmap even if checks needed Alexander Ivanov
` (12 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
In parallels_check_duplicate() We use a bitmap for duplication detection.
This bitmap is not related to used_bmap field in BDRVParallelsState. Add
a comment about it to avoid confusion.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/block/parallels.c b/block/parallels.c
index 2ce3e40ce3..481b6992f4 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -839,7 +839,10 @@ parallels_check_duplicate(BlockDriverState *bs, BdrvCheckResult *res,
bool fixed = false;
/*
- * Create a bitmap of used clusters.
+ * Create a bitmap of used clusters. Please note that this bitmap is not
+ * related to used_bmap field in BDRVParallelsState and is created only for
+ * local usage.
+ *
* If a bit is set, there is a BAT entry pointing to this cluster.
* Loop through the BAT entries, check bits relevant to an entry offset.
* If bit is set, this entry is duplicated. Otherwise set the bit.
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 09/21] parallels: Create used bitmap even if checks needed
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (7 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 08/21] parallels: Add a note about used bitmap in parallels_check_duplicate() Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 10/21] parallels: Add dirty bitmaps saving Alexander Ivanov
` (11 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
All the checks were fixed to work with used bitmap. Create used bitmap in
parallels_open() even if need_check is true.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 481b6992f4..925aa9e569 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1423,13 +1423,11 @@ 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 */
+ 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
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 10/21] parallels: Add dirty bitmaps saving
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (8 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 09/21] parallels: Create used bitmap even if checks needed Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 11/21] parallels: Mark parallels_inactivate GRAPH_RDLOCK, guard parallels_close Alexander Ivanov
` (10 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
Now dirty bitmaps can be loaded but there is no their saving. Add code for
dirty bitmap storage.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels-ext.c | 167 ++++++++++++++++++++++++++++++++++++++++++
block/parallels.c | 16 +++-
block/parallels.h | 5 ++
3 files changed, 186 insertions(+), 2 deletions(-)
diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 8a109f005a..0a632a2331 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -24,6 +24,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/error-report.h"
#include "qapi/error.h"
#include "block/block-io.h"
#include "block/block_int.h"
@@ -301,3 +302,169 @@ out:
return ret;
}
+
+static void parallels_save_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+ uint8_t **buf, int *buf_size)
+{
+ BDRVParallelsState *s = bs->opaque;
+ ParallelsFeatureHeader *fh;
+ ParallelsDirtyBitmapFeature *bh;
+ uint64_t *l1_table, l1_size, granularity, limit;
+ int64_t bm_size, ser_size, offset, buf_used;
+ int64_t alloc_size = 1;
+ const char *name;
+ uint8_t *bm_buf;
+ QemuUUID uuid;
+ int ret = 0;
+
+ if (!bdrv_dirty_bitmap_get_persistence(bitmap) ||
+ bdrv_dirty_bitmap_inconsistent(bitmap)) {
+ return;
+ }
+
+ bm_size = bdrv_dirty_bitmap_size(bitmap);
+ granularity = bdrv_dirty_bitmap_granularity(bitmap);
+ limit = bdrv_dirty_bitmap_serialization_coverage(s->cluster_size, bitmap);
+ ser_size = bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size);
+ l1_size = DIV_ROUND_UP(ser_size, s->cluster_size);
+
+ buf_used = l1_size * 8 + sizeof(*fh) + sizeof(*bh);
+ /* Check if there is enough space for the final section */
+ if (*buf_size - buf_used < sizeof(*fh)) {
+ return;
+ }
+
+ name = bdrv_dirty_bitmap_name(bitmap);
+ ret = qemu_uuid_parse(name, &uuid);
+ if (ret < 0) {
+ error_report("Can't save dirty bitmap: ID parsing error: '%s'", name);
+ return;
+ }
+
+ fh = (ParallelsFeatureHeader *)*buf;
+ bh = (ParallelsDirtyBitmapFeature *)(*buf + sizeof(*fh));
+ l1_table = (uint64_t *)((uint8_t *)bh + sizeof(*bh));
+
+ fh->magic = cpu_to_le64(PARALLELS_DIRTY_BITMAP_FEATURE_MAGIC);
+ fh->data_size = cpu_to_le32(l1_size * 8 + sizeof(*bh));
+
+ bh->l1_size = cpu_to_le32(l1_size);
+ bh->size = cpu_to_le64(bm_size >> BDRV_SECTOR_BITS);
+ bh->granularity = cpu_to_le32(granularity >> BDRV_SECTOR_BITS);
+ memcpy(bh->id, &uuid, sizeof(uuid));
+
+ bm_buf = qemu_blockalign(bs, s->cluster_size);
+
+ offset = 0;
+ while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) {
+ uint64_t idx = offset / limit;
+ int64_t cluster_off, end, write_size;
+
+ offset = QEMU_ALIGN_DOWN(offset, limit);
+ end = MIN(bm_size, offset + limit);
+ write_size = bdrv_dirty_bitmap_serialization_size(bitmap, offset,
+ end - offset);
+ assert(write_size <= s->cluster_size);
+
+ bdrv_dirty_bitmap_serialize_part(bitmap, bm_buf, offset, end - offset);
+ if (write_size < s->cluster_size) {
+ memset(bm_buf + write_size, 0, s->cluster_size - write_size);
+ }
+
+ cluster_off = parallels_allocate_host_clusters(bs, &alloc_size);
+ if (cluster_off <= 0) {
+ goto end;
+ }
+
+ ret = bdrv_pwrite(bs->file, cluster_off, s->cluster_size, bm_buf, 0);
+ if (ret < 0) {
+ memset(&fh->magic, 0, sizeof(fh->magic));
+ parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size,
+ cluster_off, 1);
+ goto end;
+ }
+
+ l1_table[idx] = cpu_to_le64(cluster_off >> BDRV_SECTOR_BITS);
+ offset = end;
+ }
+
+ *buf_size -= buf_used;
+ *buf += buf_used;
+
+end:
+ qemu_vfree(bm_buf);
+}
+
+void parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs,
+ Error **errp)
+{
+ BDRVParallelsState *s = bs->opaque;
+ BdrvDirtyBitmap *bitmap;
+ ParallelsFormatExtensionHeader *eh;
+ int remaining = s->cluster_size;
+ uint8_t *buf, *pos;
+ int64_t header_off, alloc_size = 1;
+ g_autofree uint8_t *hash = NULL;
+ size_t hash_len = 0;
+ int ret;
+
+ s->header->ext_off = 0;
+
+ if (!bdrv_has_named_bitmaps(bs)) {
+ return;
+ }
+
+ buf = qemu_blockalign0(bs, s->cluster_size);
+
+ eh = (ParallelsFormatExtensionHeader *)buf;
+ pos = buf + sizeof(*eh);
+
+ eh->magic = cpu_to_le64(PARALLELS_FORMAT_EXTENSION_MAGIC);
+
+ FOR_EACH_DIRTY_BITMAP(bs, bitmap) {
+ parallels_save_bitmap(bs, bitmap, &pos, &remaining);
+ }
+
+ header_off = parallels_allocate_host_clusters(bs, &alloc_size);
+ if (header_off < 0) {
+ error_report("Can't save dirty bitmap: cluster allocation error");
+ ret = header_off;
+ goto end;
+ }
+
+ ret = qcrypto_hash_bytes(QCRYPTO_HASH_ALG_MD5,
+ (const char *)(buf + sizeof(*eh)),
+ s->cluster_size - sizeof(*eh),
+ &hash, &hash_len, errp);
+ if (ret < 0 || hash_len != sizeof(eh->check_sum)) {
+ error_report("Can't save dirty bitmap: hash error");
+ ret = -EINVAL;
+ goto end;
+ }
+ memcpy(eh->check_sum, hash, hash_len);
+
+ ret = bdrv_pwrite(bs->file, header_off, s->cluster_size, buf, 0);
+ if (ret < 0) {
+ error_report("Can't save dirty bitmap: IO error");
+ parallels_mark_unused(bs, s->used_bmap, s->used_bmap_size,
+ header_off, 1);
+ goto end;
+ }
+
+ s->header->ext_off = cpu_to_le64(header_off / BDRV_SECTOR_SIZE);
+end:
+ qemu_vfree(buf);
+}
+
+bool coroutine_fn parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
+ const char *name,
+ uint32_t granularity,
+ Error **errp)
+{
+ if (bdrv_find_dirty_bitmap(bs, name)) {
+ error_setg(errp, "Bitmap already exists: %s", name);
+ return false;
+ }
+
+ return true;
+}
diff --git a/block/parallels.c b/block/parallels.c
index 925aa9e569..2d82e8ff6a 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1468,14 +1468,25 @@ fail:
static int parallels_inactivate(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
+ Error *err = NULL;
int ret;
+ parallels_store_persistent_dirty_bitmaps(bs, &err);
+ if (err != NULL) {
+ error_reportf_err(err, "Lost persistent bitmaps during "
+ "inactivation of node '%s': ",
+ bdrv_get_device_or_node_name(bs));
+ }
+
s->header->inuse = 0;
parallels_update_header(bs);
/* errors are ignored, so we might as well pass exact=true */
- ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
- PREALLOC_MODE_OFF, 0, NULL);
+ ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
+ true, PREALLOC_MODE_OFF, 0, NULL);
+ if (ret < 0) {
+ error_report("Failed to truncate image: %s", strerror(-ret));
+ }
return ret;
}
@@ -1525,6 +1536,7 @@ static BlockDriver bdrv_parallels = {
.bdrv_co_pdiscard = parallels_co_pdiscard,
.bdrv_co_pwrite_zeroes = parallels_co_pwrite_zeroes,
.bdrv_inactivate = parallels_inactivate,
+ .bdrv_co_can_store_new_dirty_bitmap = parallels_co_can_store_new_dirty_bitmap,
};
static void bdrv_parallels_init(void)
diff --git a/block/parallels.h b/block/parallels.h
index 4e7aa6b80f..18b4f8068e 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -100,6 +100,11 @@ int64_t parallels_allocate_host_clusters(BlockDriverState *bs,
int parallels_read_format_extension(BlockDriverState *bs,
int64_t ext_off, Error **errp);
+void parallels_store_persistent_dirty_bitmaps(BlockDriverState *bs,
+ Error **errp);
+bool coroutine_fn
+parallels_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
+ uint32_t granularity, Error **errp);
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 11/21] parallels: Mark parallels_inactivate GRAPH_RDLOCK, guard parallels_close
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (9 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 10/21] parallels: Add dirty bitmaps saving Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 12/21] parallels: Let image extensions work in RW mode Alexander Ivanov
` (9 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
Add GRAPH_RDLOCK annotation to declare parallels_inactivate() have to hold
a reader lock for the graph because it calls
bdrv_get_device_or_node_name(), which accesses the parents list of a node.
Assert we are in the main thread in parallels_close() and guard the code
with GRAPH_RDLOCK_GUARD_MAINLOOP().
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 2d82e8ff6a..4c2cb09e43 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1465,7 +1465,7 @@ fail:
return ret;
}
-static int parallels_inactivate(BlockDriverState *bs)
+static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
Error *err = NULL;
@@ -1491,10 +1491,13 @@ static int parallels_inactivate(BlockDriverState *bs)
return ret;
}
-static void parallels_close(BlockDriverState *bs)
+static void GRAPH_UNLOCKED parallels_close(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
+ GLOBAL_STATE_CODE();
+ GRAPH_RDLOCK_GUARD_MAINLOOP();
+
if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
parallels_inactivate(bs);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 12/21] parallels: Let image extensions work in RW mode
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (10 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 11/21] parallels: Mark parallels_inactivate GRAPH_RDLOCK, guard parallels_close Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 13/21] parallels: Handle L1 entries equal to one Alexander Ivanov
` (8 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
Now we support extensions saving and can let to work with them in
read-write mode.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels-ext.c | 4 ----
block/parallels.c | 17 ++++-------------
2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index 0a632a2331..a0de4f1b07 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -177,10 +177,6 @@ static BdrvDirtyBitmap *parallels_load_bitmap(BlockDriverState *bs,
return NULL;
}
- /* We support format extension only for RO parallels images. */
- assert(!(bs->open_flags & BDRV_O_RDWR));
- bdrv_dirty_bitmap_set_readonly(bitmap, true);
-
return bitmap;
}
diff --git a/block/parallels.c b/block/parallels.c
index 4c2cb09e43..0a2956b45f 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1374,19 +1374,10 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
}
if (ph.ext_off) {
- if (flags & BDRV_O_RDWR) {
- /*
- * It's unsafe to open image RW if there is an extension (as we
- * don't support it). But parallels driver in QEMU historically
- * ignores the extension, so print warning and don't care.
- */
- warn_report("Format Extension ignored in RW mode");
- } else {
- ret = parallels_read_format_extension(
- bs, le64_to_cpu(ph.ext_off) << BDRV_SECTOR_BITS, errp);
- if (ret < 0) {
- goto fail;
- }
+ ret = parallels_read_format_extension(
+ bs, le64_to_cpu(ph.ext_off) << BDRV_SECTOR_BITS, errp);
+ if (ret < 0) {
+ goto fail;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 13/21] parallels: Handle L1 entries equal to one
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (11 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 12/21] parallels: Let image extensions work in RW mode Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 14/21] parallels: Make a loaded dirty bitmap persistent Alexander Ivanov
` (7 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
If all the bits in a dirty bitmap cluster are ones, the cluster shouldn't
be written. Instead the corresponding L1 entry should be set to 1.
Check if all bits in a memory region are ones and set 1 to L1 entries
corresponding clusters filled with ones.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels-ext.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index a0de4f1b07..ebda6b0a01 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -354,7 +354,7 @@ static void parallels_save_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
offset = 0;
while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, bm_size)) >= 0) {
uint64_t idx = offset / limit;
- int64_t cluster_off, end, write_size;
+ int64_t cluster_off, end, write_size, first_zero;
offset = QEMU_ALIGN_DOWN(offset, limit);
end = MIN(bm_size, offset + limit);
@@ -367,6 +367,16 @@ static void parallels_save_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
memset(bm_buf + write_size, 0, s->cluster_size - write_size);
}
+ first_zero = bdrv_dirty_bitmap_next_zero(bitmap, offset, bm_size);
+ if (first_zero < 0) {
+ goto end;
+ }
+ if (first_zero - offset >= s->cluster_size) {
+ l1_table[idx] = 1;
+ offset = end;
+ continue;
+ }
+
cluster_off = parallels_allocate_host_clusters(bs, &alloc_size);
if (cluster_off <= 0) {
goto end;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 14/21] parallels: Make a loaded dirty bitmap persistent
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (12 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 13/21] parallels: Handle L1 entries equal to one Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 15/21] parallels: Reverse a conditional in parallels_check_leak() to reduce indents Alexander Ivanov
` (6 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
After bitmap loading the bitmap is not persistent and is removed on image
saving. Set bitmap persistence to true.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels-ext.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index ebda6b0a01..bb4478c350 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -256,6 +256,7 @@ static int parallels_parse_format_extension(BlockDriverState *bs,
if (!bitmap) {
goto fail;
}
+ bdrv_dirty_bitmap_set_persistence(bitmap, true);
bitmaps = g_slist_append(bitmaps, bitmap);
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 15/21] parallels: Reverse a conditional in parallels_check_leak() to reduce indents
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (13 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 14/21] parallels: Make a loaded dirty bitmap persistent Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 16/21] parallels: Truncate images on the last used cluster Alexander Ivanov
` (5 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
Let the function return a success code if a file size is not bigger than
image_end_offset. Thus we can decrease indents in the next code block.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 72 +++++++++++++++++++++++------------------------
1 file changed, 36 insertions(+), 36 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 0a2956b45f..354fa0f2cc 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -775,7 +775,7 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix, bool explicit)
{
BDRVParallelsState *s = bs->opaque;
- int64_t size;
+ int64_t size, count;
int ret;
size = bdrv_co_getlength(bs->file->bs);
@@ -783,43 +783,43 @@ parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
res->check_errors++;
return size;
}
+ if (size <= res->image_end_offset) {
+ return 0;
+ }
+
+ count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
+ if (explicit) {
+ fprintf(stderr,
+ "%s space leaked at the end of the image %" PRId64 "\n",
+ fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
+ size - res->image_end_offset);
+ res->leaks += count;
+ }
+ if (fix & BDRV_FIX_LEAKS) {
+ Error *local_err = NULL;
+
+ /*
+ * In order to really repair the image, we must shrink it.
+ * That means we have to pass exact=true.
+ */
+ ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
+ PREALLOC_MODE_OFF, 0, &local_err);
+ if (ret < 0) {
+ error_report_err(local_err);
+ res->check_errors++;
+ return ret;
+ }
+ s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
+
+ parallels_free_used_bitmap(bs);
+ ret = parallels_fill_used_bitmap(bs);
+ if (ret == -ENOMEM) {
+ res->check_errors++;
+ return ret;
+ }
- if (size > res->image_end_offset) {
- int64_t count;
- count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
if (explicit) {
- fprintf(stderr,
- "%s space leaked at the end of the image %" PRId64 "\n",
- fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
- size - res->image_end_offset);
- res->leaks += count;
- }
- if (fix & BDRV_FIX_LEAKS) {
- Error *local_err = NULL;
-
- /*
- * In order to really repair the image, we must shrink it.
- * That means we have to pass exact=true.
- */
- ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
- PREALLOC_MODE_OFF, 0, &local_err);
- if (ret < 0) {
- error_report_err(local_err);
- res->check_errors++;
- return ret;
- }
- s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-
- parallels_free_used_bitmap(bs);
- ret = parallels_fill_used_bitmap(bs);
- if (ret == -ENOMEM) {
- res->check_errors++;
- return ret;
- }
-
- if (explicit) {
- res->leaks_fixed += count;
- }
+ res->leaks_fixed += count;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 16/21] parallels: Truncate images on the last used cluster
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (14 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 15/21] parallels: Reverse a conditional in parallels_check_leak() to reduce indents Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 17/21] parallels: Check unused clusters in parallels_check_leak() Alexander Ivanov
` (4 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
On an image closing there can be unused clusters in the end of the image.
Truncate these clusters and update data_end field.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 354fa0f2cc..472311e2e6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1456,6 +1456,23 @@ fail:
return ret;
}
+static int parallels_truncate_unused_clusters(BlockDriverState *bs)
+{
+ BDRVParallelsState *s = bs->opaque;
+ uint64_t end_off = 0;
+ if (s->used_bmap_size > 0) {
+ end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
+ if (end_off == s->used_bmap_size) {
+ end_off = 0;
+ } else {
+ end_off = (end_off + 1) * s->cluster_size;
+ }
+ }
+ end_off += s->data_start * BDRV_SECTOR_SIZE;
+ s->data_end = end_off / BDRV_SECTOR_SIZE;
+ return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
+}
+
static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
@@ -1473,8 +1490,7 @@ static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
parallels_update_header(bs);
/* errors are ignored, so we might as well pass exact=true */
- ret = bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
- true, PREALLOC_MODE_OFF, 0, NULL);
+ ret = parallels_truncate_unused_clusters(bs);
if (ret < 0) {
error_report("Failed to truncate image: %s", strerror(-ret));
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 17/21] parallels: Check unused clusters in parallels_check_leak()
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (15 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 16/21] parallels: Truncate images on the last used cluster Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 18/21] parallels: Remove unnecessary data_end field Alexander Ivanov
` (3 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
Since we have used bitmap, leak check is useless. Transform
parallels_truncate_unused_clusters() to parallels_check_unused_clusters()
helper and use it in leak check.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 121 +++++++++++++++++++++++++---------------------
1 file changed, 67 insertions(+), 54 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 472311e2e6..d497cdbe41 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -770,57 +770,87 @@ parallels_check_outside_image(BlockDriverState *bs, BdrvCheckResult *res,
return 0;
}
+static int64_t parallels_check_unused_clusters(BlockDriverState *bs,
+ bool truncate)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int64_t leak, file_size, end_off = 0;
+ int ret;
+
+ file_size = bdrv_getlength(bs->file->bs);
+ if (file_size < 0) {
+ return file_size;
+ }
+
+ if (s->used_bmap_size > 0) {
+ end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
+ if (end_off == s->used_bmap_size) {
+ end_off = 0;
+ } else {
+ end_off = (end_off + 1) * s->cluster_size;
+ }
+ }
+
+ end_off += s->data_start * BDRV_SECTOR_SIZE;
+ leak = file_size - end_off;
+ if (leak < 0) {
+ return -EINVAL;
+ }
+ if (!truncate || leak == 0) {
+ return leak;
+ }
+
+ ret = bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
+ if (ret) {
+ return ret;
+ }
+
+ s->data_end = end_off / BDRV_SECTOR_SIZE;
+
+ parallels_free_used_bitmap(bs);
+ ret = parallels_fill_used_bitmap(bs);
+ if (ret < 0) {
+ return ret;
+ }
+
+ return leak;
+}
+
static int coroutine_fn GRAPH_RDLOCK
parallels_check_leak(BlockDriverState *bs, BdrvCheckResult *res,
BdrvCheckMode fix, bool explicit)
{
BDRVParallelsState *s = bs->opaque;
- int64_t size, count;
- int ret;
+ int64_t leak, count, size;
+
+ leak = parallels_check_unused_clusters(bs, fix & BDRV_FIX_LEAKS);
+ if (leak < 0) {
+ res->check_errors++;
+ return leak;
+ }
+ if (leak == 0) {
+ return 0;
+ }
size = bdrv_co_getlength(bs->file->bs);
if (size < 0) {
res->check_errors++;
return size;
}
- if (size <= res->image_end_offset) {
+ res->image_end_offset = size;
+
+ if (!explicit) {
return 0;
}
- count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
- if (explicit) {
- fprintf(stderr,
- "%s space leaked at the end of the image %" PRId64 "\n",
- fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
- size - res->image_end_offset);
- res->leaks += count;
- }
+ count = DIV_ROUND_UP(leak, s->cluster_size);
+ fprintf(stderr,
+ "%s space leaked at the end of the image %" PRId64 "\n",
+ fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR", leak);
+ res->leaks += count;
+
if (fix & BDRV_FIX_LEAKS) {
- Error *local_err = NULL;
-
- /*
- * In order to really repair the image, we must shrink it.
- * That means we have to pass exact=true.
- */
- ret = bdrv_co_truncate(bs->file, res->image_end_offset, true,
- PREALLOC_MODE_OFF, 0, &local_err);
- if (ret < 0) {
- error_report_err(local_err);
- res->check_errors++;
- return ret;
- }
- s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-
- parallels_free_used_bitmap(bs);
- ret = parallels_fill_used_bitmap(bs);
- if (ret == -ENOMEM) {
- res->check_errors++;
- return ret;
- }
-
- if (explicit) {
- res->leaks_fixed += count;
- }
+ res->leaks_fixed += count;
}
return 0;
@@ -1456,23 +1486,6 @@ fail:
return ret;
}
-static int parallels_truncate_unused_clusters(BlockDriverState *bs)
-{
- BDRVParallelsState *s = bs->opaque;
- uint64_t end_off = 0;
- if (s->used_bmap_size > 0) {
- end_off = find_last_bit(s->used_bmap, s->used_bmap_size);
- if (end_off == s->used_bmap_size) {
- end_off = 0;
- } else {
- end_off = (end_off + 1) * s->cluster_size;
- }
- }
- end_off += s->data_start * BDRV_SECTOR_SIZE;
- s->data_end = end_off / BDRV_SECTOR_SIZE;
- return bdrv_truncate(bs->file, end_off, true, PREALLOC_MODE_OFF, 0, NULL);
-}
-
static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
{
BDRVParallelsState *s = bs->opaque;
@@ -1490,7 +1503,7 @@ static int GRAPH_RDLOCK parallels_inactivate(BlockDriverState *bs)
parallels_update_header(bs);
/* errors are ignored, so we might as well pass exact=true */
- ret = parallels_truncate_unused_clusters(bs);
+ ret = parallels_check_unused_clusters(bs, true);
if (ret < 0) {
error_report("Failed to truncate image: %s", strerror(-ret));
}
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 18/21] parallels: Remove unnecessary data_end field
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (16 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 17/21] parallels: Check unused clusters in parallels_check_leak() Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 19/21] tests: Add parallels images support to test 165 Alexander Ivanov
` (2 subsequent siblings)
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
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 d497cdbe41..98967dff90 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -266,6 +266,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 parallels_allocate_host_clusters(BlockDriverState *bs,
int64_t *clusters)
{
@@ -277,7 +284,7 @@ int64_t 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;
@@ -300,9 +307,6 @@ int64_t 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);
@@ -318,8 +322,7 @@ int64_t 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;
@@ -760,13 +763,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;
}
@@ -805,8 +802,6 @@ static int64_t parallels_check_unused_clusters(BlockDriverState *bs,
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...
@@ -1438,11 +1432,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 18b4f8068e..a6a048d890 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;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 19/21] tests: Add parallels images support to test 165
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (17 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 18/21] parallels: Remove unnecessary data_end field Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 20/21] tests: Turned on 256, 299, 304 and block-status-cache for parallels format Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 21/21] tests: Add parallels format support to image-fleecing Alexander Ivanov
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
Use a different bitmap name for parallels images because their has own ID
format, and can't contain an arbitrary string.
Replace image reopen by shutdown/launch VM because parallels images doesn't
support reopen.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
tests/qemu-iotests/165 | 40 +++++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
index b24907a62f..f732db257c 100755
--- a/tests/qemu-iotests/165
+++ b/tests/qemu-iotests/165
@@ -38,6 +38,10 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
def setUp(self):
qemu_img('create', '-f', iotests.imgfmt, disk, str(disk_size))
+ if iotests.imgfmt == 'parallels':
+ self.bitmap_name = '00000000-0000-0000-0000-000000000000'
+ else:
+ self.bitmap_name = 'bitmap0'
def tearDown(self):
os.remove(disk)
@@ -50,12 +54,12 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
def getSha256(self):
result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256',
- node='drive0', name='bitmap0')
+ node='drive0', name=self.bitmap_name)
return result['return']['sha256']
def checkBitmap(self, sha256):
result = self.vm.qmp('x-debug-block-dirty-bitmap-sha256',
- node='drive0', name='bitmap0')
+ node='drive0', name=self.bitmap_name)
self.assert_qmp(result, 'return/sha256', sha256);
def writeRegions(self, regions):
@@ -65,7 +69,7 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
def qmpAddBitmap(self):
self.vm.qmp('block-dirty-bitmap-add', node='drive0',
- name='bitmap0', persistent=True)
+ name=self.bitmap_name, persistent=True)
def test_persistent(self):
self.vm = self.mkVm()
@@ -117,7 +121,7 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
assert sha256_1 != sha256_2 # Otherwise, it's not very interesting.
self.vm.cmd('block-dirty-bitmap-clear', node='drive0',
- name='bitmap0')
+ name=self.bitmap_name)
# Start with regions1
@@ -135,16 +139,22 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
self.writeRegions(regions2)
assert sha256_1 == self.getSha256()
- # Reopen to RW
- self.vm.cmd('blockdev-reopen', options=[{
- 'node-name': 'node0',
- 'driver': iotests.imgfmt,
- 'file': {
- 'driver': 'file',
- 'filename': disk
- },
- 'read-only': False
- }])
+ if iotests.imgfmt == 'parallels':
+ # parallels doesn't support reopen
+ self.vm.shutdown()
+ self.vm = self.mkVm()
+ self.vm.launch()
+ else:
+ # Reopen to RW
+ self.vm.cmd('blockdev-reopen', options=[{
+ 'node-name': 'node0',
+ 'driver': iotests.imgfmt,
+ 'file': {
+ 'driver': 'file',
+ 'filename': disk
+ },
+ 'read-only': False
+ }])
# Check that bitmap is reopened to RW and we can write to it.
self.writeRegions(regions2)
@@ -154,6 +164,6 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
if __name__ == '__main__':
- iotests.main(supported_fmts=['qcow2'],
+ iotests.main(supported_fmts=['qcow2', 'parallels'],
supported_protocols=['file'],
unsupported_imgopts=['compat'])
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 20/21] tests: Turned on 256, 299, 304 and block-status-cache for parallels format
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (18 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 19/21] tests: Add parallels images support to test 165 Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
2023-10-27 7:46 ` [PATCH v3 21/21] tests: Add parallels format support to image-fleecing Alexander Ivanov
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
These tests pass with parallels format. Add parallels to supporting
formats for these tests.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
tests/qemu-iotests/256 | 2 +-
tests/qemu-iotests/299 | 2 +-
tests/qemu-iotests/304 | 2 +-
tests/qemu-iotests/tests/block-status-cache | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
index f34af6cef7..1a4c9c6885 100755
--- a/tests/qemu-iotests/256
+++ b/tests/qemu-iotests/256
@@ -26,7 +26,7 @@ from iotests import log
iotests.verify_virtio_scsi_pci_or_ccw()
-iotests.script_initialize(supported_fmts=['qcow2'])
+iotests.script_initialize(supported_fmts=['qcow2', 'parallels'])
size = 64 * 1024 * 1024
with iotests.FilePath('img0') as img0_path, \
diff --git a/tests/qemu-iotests/299 b/tests/qemu-iotests/299
index a7122941fd..d8c4399446 100755
--- a/tests/qemu-iotests/299
+++ b/tests/qemu-iotests/299
@@ -23,7 +23,7 @@ import iotests
# The test is unrelated to formats, restrict it to qcow2 to avoid extra runs
iotests.script_initialize(
- supported_fmts=['qcow2'],
+ supported_fmts=['qcow2', 'parallels'],
)
nbd_sock = iotests.file_path('nbd.sock', base_dir=iotests.sock_dir)
diff --git a/tests/qemu-iotests/304 b/tests/qemu-iotests/304
index 198f282087..1ebf999930 100755
--- a/tests/qemu-iotests/304
+++ b/tests/qemu-iotests/304
@@ -23,7 +23,7 @@
import iotests
from iotests import qemu_img_create, qemu_img_log, file_path
-iotests.script_initialize(supported_fmts=['qcow2'],
+iotests.script_initialize(supported_fmts=['qcow2', 'parallels'],
supported_protocols=['file'])
test_img = file_path('test.qcow2')
diff --git a/tests/qemu-iotests/tests/block-status-cache b/tests/qemu-iotests/tests/block-status-cache
index 5a7bc2c149..ade3d5b169 100755
--- a/tests/qemu-iotests/tests/block-status-cache
+++ b/tests/qemu-iotests/tests/block-status-cache
@@ -131,5 +131,5 @@ class TestBscWithNbd(iotests.QMPTestCase):
if __name__ == '__main__':
# The block-status cache only works on the protocol layer, so to test it,
# we can only use the raw format
- iotests.main(supported_fmts=['raw'],
+ iotests.main(supported_fmts=['raw', 'parallels'],
supported_protocols=['file'])
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH v3 21/21] tests: Add parallels format support to image-fleecing
2023-10-27 7:46 [PATCH v3 00/21] parallels: Add full dirty bitmap support Alexander Ivanov
` (19 preceding siblings ...)
2023-10-27 7:46 ` [PATCH v3 20/21] tests: Turned on 256, 299, 304 and block-status-cache for parallels format Alexander Ivanov
@ 2023-10-27 7:46 ` Alexander Ivanov
20 siblings, 0 replies; 28+ messages in thread
From: Alexander Ivanov @ 2023-10-27 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, den, stefanha, vsementsov, kwolf, hreitz
Use a different bitmap name for parallels images because their has own ID
format, and can't contain an arbitrary string.
Replace hardcoded 'qcow2' format to iotests.imgfmt.
Add 'parallels' to supported formats.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
tests/qemu-iotests/tests/image-fleecing | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index 5e3b2c7e46..dd940b7203 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -28,7 +28,7 @@ import iotests
from iotests import log, qemu_img, qemu_io
iotests.script_initialize(
- supported_fmts=['qcow2'],
+ supported_fmts=['qcow2', 'parallels'],
supported_platforms=['linux'],
required_fmts=['copy-before-write'],
unsupported_imgopts=['compat']
@@ -61,12 +61,17 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
if push_backup:
assert use_cbw
+ if iotests.imgfmt == 'parallels':
+ bitmap_name = '00000000-0000-0000-0000-000000000000'
+ else:
+ bitmap_name = 'bitmap0'
+
log('--- Setting up images ---')
log('')
qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M')
if bitmap:
- qemu_img('bitmap', '--add', base_img_path, 'bitmap0')
+ qemu_img('bitmap', '--add', base_img_path, bitmap_name)
if use_snapshot_access_filter:
assert use_cbw
@@ -75,7 +80,7 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M')
if push_backup:
- qemu_img('create', '-f', 'qcow2', target_img_path, '64M')
+ qemu_img('create', '-f', iotests.imgfmt, target_img_path, '64M')
for p in patterns:
qemu_io('-f', iotests.imgfmt,
@@ -130,7 +135,7 @@ def do_test(vm, use_cbw, use_snapshot_access_filter, base_img_path,
}
if bitmap:
- fl_cbw['bitmap'] = {'node': src_node, 'name': 'bitmap0'}
+ fl_cbw['bitmap'] = {'node': src_node, 'name': bitmap_name}
log(vm.qmp('blockdev-add', fl_cbw))
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread