Don't let high_off be more than the file size
even if we don't fix the image.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 93bc2750ef..7e8cdbbc3a 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -460,12 +460,12 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
res->corruptions++;
if (fix & BDRV_FIX_ERRORS) {
- prev_off = 0;
s->bat_bitmap[i] = 0;
res->corruptions_fixed++;
flush_bat = true;
- continue;
}
+ prev_off = 0;
+ continue;
}
res->bfi.allocated_clusters++;
--
2.34.1Please ignore the patches in this branch.
Set data_end to the end of the last cluster inside the image.
In such a way we can be sure that corrupted offsets in the BAT
can't affect on the image size.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/parallels.c b/block/parallels.c
index 7e8cdbbc3a..c1ff8bb5f0 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -514,6 +514,8 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
}
}
+ s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
+
out:
qemu_co_mutex_unlock(&s->lock);
return ret;
--
2.34.1This helper will be reused in next patches during parallels_co_check
rework to simplify its code.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/parallels.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index c1ff8bb5f0..52a5cce46c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -165,6 +165,13 @@ static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,
return start_off;
}
+static void parallels_set_bat_entry(BDRVParallelsState *s,
+ uint32_t index, uint32_t offset)
+{
+ s->bat_bitmap[index] = cpu_to_le32(offset);
+ bitmap_set(s->bat_dirty_bmap, bat_entry_off(index) / s->bat_dirty_block, 1);
+}
+
static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, int *pnum)
{
@@ -250,10 +257,8 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
}
for (i = 0; i < to_allocate; i++) {
- s->bat_bitmap[idx + i] = cpu_to_le32(s->data_end / s->off_multiplier);
+ parallels_set_bat_entry(s, idx + i, s->data_end / s->off_multiplier);
s->data_end += s->tracks;
- bitmap_set(s->bat_dirty_bmap,
- bat_entry_off(idx + i) / s->bat_dirty_block, 1);
}
return bat2sect(s, idx) + sector_num % s->tracks;
--
2.34.1BAT is written in the context of conventional operations over
the image inside bdrv_co_flush() when it calls
parallels_co_flush_to_os() callback. Thus we should not
modify BAT array directly, but call parallels_set_bat_entry()
helper and bdrv_co_flush() further on. After that there is no
need to manually write BAT and track its modification.
This makes code more generic and allows to split
parallels_set_bat_entry() for independent pieces.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 52a5cce46c..b4a85b8aa7 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -425,9 +425,8 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
{
BDRVParallelsState *s = bs->opaque;
int64_t size, prev_off, high_off;
- int ret;
+ int ret = 0;
uint32_t i;
- bool flush_bat = false;
size = bdrv_getlength(bs->file->bs);
if (size < 0) {
@@ -465,9 +464,8 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
res->corruptions++;
if (fix & BDRV_FIX_ERRORS) {
- s->bat_bitmap[i] = 0;
+ parallels_set_bat_entry(s, i, 0);
res->corruptions_fixed++;
- flush_bat = true;
}
prev_off = 0;
continue;
@@ -484,15 +482,6 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
prev_off = off;
}
- ret = 0;
- if (flush_bat) {
- ret = bdrv_co_pwrite_sync(bs->file, 0, s->header_size, s->header, 0);
- if (ret < 0) {
- res->check_errors++;
- goto out;
- }
- }
-
res->image_end_offset = high_off + s->cluster_size;
if (size > res->image_end_offset) {
int64_t count;
@@ -523,6 +512,14 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
out:
qemu_co_mutex_unlock(&s->lock);
+
+ if (ret == 0) {
+ ret = bdrv_co_flush(bs);
+ if (ret < 0) {
+ res->check_errors++;
+ }
+ }
+
return ret;
}
--
2.34.1We will add more and more checks so we need a better code structure
in parallels_co_check. Let each check performs in a separate loop
in a separate helper.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/parallels.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index b4a85b8aa7..eea318f809 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -418,6 +418,25 @@ static coroutine_fn int parallels_co_readv(BlockDriverState *bs,
return ret;
}
+static void parallels_check_unclean(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
+{
+ BDRVParallelsState *s = bs->opaque;
+
+ if (!s->header_unclean) {
+ return;
+ }
+
+ fprintf(stderr, "%s image was not closed correctly\n",
+ fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
+ res->corruptions++;
+ if (fix & BDRV_FIX_ERRORS) {
+ /* parallels_close will do the job right */
+ res->corruptions_fixed++;
+ s->header_unclean = false;
+ }
+}
static int coroutine_fn parallels_co_check(BlockDriverState *bs,
BdrvCheckResult *res,
@@ -435,16 +454,8 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
}
qemu_co_mutex_lock(&s->lock);
- if (s->header_unclean) {
- fprintf(stderr, "%s image was not closed correctly\n",
- fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR");
- res->corruptions++;
- if (fix & BDRV_FIX_ERRORS) {
- /* parallels_close will do the job right */
- res->corruptions_fixed++;
- s->header_unclean = false;
- }
- }
+
+ parallels_check_unclean(bs, res, fix);
res->bfi.total_clusters = s->bat_size;
res->bfi.compressed_clusters = 0; /* compression is not supported */
--
2.34.1We will add more and more checks so we need a better code structure
in parallels_co_check. Let each check performs in a separate loop
in a separate helper.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 59 ++++++++++++++++++++++++++++++++++-------------
1 file changed, 43 insertions(+), 16 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index eea318f809..f50cd232aa 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -438,13 +438,50 @@ static void parallels_check_unclean(BlockDriverState *bs,
}
}
+static int parallels_check_outside_image(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
+{
+ BDRVParallelsState *s = bs->opaque;
+ uint32_t i;
+ int64_t off, high_off, size;
+
+ size = bdrv_getlength(bs->file->bs);
+ if (size < 0) {
+ res->check_errors++;
+ return size;
+ }
+
+ high_off = 0;
+ for (i = 0; i < s->bat_size; i++) {
+ off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+ if (off > size) {
+ fprintf(stderr, "%s cluster %u is outside image\n",
+ fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
+ res->corruptions++;
+ if (fix & BDRV_FIX_ERRORS) {
+ parallels_set_bat_entry(s, i, 0);
+ res->corruptions_fixed++;
+ }
+ continue;
+ }
+ if (high_off < off) {
+ high_off = off;
+ }
+ }
+
+ s->data_end = (high_off + s->cluster_size) >> BDRV_SECTOR_BITS;
+
+ return 0;
+}
+
static int coroutine_fn parallels_co_check(BlockDriverState *bs,
BdrvCheckResult *res,
BdrvCheckMode fix)
{
BDRVParallelsState *s = bs->opaque;
int64_t size, prev_off, high_off;
- int ret = 0;
+ int ret;
uint32_t i;
size = bdrv_getlength(bs->file->bs);
@@ -457,6 +494,11 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
parallels_check_unclean(bs, res, fix);
+ ret = parallels_check_outside_image(bs, res, fix);
+ if (ret < 0) {
+ goto out;
+ }
+
res->bfi.total_clusters = s->bat_size;
res->bfi.compressed_clusters = 0; /* compression is not supported */
@@ -469,19 +511,6 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
continue;
}
- /* cluster outside the image */
- if (off > size) {
- fprintf(stderr, "%s cluster %u is outside image\n",
- fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
- res->corruptions++;
- if (fix & BDRV_FIX_ERRORS) {
- parallels_set_bat_entry(s, i, 0);
- res->corruptions_fixed++;
- }
- prev_off = 0;
- continue;
- }
-
res->bfi.allocated_clusters++;
if (off > high_off) {
high_off = off;
@@ -519,8 +548,6 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
}
}
- s->data_end = res->image_end_offset >> BDRV_SECTOR_BITS;
-
out:
qemu_co_mutex_unlock(&s->lock);
--
2.34.1We will add more and more checks so we need a better code structure
in parallels_co_check. Let each check performs in a separate loop
in a separate helper.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 84 +++++++++++++++++++++++++++++------------------
1 file changed, 52 insertions(+), 32 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index f50cd232aa..1874045c51 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -475,14 +475,14 @@ static int parallels_check_outside_image(BlockDriverState *bs,
return 0;
}
-static int coroutine_fn parallels_co_check(BlockDriverState *bs,
- BdrvCheckResult *res,
- BdrvCheckMode fix)
+static int parallels_check_leak(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
{
BDRVParallelsState *s = bs->opaque;
- int64_t size, prev_off, high_off;
- int ret;
+ int64_t size, off, high_off, count;
uint32_t i;
+ int ret;
size = bdrv_getlength(bs->file->bs);
if (size < 0) {
@@ -490,41 +490,16 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
return size;
}
- qemu_co_mutex_lock(&s->lock);
-
- parallels_check_unclean(bs, res, fix);
-
- ret = parallels_check_outside_image(bs, res, fix);
- if (ret < 0) {
- goto out;
- }
-
- res->bfi.total_clusters = s->bat_size;
- res->bfi.compressed_clusters = 0; /* compression is not supported */
-
high_off = 0;
- prev_off = 0;
for (i = 0; i < s->bat_size; i++) {
- int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
- if (off == 0) {
- prev_off = 0;
- continue;
- }
-
- res->bfi.allocated_clusters++;
+ off = bat2sect(s, i) << BDRV_SECTOR_BITS;
if (off > high_off) {
high_off = off;
}
-
- if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
- res->bfi.fragmented_clusters++;
- }
- prev_off = off;
}
res->image_end_offset = high_off + s->cluster_size;
if (size > res->image_end_offset) {
- int64_t count;
count = DIV_ROUND_UP(size - res->image_end_offset, s->cluster_size);
fprintf(stderr, "%s space leaked at the end of the image %" PRId64 "\n",
fix & BDRV_FIX_LEAKS ? "Repairing" : "ERROR",
@@ -542,12 +517,57 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
if (ret < 0) {
error_report_err(local_err);
res->check_errors++;
- goto out;
+ return ret;
}
res->leaks_fixed += count;
}
}
+ return 0;
+}
+
+static int coroutine_fn parallels_co_check(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int64_t prev_off;
+ int ret;
+ uint32_t i;
+
+ qemu_co_mutex_lock(&s->lock);
+
+ parallels_check_unclean(bs, res, fix);
+
+ ret = parallels_check_outside_image(bs, res, fix);
+ if (ret < 0) {
+ goto out;
+ }
+
+ ret = parallels_check_leak(bs, res, fix);
+ if (ret < 0) {
+ goto out;
+ }
+
+ res->bfi.total_clusters = s->bat_size;
+ res->bfi.compressed_clusters = 0; /* compression is not supported */
+
+ prev_off = 0;
+ for (i = 0; i < s->bat_size; i++) {
+ int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+ if (off == 0) {
+ prev_off = 0;
+ continue;
+ }
+
+ res->bfi.allocated_clusters++;
+
+ if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
+ res->bfi.fragmented_clusters++;
+ }
+ prev_off = off;
+ }
+
out:
qemu_co_mutex_unlock(&s->lock);
--
2.34.1We will add more and more checks so we need a better code structure
in parallels_co_check. Let each check performs in a separate loop
in a separate helper.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
block/parallels.c | 53 +++++++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 1874045c51..eacfdea4b6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -526,47 +526,56 @@ static int parallels_check_leak(BlockDriverState *bs,
return 0;
}
-static int coroutine_fn parallels_co_check(BlockDriverState *bs,
- BdrvCheckResult *res,
- BdrvCheckMode fix)
+static void parallels_collect_statistics(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
{
BDRVParallelsState *s = bs->opaque;
- int64_t prev_off;
- int ret;
+ int64_t off, prev_off;
uint32_t i;
- qemu_co_mutex_lock(&s->lock);
-
- parallels_check_unclean(bs, res, fix);
-
- ret = parallels_check_outside_image(bs, res, fix);
- if (ret < 0) {
- goto out;
- }
-
- ret = parallels_check_leak(bs, res, fix);
- if (ret < 0) {
- goto out;
- }
-
res->bfi.total_clusters = s->bat_size;
res->bfi.compressed_clusters = 0; /* compression is not supported */
prev_off = 0;
for (i = 0; i < s->bat_size; i++) {
- int64_t off = bat2sect(s, i) << BDRV_SECTOR_BITS;
+ off = bat2sect(s, i) << BDRV_SECTOR_BITS;
if (off == 0) {
prev_off = 0;
continue;
}
- res->bfi.allocated_clusters++;
-
if (prev_off != 0 && (prev_off + s->cluster_size) != off) {
res->bfi.fragmented_clusters++;
}
+
prev_off = off;
+ res->bfi.allocated_clusters++;
}
+}
+
+static int coroutine_fn parallels_co_check(BlockDriverState *bs,
+ BdrvCheckResult *res,
+ BdrvCheckMode fix)
+{
+ BDRVParallelsState *s = bs->opaque;
+ int ret;
+
+ qemu_co_mutex_lock(&s->lock);
+
+ parallels_check_unclean(bs, res, fix);
+
+ ret = parallels_check_outside_image(bs, res, fix);
+ if (ret < 0) {
+ goto out;
+ }
+
+ ret = parallels_check_leak(bs, res, fix);
+ if (ret < 0) {
+ goto out;
+ }
+
+ parallels_collect_statistics(bs, res, fix);
out:
qemu_co_mutex_unlock(&s->lock);
--
2.34.1Replace the way we use mutex in parallels_co_check() for simplier
and less error prone code.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index eacfdea4b6..8943eccbf5 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -561,30 +561,25 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
BDRVParallelsState *s = bs->opaque;
int ret;
- qemu_co_mutex_lock(&s->lock);
+ WITH_QEMU_LOCK_GUARD(&s->lock) {
+ parallels_check_unclean(bs, res, fix);
- parallels_check_unclean(bs, res, fix);
+ ret = parallels_check_outside_image(bs, res, fix);
+ if (ret < 0) {
+ return ret;
+ }
- ret = parallels_check_outside_image(bs, res, fix);
- if (ret < 0) {
- goto out;
- }
+ ret = parallels_check_leak(bs, res, fix);
+ if (ret < 0) {
+ return ret;
+ }
- ret = parallels_check_leak(bs, res, fix);
- if (ret < 0) {
- goto out;
+ parallels_collect_statistics(bs, res, fix);
}
- parallels_collect_statistics(bs, res, fix);
-
-out:
- qemu_co_mutex_unlock(&s->lock);
-
- if (ret == 0) {
- ret = bdrv_co_flush(bs);
- if (ret < 0) {
- res->check_errors++;
- }
+ ret = bdrv_co_flush(bs);
+ if (ret < 0) {
+ res->check_errors++;
}
return ret;
--
2.34.1All the offsets in the BAT must be lower than the file size.
Fix the check condition for correct check.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
block/parallels.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/parallels.c b/block/parallels.c
index 8943eccbf5..e6e8b9e369 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -455,7 +455,7 @@ static int parallels_check_outside_image(BlockDriverState *bs,
high_off = 0;
for (i = 0; i < s->bat_size; i++) {
off = bat2sect(s, i) << BDRV_SECTOR_BITS;
- if (off > size) {
+ if (off >= size) {
fprintf(stderr, "%s cluster %u is outside image\n",
fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i);
res->corruptions++;
--
2.34.1© 2016 - 2026 Red Hat, Inc.