For parallels images extensions we need to allocate host clusters
without any connection to BAT. Move host clusters allocation code to
parallels_allocate_host_clusters().
This function can be called not only from coroutines so all the
*_co_* functions were replaced by corresponding wrappers.
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
block/parallels.c | 128 ++++++++++++++++++++++++----------------------
block/parallels.h | 3 ++
2 files changed, 71 insertions(+), 60 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 13726fb3d5..658902ae51 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -268,58 +268,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 GRAPH_RDLOCK 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;
@@ -329,15 +302,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;
@@ -349,14 +322,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
@@ -373,33 +391,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 02b857b4a4..493c89e976 100644
--- a/block/parallels.h
+++ b/block/parallels.h
@@ -95,6 +95,9 @@ 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 GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
+ int64_t *clusters);
+
int GRAPH_RDLOCK
parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off,
Error **errp);
--
2.40.1On 12/28/23 11:12, Alexander Ivanov wrote:
> For parallels images extensions we need to allocate host clusters
> without any connection to BAT. Move host clusters allocation code to
> parallels_allocate_host_clusters().
>
> This function can be called not only from coroutines so all the
> *_co_* functions were replaced by corresponding wrappers.
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
> block/parallels.c | 128 ++++++++++++++++++++++++----------------------
> block/parallels.h | 3 ++
> 2 files changed, 71 insertions(+), 60 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 13726fb3d5..658902ae51 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -268,58 +268,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 GRAPH_RDLOCK 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.
> - */
This comment seems useful. I'd better keep it as is.
For now (if we will not have a re-submission, I'll have
the comment back in the code). If the code will be
resubmitted, please add it back.
> 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;
> @@ -329,15 +302,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;
> @@ -349,14 +322,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
> @@ -373,33 +391,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 02b857b4a4..493c89e976 100644
> --- a/block/parallels.h
> +++ b/block/parallels.h
> @@ -95,6 +95,9 @@ 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 GRAPH_RDLOCK parallels_allocate_host_clusters(BlockDriverState *bs,
> + int64_t *clusters);
> +
> int GRAPH_RDLOCK
> parallels_read_format_extension(BlockDriverState *bs, int64_t ext_off,
> Error **errp);
There is a difference in between what we have had before
this patch and after. On a error originally we have had
data_end unchanged, now it points to a wrong location.
May be this would be mitigated later, but I'd prefer
to have data_end updated in mark_unused. That would make
a lot of sense.
Anyway, with data_end dropped at the end of the series,
this would not worth efforts. Thus this is fine.
With a note about comment,
Reviewed-by: Denis V. Lunev <den@openvz.org>
© 2016 - 2024 Red Hat, Inc.