[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] vl: convert -m to QemuOpts
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] vl: convert -m to QemuOpts |
Date: |
Thu, 13 Feb 2014 08:14:45 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 02/13/2014 06:50 AM, Igor Mammedov wrote:
> Adds option to -m
> "mem" - startup memory amount
>
> For compatibility with legacy CLI if suffix-less number is passed,
> it assumes amount in Mb.
>
> Otherwise user is free to use suffixed number using suffixes b,k/K,M,G
>
> Signed-off-by: Igor Mammedov <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> v2:
> - various fixes suggested by Laszlo Ersek <address@hidden>
> - check for overflow
> - check for empty mem="" case
> - reshuffle code to avoid conflicts with another series on list
> - some style/typo fixes
> ---
> qemu-options.hx | 12 ++++++---
> vl.c | 68
> ++++++++++++++++++++++++++++++++++++++++++++++---------
> 2 files changed, 65 insertions(+), 15 deletions(-)
>
> + " configure guest RAM size\n"
> + " mem: initial amount of guest memory (default: "
> + stringify(DEFAULT_RAM_SIZE) "MB)\n",
MB here...
> + QEMU_ARCH_ALL)
> STEXI
> @item -m @var{megs}
> @findex -m
> -Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB.
> Optionally,
> +Set virtual RAM size to @var{size} megabytes. Default is 128 MiB.
> Optionally,
> a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or
> -gigabytes respectively.
> +gigabytes respectively. If @var{size} is provided without unit suffix then MB
> +is assumed.
...and inconsistent on MiB vs MB here. If we're going to use MiB (which
is the correct term), it would be worth using it everywhere.
> + opts = qemu_opts_parse(qemu_find_opts(MEMORY_OPTS),
> + optarg, 1);
> + if (!opts) {
> + exit(1);
I prefer EXIT_FAILURE, but that's a change for another patch (you're
just following conventions).
> + if (!strlen(mem_str)) {
More efficient as 'if (!*mem_str)'...
> + fprintf(stderr, "qemu: missing 'mem' option value\n");
> exit(1);
> }
> - sz = QEMU_ALIGN_UP((uint64_t)value, 8192);
> +
> + sz = qemu_opt_get_size(opts, "mem", ram_size);
> +
> + /* Fix up legacy suffix-less format */
> + if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
...either that, or cache your strlen() result so you only call it once.
> + uint64_t overflow_check = sz;
> +
> + sz <<= 20;
> + if ((sz >> 20) != overflow_check) {
> + fprintf(stderr, "qemu: too large 'mem' option "
> + "value\n");
> + }
> + }
What? We print the warning, but then use the bogus value? Missing exit().
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature