qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 03/21] parallels: invent parallels_opts_prealloc() helper to


From: Denis V. Lunev
Subject: Re: [PATCH 03/21] parallels: invent parallels_opts_prealloc() helper to parse prealloc opts
Date: Sun, 17 Sep 2023 15:06:02 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1

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 <den@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 <den@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(&parallels_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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]