Re: [PATCH 04/22] parallels: invent parallels_opts_prealloc() helper to parse prealloc opts
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
---
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(_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(_mode_lookup, buf,
+ PRL_PREALLOC_MODE_FALLOCATE,
+ _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(_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(_mode_lookup, buf,
- PRL_PREALLOC_MODE_FALLOCATE,
- _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
--
Best regards,
Alexander Ivanov
[PATCH 04/22] parallels: invent parallels_opts_prealloc() helper to parse prealloc opts
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
---
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(_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(_mode_lookup, buf,
+ PRL_PREALLOC_MODE_FALLOCATE,
+ _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(_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(_mode_lookup, buf,
- PRL_PREALLOC_MODE_FALLOCATE,
- _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.
--
2.34.1
