On 9/18/23 20:00, Denis V. Lunev wrote:
> This patch creates above mentioned helper and moves its usage to the
> beginning of parallels_open(). This simplifies parallels_open() a bit.
>
> The patch also ensures that we store prealloc_size on block driver state
> always in sectors. This makes code cleaner and avoids wrong opinion at
> the assignment that the value is in bytes.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
> block/parallels.c | 72 +++++++++++++++++++++++++++++------------------
> 1 file changed, 44 insertions(+), 28 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index af7be427c9..ae006e7fc7 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -1025,6 +1025,44 @@ static int parallels_update_header(BlockDriverState *bs)
> return bdrv_pwrite_sync(bs->file, 0, size, s->header, 0);
> }
>
> +
> +static int parallels_opts_prealloc(BlockDriverState *bs, QDict *options,
> + Error **errp)
> +{
> + int err;
> + char *buf;
> + int64_t bytes;
> + BDRVParallelsState *s = bs->opaque;
> + Error *local_err = NULL;
> + QemuOpts *opts = qemu_opts_create(¶llels_runtime_opts, NULL, 0, errp);
> + if (!opts) {
> + return -ENOMEM;
> + }
> +
> + err = -EINVAL;
> + if (!qemu_opts_absorb_qdict(opts, options, errp)) {
> + goto done;
> + }
> +
> + bytes = qemu_opt_get_size_del(opts, PARALLELS_OPT_PREALLOC_SIZE, 0);
> + s->prealloc_size = bytes >> BDRV_SECTOR_BITS;
> + buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE);
> + /* prealloc_mode can be downgraded later during allocate_clusters */
> + s->prealloc_mode = qapi_enum_parse(&prealloc_mode_lookup, buf,
> + PRL_PREALLOC_MODE_FALLOCATE,
> + &local_err);
> + g_free(buf);
> + if (local_err != NULL) {
> + error_propagate(errp, local_err);
> + goto done;
> + }
> + err = 0;
> +
> +done:
> + qemu_opts_del(opts);
> + return err;
> +}
> +
> static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
> Error **errp)
> {
> @@ -1033,11 +1071,13 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
> int ret, size, i;
> int64_t file_nb_sectors, sector;
> uint32_t data_start;
> - QemuOpts *opts = NULL;
> - Error *local_err = NULL;
> - char *buf;
> bool data_off_is_correct;
>
> + ret = parallels_opts_prealloc(bs, options, errp);
> + if (ret < 0) {
> + return ret;
> + }
> +
> ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
> if (ret < 0) {
> return ret;
> @@ -1078,6 +1118,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
> ret = -EFBIG;
> goto fail;
> }
> + s->prealloc_size = MAX(s->tracks, s->prealloc_size);
> s->cluster_size = s->tracks << BDRV_SECTOR_BITS;
>
> s->bat_size = le32_to_cpu(ph.bat_entries);
> @@ -1117,29 +1158,6 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
> s->header_size = size;
> }
>
> - opts = qemu_opts_create(¶llels_runtime_opts, NULL, 0, errp);
> - if (!opts) {
> - goto fail_options;
> - }
> -
> - if (!qemu_opts_absorb_qdict(opts, options, errp)) {
> - goto fail_options;
> - }
> -
> - s->prealloc_size =
> - qemu_opt_get_size_del(opts, PARALLELS_OPT_PREALLOC_SIZE, 0);
> - s->prealloc_size = MAX(s->tracks, s->prealloc_size >> BDRV_SECTOR_BITS);
> - buf = qemu_opt_get_del(opts, PARALLELS_OPT_PREALLOC_MODE);
> - /* prealloc_mode can be downgraded later during allocate_clusters */
> - s->prealloc_mode = qapi_enum_parse(&prealloc_mode_lookup, buf,
> - PRL_PREALLOC_MODE_FALLOCATE,
> - &local_err);
> - g_free(buf);
> - if (local_err != NULL) {
> - error_propagate(errp, local_err);
> - goto fail_options;
> - }
> -
> if (ph.ext_off) {
> if (flags & BDRV_O_RDWR) {
> /*
> @@ -1214,10 +1232,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
>
> fail_format:
> error_setg(errp, "Image not in Parallels format");
> -fail_options:
> ret = -EINVAL;
> fail:
> - qemu_opts_del(opts);
> /*
> * "s" object was allocated by g_malloc0 so we can safely
> * try to free its fields even they were not allocated.
Reviewed-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
--
Best regards,
Alexander Ivanov