On 9/17/23 13:40, Mike Maslenkin wrote:
This is not introduced by this patch, but looks like qemu_opts_del(opts) missed.
nice spot!
On Fri, Sep 15, 2023 at 9:41 PM Denis V. Lunev <d...@openvz.org> 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 <d...@openvz.org> --- block/parallels.c | 65 +++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index 428f72de1c..1d5409f2ba 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -1025,6 +1025,38 @@ 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) +{ + 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; + } + + if (!qemu_opts_absorb_qdict(opts, options, errp)) { + return -EINVAL; + } + + bytes = qemu_opt_get_size_del(opts, PARALLELS_OPT_PREALLOC_SIZE, 0); + s->prealloc_size = bytes >> BDRV_SECTOR_BITS;qemu_opt_get_size_del returns uint64_t, so what's a reason to declare "bytes" variable as int64_t and then shift it to the right? I see here it can not be negative, but it's a common to use signed values and not to add explicit check before shifting to right In this file I takes time to ensure that initial values are not negative. Regards, Mike.
apparently no specific reason. Just all variables here dealing with offsets are int64_t. Will change. Thanks, Den
